git: Parse and display gitaly limit error

Gitaly will soon return errors of the type LimitError. These can occur
whenever Gitaly determines that it is overloaded and can't keep up with
the current request load. It is a way for Gitaly to impose backpressure
on clients.

When this error gets returned to workhorse, we want to recognize it and
write something meaningful that Git recognizes and can pass through to
the end user.

This commit changes how we handle errors for upload-pack and
receive-pack.

Changelog: changed

workhorse/go.mod

@@ -36,5 +36,6 @@ require (
 	golang.org/x/net v0.0.0-20211008194852-3b03d305991f
 	golang.org/x/tools v0.1.5
 	google.golang.org/grpc v1.40.0
+	google.golang.org/protobuf v1.27.1
 	honnef.co/go/tools v0.1.3
 )

workhorse/internal/git/error.go

 package git
 
+import (
+	"errors"
+	"fmt"
+	"io"
+
+	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
+	"google.golang.org/grpc/status"
+
+	"gitlab.com/gitlab-org/gitlab/workhorse/internal/log"
+)
+
+// For unwrapping google.golang.org/grpc/internal/status.Error
+type grpcErr interface {
+	GRPCStatus() *status.Status
+	Error() string
+}
+
 // For cosmetic purposes in Sentry
 type copyError struct{ error }
+
+// handleLimitErr 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 handleLimitErr(err error, w io.Writer, f func(w io.Writer) error) {
+	var statusErr grpcErr
+	if !errors.As(err, &statusErr) {
+		return
+	}
+
+	if st, ok := status.FromError(statusErr); ok {
+		details := st.Details()
+		for _, detail := range details {
+			switch detail.(type) {
+			case *gitalypb.LimitError:
+				if err := f(w); err != nil {
+					log.WithError(fmt.Errorf("handling limit error: %w", err))
+				}
+			}
+		}
+	}
+}
+
+// writeReceivePackError writes a "server is busy" error message to the
+// git-recieve-pack-result.
+//
+// 0023\x01001aunpack server is busy
+// 00000044\x2GitLab is currently unable to handle this request due to load.
+// 0000
+//
+// We write a line reporting that unpack failed, and then provide some progress
+// information through the side-band 2 channel.
+// See https://gitlab.com/gitlab-org/gitaly/-/tree/jc-return-structured-error-limits
+// for more details.
+func writeReceivePackError(w io.Writer) error {
+	if _, err := fmt.Fprintf(w, "%04x", 35); err != nil {
+		return err
+	}
+
+	if _, err := w.Write([]byte{0x01}); err != nil {
+		return err
+	}
+
+	if _, err := fmt.Fprintf(w, "%04xunpack server is busy\n", 26); err != nil {
+		return err
+	}
+
+	if _, err := w.Write([]byte("0000")); err != nil {
+		return err
+	}
+
+	if _, err := fmt.Fprintf(w, "%04x", 68); err != nil {
+		return err
+	}
+
+	if _, err := w.Write([]byte{0x2}); err != nil {
+		return err
+	}
+
+	if _, err := fmt.Fprint(w, "GitLab is currently unable to handle this request due to load.\n"); err != nil {
+		return err
+	}
+
+	if _, err := w.Write([]byte("0000")); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// writeUploadPackError writes a "server is busy" error message that git
+// understands and prints to stdout. UploadPack expects to receive pack data in
+// PKT-LINE format. An error-line can be passed that begins with ERR.
+// See https://git-scm.com/docs/pack-protocol/2.29.0#_pkt_line_format.
+func writeUploadPackError(w io.Writer) error {
+	_, err := fmt.Fprintf(w, "%04xERR GitLab is currently unable to handle this request due to load.\n", 71)
+	return err
+}

workhorse/internal/git/error_test.go

+package git
+
+import (
+	"bytes"
+	"fmt"
+	"io"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+	"google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/types/known/anypb"
+	"google.golang.org/protobuf/types/known/durationpb"
+)
+
+func TestHandleLimitErr(t *testing.T) {
+	testCases := []struct {
+		desc          string
+		errWriter     func(io.Writer) error
+		expectedBytes []byte
+	}{
+		{
+			desc:      "upload pack",
+			errWriter: writeUploadPackError,
+			expectedBytes: bytes.Join([][]byte{
+				[]byte{'0', '0', '4', '7'},
+				[]byte("ERR GitLab is currently unable to handle this request due to load.\n"),
+			}, []byte{}),
+		},
+		{
+			desc:      "recieve pack",
+			errWriter: writeReceivePackError,
+			expectedBytes: bytes.Join([][]byte{
+				{'0', '0', '2', '3', 1, '0', '0', '1', 'a'},
+				[]byte("unpack server is busy\n"),
+				{'0', '0', '0', '0', '0', '0', '4', '4', 2},
+				[]byte("GitLab is currently unable to handle this request due to load.\n"),
+				{'0', '0', '0', '0'},
+			}, []byte{}),
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			var body bytes.Buffer
+			err := errWithDetail(t, &gitalypb.LimitError{
+				ErrorMessage: "concurrency queue wait time reached",
+				RetryAfter:   durationpb.New(0)})
+
+			handleLimitErr(fmt.Errorf("wrapped error: %w", err), &body, tc.errWriter)
+			require.Equal(t, tc.expectedBytes, body.Bytes())
+		})
+	}
+
+	t.Run("non LimitError", func(t *testing.T) {
+		var body bytes.Buffer
+		err := status.Error(codes.Internal, "some internal error")
+		handleLimitErr(fmt.Errorf("wrapped error: %w", err), &body, writeUploadPackError)
+		require.Equal(t, []byte(nil), body.Bytes())
+
+		handleLimitErr(fmt.Errorf("wrapped error: %w", err), &body, writeReceivePackError)
+		require.Equal(t, []byte(nil), body.Bytes())
+
+	})
+}
+
+// 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 := status.New(codes.Unavailable, "too busy")
+
+	proto := st.Proto()
+	marshaled, err := anypb.New(detail)
+	require.NoError(t, err)
+
+	proto.Details = append(proto.Details, marshaled)
+
+	return status.ErrorProto(proto)
+}

