summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlejandro Rodríguez <alejorro70@gmail.com>2022-10-24 20:04:50 +0200
committerAlejandro Rodríguez <alejorro70@gmail.com>2022-10-24 20:04:50 +0200
commit9a58cbd85d8299c992145b8dc737522b1c19ec75 (patch)
tree03dbb7f286454bf259c6351f85a219a25109cc06
parentb42a398c92565630b541e55c2c6c0ce47cf10b58 (diff)
downloadgitlab-shell-9a58cbd85d8299c992145b8dc737522b1c19ec75.tar.gz
Improve error message for Gitaly `LimitError`s
-rw-r--r--internal/handler/exec.go20
-rw-r--r--internal/handler/exec_test.go31
2 files changed, 50 insertions, 1 deletions
diff --git a/internal/handler/exec.go b/internal/handler/exec.go
index f2d2da8..4036286 100644
--- a/internal/handler/exec.go
+++ b/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)
}
}
diff --git a/internal/handler/exec_test.go b/internal/handler/exec_test.go
index 6404b7a..ecc4435 100644
--- a/internal/handler/exec_test.go
+++ b/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)
+}