diff options
author | Yadong Wang <wangyadong1108@foxmail.com> | 2024-01-04 10:35:36 +0300 |
---|---|---|
committer | GitLab <noreply@gitlab.com> | 2024-01-04 10:35:36 +0300 |
commit | c2deaab9ba54eb606f7a9aa07cbcbee40dfa57e8 (patch) | |
tree | 9b49b452265dd10e60b26fff3e8510dd8e123cc1 /internal/gitaly/service/operations/revert.go | |
parent | 82dcc6a7a5692244932c4f6d4d92b3fa615aeb85 (diff) |
operations: Return structured errors in UserRevert
Diffstat (limited to 'internal/gitaly/service/operations/revert.go')
-rw-r--r-- | internal/gitaly/service/operations/revert.go | 78 |
1 files changed, 62 insertions, 16 deletions
diff --git a/internal/gitaly/service/operations/revert.go b/internal/gitaly/service/operations/revert.go index 90d03a7dd..72e45dc8f 100644 --- a/internal/gitaly/service/operations/revert.go +++ b/internal/gitaly/service/operations/revert.go @@ -6,6 +6,7 @@ import ( "fmt" "time" + "gitlab.com/gitlab-org/gitaly/v16/internal/featureflag" "gitlab.com/gitlab-org/gitaly/v16/internal/git" "gitlab.com/gitlab-org/gitaly/v16/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v16/internal/git/remoterepo" @@ -87,22 +88,46 @@ func (s *Server) UserRevert(ctx context.Context, req *gitalypb.UserRevertRequest if err != nil { var conflictErr *localrepo.MergeTreeConflictError if errors.As(err, &conflictErr) { - return &gitalypb.UserRevertResponse{ - // it's better that this error matches the git2go for now - CreateTreeError: "revert: could not apply due to conflicts", - CreateTreeErrorCode: gitalypb.UserRevertResponse_CONFLICT, - }, nil + if featureflag.ReturnStructuredErrorsInUserRevert.IsEnabled(ctx) { + conflictingFiles := make([][]byte, 0, len(conflictErr.ConflictingFileInfo)) + for _, conflictingFileInfo := range conflictErr.ConflictingFileInfo { + conflictingFiles = append(conflictingFiles, []byte(conflictingFileInfo.FileName)) + } + return nil, structerr.NewFailedPrecondition("revert: there are conflicting files").WithDetail( + &gitalypb.UserRevertError{ + Error: &gitalypb.UserRevertError_MergeConflict{ + MergeConflict: &gitalypb.MergeConflictError{ + ConflictingFiles: conflictingFiles, + }, + }, + }) + } else { + return &gitalypb.UserRevertResponse{ + // it's better that this error matches the git2go for now + CreateTreeError: "revert: could not apply due to conflicts", + CreateTreeErrorCode: gitalypb.UserRevertResponse_CONFLICT, + }, nil + } } return nil, structerr.NewInternal("merge-tree: %w", err) } if oursCommit.TreeId == treeOID.String() { - return &gitalypb.UserRevertResponse{ - // it's better that this error matches the git2go for now - CreateTreeError: "revert: could not apply because the result was empty", - CreateTreeErrorCode: gitalypb.UserRevertResponse_EMPTY, - }, nil + if featureflag.ReturnStructuredErrorsInUserRevert.IsEnabled(ctx) { + return nil, structerr.NewFailedPrecondition("revert: could not apply because the result was empty").WithDetail( + &gitalypb.UserRevertError{ + Error: &gitalypb.UserRevertError_ChangesAlreadyApplied{ + ChangesAlreadyApplied: &gitalypb.ChangesAlreadyAppliedError{}, + }, + }) + } else { + return &gitalypb.UserRevertResponse{ + // it's better that this error matches the git2go for now + CreateTreeError: "revert: could not apply because the result was empty", + CreateTreeErrorCode: gitalypb.UserRevertResponse_EMPTY, + }, nil + } } newrev, err = quarantineRepo.WriteCommit( @@ -166,18 +191,39 @@ func (s *Server) UserRevert(ctx context.Context, req *gitalypb.UserRevertRequest return nil, structerr.NewInternal("checking for ancestry: %w", err) } if !ancestor { - return &gitalypb.UserRevertResponse{ - CommitError: "Branch diverged", - }, nil + if featureflag.ReturnStructuredErrorsInUserRevert.IsEnabled(ctx) { + return nil, structerr.NewFailedPrecondition("revert: branch diverged").WithDetail( + &gitalypb.UserRevertError{ + Error: &gitalypb.UserRevertError_NotAncestor{ + NotAncestor: &gitalypb.NotAncestorError{ + ParentRevision: []byte(oldrev.Revision()), + ChildRevision: []byte(newrev.Revision()), + }, + }, + }) + } else { + return &gitalypb.UserRevertResponse{ + CommitError: "Branch diverged", + }, nil + } } } if err := s.updateReferenceWithHooks(ctx, req.GetRepository(), req.User, quarantineDir, referenceName, newrev, oldrev); err != nil { var customHookErr updateref.CustomHookError if errors.As(err, &customHookErr) { - return &gitalypb.UserRevertResponse{ - PreReceiveError: customHookErr.Error(), - }, nil + if featureflag.ReturnStructuredErrorsInUserRevert.IsEnabled(ctx) { + return nil, structerr.NewPermissionDenied("revert: custom hook error").WithDetail( + &gitalypb.UserRevertError{ + Error: &gitalypb.UserRevertError_CustomHook{ + CustomHook: customHookErr.Proto(), + }, + }) + } else { + return &gitalypb.UserRevertResponse{ + PreReceiveError: customHookErr.Error(), + }, nil + } } return nil, fmt.Errorf("update reference with hooks: %w", err) |