workhorse/internal/git/git-http.go

@@ -22,11 +22,11 @@ const (
 )
 
 func ReceivePack(a *api.API) http.Handler {
-	return postRPCHandler(a, "handleReceivePack", handleReceivePack)
+	return postRPCHandler(a, "handleReceivePack", handleReceivePack, writeReceivePackError)
 }
 
 func UploadPack(a *api.API) http.Handler {
-	return postRPCHandler(a, "handleUploadPack", handleUploadPack)
+	return postRPCHandler(a, "handleUploadPack", handleUploadPack, writeUploadPackError)
 }
 
 func gitConfigOptions(a *api.Response) []string {
@@ -39,7 +39,12 @@ func gitConfigOptions(a *api.Response) []string {
 	return out
 }
 
-func postRPCHandler(a *api.API, name string, handler func(*HttpResponseWriter, *http.Request, *api.Response) error) http.Handler {
+func postRPCHandler(
+	a *api.API,
+	name string,
+	handler func(*HttpResponseWriter, *http.Request, *api.Response) error,
+	errWriter func(io.Writer) error,
+) http.Handler {
 	return repoPreAuthorizeHandler(a, func(rw http.ResponseWriter, r *http.Request, ar *api.Response) {
 		cr := &countReadCloser{ReadCloser: r.Body}
 		r.Body = cr
@@ -50,7 +55,8 @@ func postRPCHandler(a *api.API, name string, handler func(*HttpResponseWriter, *
 		}()
 
 		if err := handler(w, r, ar); err != nil {
-			// If the handler already wrote a response this WriteHeader call is a
+			handleLimitErr(err, w, errWriter)
+			// If the handler, or handleLimitErr already wrote a response this WriteHeader call is a
 			// no-op. It never reaches net/http because GitHttpResponseWriter calls
 			// WriteHeader on its underlying ResponseWriter at most once.
 			w.WriteHeader(500)

workhorse/internal/git/receive-pack.go

@@ -26,7 +26,7 @@ func handleReceivePack(w *HttpResponseWriter, r *http.Request, a *api.Response)
 	}
 
 	if err := smarthttp.ReceivePack(ctx, &a.Repository, a.GL_ID, a.GL_USERNAME, a.GL_REPOSITORY, a.GitConfigOptions, cr, cw, gitProtocol); err != nil {
-		return fmt.Errorf("smarthttp.ReceivePack: %v", err)
+		return fmt.Errorf("smarthttp.ReceivePack: %w", err)
 	}
 
 	return nil