Merge branch 'gitaly-limit-error' into 'main'

Improve error message for Gitaly `LimitError`s

Closes #556

See merge request https://gitlab.com/gitlab-org/gitlab-shell/-/merge_requests/691

Merged-by: Igor Drozdov <[email protected]>
Approved-by: John Cai <[email protected]>
Approved-by: Igor Drozdov <[email protected]>
Co-authored-by: Alejandro Rodríguez <[email protected]>

Author: Igor Drozdov

internal/handler/exec.go

@@ -41,6 +41,24 @@ func NewGitalyCommand(cfg *config.Config, serviceName string, response *accessve
 	return &GitalyCommand{Config: cfg, Response: response, Command: gc}
 }
 
+// processGitalyError handles errors that come back from Gitaly that may be a
+// LimitError. A LimitError is returned by Gitaly when it is at its limit in
+// handling requests. Since this is a known error, we should print a sensible
+// error message to the end user.
+func processGitalyError(statusErr error) error {
+	if st, ok := grpcstatus.FromError(statusErr); ok {
+		details := st.Details()
+		for _, detail := range details {
+			switch detail.(type) {
+			case *pb.LimitError:
+				return grpcstatus.Error(grpccodes.Unavailable, "GitLab is currently unable to handle this request due to load.")
+			}
+		}
+	}
+
+	return grpcstatus.Error(grpccodes.Unavailable, "The git server, Gitaly, is not available at this time. Please contact your administrator.")
+}
+
 // RunGitalyCommand provides a bootstrap for Gitaly commands executed
 // through GitLab-Shell. It ensures that logging, tracing and other
 // common concerns are configured before executing the `handler`.
@@ -61,7 +79,7 @@ func (gc *GitalyCommand) RunGitalyCommand(ctx context.Context, handler GitalyHan
 		ctxlog.WithError(err).WithFields(log.Fields{"exit_status": exitStatus}).Error("Failed to execute Git command")
 
 		if grpcstatus.Code(err) == grpccodes.Unavailable {
-			return grpcstatus.Error(grpccodes.Unavailable, "The git server, Gitaly, is not available at this time. Please contact your administrator.")
+			return processGitalyError(err)
 		}
 	}
 

internal/handler/exec_test.go

@@ -16,6 +16,9 @@ import (
 	"gitlab.com/gitlab-org/gitlab-shell/v14/internal/config"
 	"gitlab.com/gitlab-org/gitlab-shell/v14/internal/gitlabnet/accessverifier"
 	"gitlab.com/gitlab-org/gitlab-shell/v14/internal/sshenv"
+	"google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/types/known/anypb"
+	"google.golang.org/protobuf/types/known/durationpb"
 )
 
 func makeHandler(t *testing.T, err error) func(context.Context, *grpc.ClientConn) (int32, error) {
@@ -88,6 +91,21 @@ func TestUnavailableGitalyErr(t *testing.T) {
 	require.Equal(t, err, grpcstatus.Error(grpccodes.Unavailable, "The git server, Gitaly, is not available at this time. Please contact your administrator."))
 }
 
+func TestGitalyLimitErr(t *testing.T) {
+	cmd := NewGitalyCommand(
+		newConfig(),
+		string(commandargs.UploadPack),
+		&accessverifier.Response{
+			Gitaly: accessverifier.Gitaly{Address: "tcp://localhost:9999"},
+		},
+	)
+	limitErr := errWithDetail(t, &pb.LimitError{
+		ErrorMessage: "concurrency queue wait time reached",
+		RetryAfter:   durationpb.New(0)})
+	err := cmd.RunGitalyCommand(context.Background(), makeHandler(t, limitErr))
+	require.Equal(t, err, grpcstatus.Error(grpccodes.Unavailable, "GitLab is currently unable to handle this request due to load."))
+}
+
 func TestRunGitalyCommandMetadata(t *testing.T) {
 	tests := []struct {
 		name string
@@ -211,3 +229,16 @@ func newConfig() *config.Config {
 	cfg.GitalyClient.InitSidechannelRegistry(context.Background())
 	return cfg
 }
+
+// errWithDetail adds the given details to the error if it is a gRPC status whose code is not OK.
+func errWithDetail(t *testing.T, detail proto.Message) error {
+	st := grpcstatus.New(grpccodes.Unavailable, "too busy")
+
+	proto := st.Proto()
+	marshaled, err := anypb.New(detail)
+	require.NoError(t, err)
+
+	proto.Details = append(proto.Details, marshaled)
+
+	return grpcstatus.ErrorProto(proto)
+}