updateref: Generalize `PreReceiveError` according to its usage

When any of the hooks fails to execute in our `UpdaterWithHooks`, then
we return a `PreReceiveError` to the caller. This is quite misleading
though: we not only execute the pre-receive hook, but also others, and
they all return the same error.

Refactor the error to instead be a `HookError`. While at it, we also
embed stdout and stderr in the error such that it can handle generation
of the error message itself instead of using `hookErrorMessage()`, and
implement `Unwrap()` such that we can use `errors.As()` to retrieve the
embedded error. This will be used at a later point to distinguish
`HookError`s and `NotAllowedError`s.

1 files changed, 6 insertions, 3 deletions

diff --git a/internal/gitaly/service/operations/submodules_test.go b/internal/gitaly/service/operations/submodules_test.go
index 9c8c7c7fa..7f5f653f3 100644
--- a/internal/gitaly/service/operations/submodules_test.go
+++ b/internal/gitaly/service/operations/submodules_test.go
@@ -3,7 +3,7 @@ package operations
 import (
 	"bytes"
 	"fmt"
-	"strings"
+	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -11,6 +11,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/localrepo"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/lstree"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/helper/text"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
@@ -160,7 +161,8 @@ func TestUserUpdateSubmoduleQuarantine(t *testing.T) {
 
 	// Set up a hook that parses the new object and then aborts the update. Like this, we can
 	// assert that the object does not end up in the main repository.
-	hookScript := fmt.Sprintf("#!/bin/sh\n%s rev-parse $3^{commit} && exit 1", cfg.Git.BinPath)
+	outputPath := filepath.Join(testhelper.TempDir(t), "output")
+	hookScript := fmt.Sprintf("#!/bin/sh\n%s rev-parse $3^{commit} >%s && exit 1", cfg.Git.BinPath, outputPath)
 	gittest.WriteCustomHook(t, repoPath, "update", []byte(hookScript))
 
 	response, err := client.UserUpdateSubmodule(ctx, &gitalypb.UserUpdateSubmoduleRequest{
@@ -176,7 +178,8 @@ func TestUserUpdateSubmoduleQuarantine(t *testing.T) {
 	require.NotNil(t, response)
 	require.NotEmpty(t, response.GetPreReceiveError())
 
-	oid, err := git.NewObjectIDFromHex(strings.TrimSpace(response.PreReceiveError))
+	hookOutput := testhelper.MustReadFile(t, outputPath)
+	oid, err := git.NewObjectIDFromHex(text.ChompBytes(hookOutput))
 	require.NoError(t, err)
 	exists, err := repo.HasRevision(ctx, oid.Revision()+"^{commit}")
 	require.NoError(t, err)