ssh: Recognize cancelled pushes in SSHReceivePackpks-ssh-receive-pack-missing-error

Recently we've fixed cases where fetches which are cancelled by the user
will be logged as `Internal` errors  and thus easily impact our SLOs.
This shortcoming has been fixed has been fixed via a2fee3dd5 (ssh:
Improve errors for fetches canceled by the user, 2022-07-20), but now
that we started to report errors in SSHReceivePack we are likely to hit
the same issue there.

Let's apply the same fix to SSHReceivePack and special-case this
specific error.

2 files changed, 16 insertions, 1 deletions

diff --git a/internal/gitaly/service/ssh/receive_pack.go b/internal/gitaly/service/ssh/receive_pack.go
index faf913a3f..929898f8d 100644
--- a/internal/gitaly/service/ssh/receive_pack.go
+++ b/internal/gitaly/service/ssh/receive_pack.go
@@ -3,6 +3,8 @@ package ssh
 import (
 	"errors"
 	"fmt"
+	"io"
+	"strings"
 	"sync"
 
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus"
@@ -55,9 +57,15 @@ func (s *server) sshReceivePack(stream gitalypb.SSHService_SSHReceivePackServer,
 	stdout := streamio.NewSyncWriter(&m, func(p []byte) error {
 		return stream.Send(&gitalypb.SSHReceivePackResponse{Stdout: p})
 	})
+
+	// We both need to listen in on the stderr stream in order to be able to judge what exactly
+	// is happening, but also relay the output to the client. We thus create a MultiWriter to
+	// enable both at the same time.
+	var stderrBuilder strings.Builder
 	stderr := streamio.NewSyncWriter(&m, func(p []byte) error {
 		return stream.Send(&gitalypb.SSHReceivePackResponse{Stderr: p})
 	})
+	stderr = io.MultiWriter(&stderrBuilder, stderr)
 
 	repoPath, err := s.locator.GetRepoPath(req.Repository)
 	if err != nil {
@@ -100,6 +108,13 @@ func (s *server) sshReceivePack(stream gitalypb.SSHService_SSHReceivePackServer,
 			ctxlogrus.Extract(ctx).WithError(errSend).Error("send final status code")
 		}
 
+		// Detect the case where the user has cancelled the push and log it with a proper
+		// gRPC error code. We can't do anything about this error anyway and it is a totally
+		// valid outcome.
+		if stderrBuilder.String() == "fatal: the remote end hung up unexpectedly\n" {
+			return helper.ErrCanceledf("user canceled the push")
+		}
+
 		return fmt.Errorf("cmd wait: %v", err)
 	}
 
diff --git a/internal/gitaly/service/ssh/receive_pack_test.go b/internal/gitaly/service/ssh/receive_pack_test.go
index 072f5345f..0cd757de0 100644
--- a/internal/gitaly/service/ssh/receive_pack_test.go
+++ b/internal/gitaly/service/ssh/receive_pack_test.go
@@ -235,7 +235,7 @@ func TestReceivePack_client(t *testing.T) {
 			writeRequest: func(t *testing.T, stdin io.Writer) {
 				gittest.WritePktlinef(t, stdin, "%[1]s %[1]s refs/heads/main", gittest.DefaultObjectHash.ZeroOID)
 			},
-			expectedErr:       helper.ErrInternalf("cmd wait: exit status 128"),
+			expectedErr:       helper.ErrCanceledf("user canceled the push"),
 			expectedErrorCode: 128,
 			expectedStderr:    "fatal: the remote end hung up unexpectedly\n",
 		},