Merge branch 'jv-pack-objects-sidechannel-canceled' into 'master'

PackObjectsHookWithSidechannel: return Canceled when client hangs up

Closes gitlab-com/gl-infra/scalability#1270

See merge request gitlab-org/gitaly!3811

2 files changed, 36 insertions, 0 deletions

diff --git a/internal/gitaly/service/hook/pack_objects.go b/internal/gitaly/service/hook/pack_objects.go
index 2fc156aa2..e9eb34430 100644
--- a/internal/gitaly/service/hook/pack_objects.go
+++ b/internal/gitaly/service/hook/pack_objects.go
@@ -13,6 +13,7 @@ import (
 	"io/ioutil"
 	"os"
 	"strings"
+	"syscall"
 
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus"
 	"github.com/prometheus/client_golang/prometheus"
@@ -365,6 +366,12 @@ func (s *server) PackObjectsHookWithSidechannel(ctx context.Context, req *gitaly
 
 	output := func(r io.Reader) (int64, error) { return io.Copy(c, r) }
 	if err := s.packObjectsHook(ctx, req.Repository, req, args, c, output); err != nil {
+		if errors.Is(err, syscall.EPIPE) {
+			// EPIPE is the error we get if we try to write to c after the client has
+			// closed its side of the connection. By convention, we label server side
+			// errors caused by the client disconnecting with the Canceled gRPC code.
+			err = helper.ErrCanceled(err)
+		}
 		return nil, err
 	}
 
diff --git a/internal/gitaly/service/hook/pack_objects_test.go b/internal/gitaly/service/hook/pack_objects_test.go
index b7587e086..fc2c81cfc 100644
--- a/internal/gitaly/service/hook/pack_objects_test.go
+++ b/internal/gitaly/service/hook/pack_objects_test.go
@@ -455,3 +455,32 @@ func TestServer_PackObjectsHookWithSidechannel_invalidArgument(t *testing.T) {
 		})
 	}
 }
+
+func TestServer_PackObjectsHookWithSidechannel_Canceled(t *testing.T) {
+	cfg, repo, _ := cfgWithCache(t)
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	ctx, wt, err := hookPkg.SetupSidechannel(
+		ctx,
+		func(c *net.UnixConn) error {
+			// Simulate a client that successfully initiates a request, but hangs up
+			// before fully consuming the response.
+			_, err := io.WriteString(c, "3dd08961455abf80ef9115f4afdc1c6f968b503c\n--not\n\n")
+			return err
+		},
+	)
+	require.NoError(t, err)
+	defer wt.Close()
+
+	client, conn := newHooksClient(t, runHooksServer(t, cfg, nil))
+	defer conn.Close()
+
+	_, err = client.PackObjectsHookWithSidechannel(ctx, &gitalypb.PackObjectsHookWithSidechannelRequest{
+		Repository: repo,
+		Args:       []string{"pack-objects", "--revs", "--thin", "--stdout", "--progress", "--delta-base-offset"},
+	})
+	testhelper.RequireGrpcError(t, err, codes.Canceled)
+
+	require.NoError(t, wt.Wait())
+}