diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-07-15 13:55:33 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-07-15 13:59:18 +0300 |
commit | b4e003aab8e0c6173f50b4229af08f7b40b45f0f (patch) | |
tree | d47b04303e5acbcbda044af451bf7fc67b7d3406 | |
parent | 225f65632a92cfce478c02fad065f01b82592eb6 (diff) |
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.
-rw-r--r-- | internal/gitaly/service/operations/cherry_pick.go | 11 | ||||
-rw-r--r-- | internal/gitaly/service/operations/rebase.go | 8 |
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(), }, }, }, |