operations: Properly report unwrapped custom hook errors

Custom hook errors must be returned to the caller without modification
given that they may contain special prefixes that are parsed by the
Rails application. We have thus introduced the CustomHookError struct so
that we can unwrap that kind of error and then pass the error message to
the caller directly. But while we properly unwrap the error everywhere,
we don't in fact bubble up the contained error as expected.

Fix this and bubble up the unwrapped error's message. This allows us to
change error messages in those code paths to properly leave breadcrumbs
of the error trace.

2 files changed, 12 insertions, 7 deletions

diff --git a/internal/gitaly/service/operations/cherry_pick.go b/internal/gitaly/service/operations/cherry_pick.go
index b35a078b8..7b8994479 100644
--- a/internal/gitaly/service/operations/cherry_pick.go
+++ b/internal/gitaly/service/operations/cherry_pick.go
@@ -175,13 +175,15 @@ func (s *Server) UserCherryPick(ctx context.Context, req *gitalypb.UserCherryPic
 
 	if err := s.updateReferenceWithHooks(ctx, req.GetRepository(), req.User, quarantineDir, referenceName, newrev, oldrev); err != nil {
 		if featureflag.CherryPickStructuredErrors.IsEnabled(ctx) {
-			if errors.As(err, &updateref.CustomHookError{}) {
+			var customHookErr updateref.CustomHookError
+
+			if errors.As(err, &customHookErr) {
 				detailedErr, errGeneratingDetailedErr := helper.ErrWithDetails(
 					helper.ErrFailedPrecondition(errors.New("access check failed")),
 					&gitalypb.UserCherryPickError{
 						Error: &gitalypb.UserCherryPickError_AccessCheck{
 							AccessCheck: &gitalypb.AccessCheckError{
-								ErrorMessage: strings.TrimSuffix(err.Error(), "\n"),
+								ErrorMessage: strings.TrimSuffix(customHookErr.Error(), "\n"),
 							},
 						},
 					})
@@ -196,9 +198,10 @@ func (s *Server) UserCherryPick(ctx context.Context, req *gitalypb.UserCherryPic
 			return nil, fmt.Errorf("update reference with hooks: %w", err)
 		}
 
-		if errors.As(err, &updateref.CustomHookError{}) {
+		var customHookErr updateref.CustomHookError
+		if errors.As(err, &customHookErr) {
 			return &gitalypb.UserCherryPickResponse{
-				PreReceiveError: err.Error(),
+				PreReceiveError: customHookErr.Error(),
 			}, nil
 		}
 
diff --git a/internal/gitaly/service/operations/rebase.go b/internal/gitaly/service/operations/rebase.go
index 8e1340fce..1ab3458e8 100644
--- a/internal/gitaly/service/operations/rebase.go
+++ b/internal/gitaly/service/operations/rebase.go
@@ -130,16 +130,18 @@ func (s *Server) UserRebaseConfirmable(stream gitalypb.OperationService_UserReba
 		branch,
 		newrev,
 		oldrev,
-		header.GitPushOptions...); err != nil {
+		header.GitPushOptions...,
+	); err != nil {
+		var customHookErr updateref.CustomHookError
 		switch {
-		case errors.As(err, &updateref.CustomHookError{}):
+		case errors.As(err, &customHookErr):
 			if featureflag.UserRebaseConfirmableImprovedErrorHandling.IsEnabled(ctx) {
 				detailedErr, err := helper.ErrWithDetails(
 					helper.ErrPermissionDeniedf("access check: %q", err),
 					&gitalypb.UserRebaseConfirmableError{
 						Error: &gitalypb.UserRebaseConfirmableError_AccessCheck{
 							AccessCheck: &gitalypb.AccessCheckError{
-								ErrorMessage: err.Error(),
+								ErrorMessage: customHookErr.Error(),
 							},
 						},
 					},