diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-11-10 18:08:15 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-11-10 18:08:15 +0300 |
commit | 2bd521042d2f57792c1fba46900f09bc5a32929e (patch) | |
tree | 1c89708dfc36bea2c783d6ee51b8c846344f2a93 | |
parent | ef4f348ae0376ede3f4383cac0cab8cfc115723d (diff) | |
parent | 50f80d98286cc612a67ea319eb01b2b084ab7007 (diff) |
Merge branch 'revert-9737f0aa' into 'master'
Revert "Merge branch 'ps-ref-fix' into 'master'"
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5039
Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: karthik nayak <knayak@gitlab.com>
Co-authored-by: Pavlo Strokov <pstrokov@gitlab.com>
18 files changed, 71 insertions, 65 deletions
diff --git a/internal/gitaly/service/ref/branches.go b/internal/gitaly/service/ref/branches.go index c5a67ed24..686e29f3f 100644 --- a/internal/gitaly/service/ref/branches.go +++ b/internal/gitaly/service/ref/branches.go @@ -8,6 +8,8 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) func (s *server) FindBranch(ctx context.Context, req *gitalypb.FindBranchRequest) (*gitalypb.FindBranchResponse, error) { @@ -16,7 +18,7 @@ func (s *server) FindBranch(ctx context.Context, req *gitalypb.FindBranchRequest return nil, helper.ErrInvalidArgument(err) } if len(req.GetName()) == 0 { - return nil, helper.ErrInvalidArgumentf("Branch name cannot be empty") + return nil, status.Errorf(codes.InvalidArgument, "Branch name cannot be empty") } repo := s.localrepo(repository) @@ -36,7 +38,7 @@ func (s *server) FindBranch(ctx context.Context, req *gitalypb.FindBranchRequest branch, ok := branchName.Branch() if !ok { - return nil, helper.ErrInvalidArgumentf("reference is not a branch") + return nil, status.Errorf(codes.InvalidArgument, "reference is not a branch") } return &gitalypb.FindBranchResponse{ diff --git a/internal/gitaly/service/ref/delete_refs.go b/internal/gitaly/service/ref/delete_refs.go index f282fc0e1..198621184 100644 --- a/internal/gitaly/service/ref/delete_refs.go +++ b/internal/gitaly/service/ref/delete_refs.go @@ -15,11 +15,13 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/v15/internal/transaction/voting" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) func (s *server) DeleteRefs(ctx context.Context, in *gitalypb.DeleteRefsRequest) (*gitalypb.DeleteRefsResponse, error) { if err := validateDeleteRefRequest(in); err != nil { - return nil, helper.ErrInvalidArgument(err) + return nil, status.Errorf(codes.InvalidArgument, "DeleteRefs: %v", err) } repo := s.localrepo(in.GetRepository()) @@ -77,7 +79,7 @@ func (s *server) DeleteRefs(ctx context.Context, in *gitalypb.DeleteRefsRequest) } if _, err := voteHash.Write([]byte(ref.String() + "\n")); err != nil { - return nil, helper.ErrInternalf("could not update vote hash: %w", err) + return nil, helper.ErrInternalf("could not update vote hash: %v", err) } } @@ -112,7 +114,7 @@ func (s *server) DeleteRefs(ctx context.Context, in *gitalypb.DeleteRefsRequest) vote, err := voteHash.Vote() if err != nil { - return nil, helper.ErrInternalf("could not compute vote: %w", err) + return nil, helper.ErrInternalf("could not compute vote: %v", err) } // All deletes we're doing in this RPC are force deletions. Because we're required to filter @@ -185,22 +187,22 @@ func validateDeleteRefRequest(req *gitalypb.DeleteRefsRequest) error { } if len(req.ExceptWithPrefix) > 0 && len(req.Refs) > 0 { - return errors.New("ExceptWithPrefix and Refs are mutually exclusive") + return fmt.Errorf("ExceptWithPrefix and Refs are mutually exclusive") } if len(req.ExceptWithPrefix) == 0 && len(req.Refs) == 0 { // You can't delete all refs - return errors.New("empty ExceptWithPrefix and Refs") + return fmt.Errorf("empty ExceptWithPrefix and Refs") } for _, prefix := range req.ExceptWithPrefix { if len(prefix) == 0 { - return errors.New("empty prefix for exclusion") + return fmt.Errorf("empty prefix for exclusion") } } for _, ref := range req.Refs { if len(ref) == 0 { - return errors.New("empty ref") + return fmt.Errorf("empty ref") } } diff --git a/internal/gitaly/service/ref/delete_refs_test.go b/internal/gitaly/service/ref/delete_refs_test.go index 39a4d0c4f..03f252581 100644 --- a/internal/gitaly/service/ref/delete_refs_test.go +++ b/internal/gitaly/service/ref/delete_refs_test.go @@ -278,7 +278,7 @@ func TestDeleteRefs_validation(t *testing.T) { desc: "no repository provided", request: &gitalypb.DeleteRefsRequest{Repository: nil}, expectedErr: status.Error(codes.InvalidArgument, testhelper.GitalyOrPraefectMessage( - "empty Repository", + "DeleteRefs: empty Repository", "repo scoped: empty Repository", )), }, @@ -300,7 +300,7 @@ func TestDeleteRefs_validation(t *testing.T) { ExceptWithPrefix: [][]byte{[]byte("exclude-this")}, }, expectedErr: status.Error(codes.InvalidArgument, testhelper.GitalyOrPraefectMessage( - "empty Repository", + "DeleteRefs: empty Repository", "repo scoped: empty Repository", )), }, @@ -309,7 +309,7 @@ func TestDeleteRefs_validation(t *testing.T) { request: &gitalypb.DeleteRefsRequest{ Repository: repo, }, - expectedErr: status.Error(codes.InvalidArgument, "empty ExceptWithPrefix and Refs"), + expectedErr: status.Error(codes.InvalidArgument, "DeleteRefs: empty ExceptWithPrefix and Refs"), }, { desc: "prefixes with refs", @@ -318,7 +318,7 @@ func TestDeleteRefs_validation(t *testing.T) { ExceptWithPrefix: [][]byte{[]byte("exclude-this")}, Refs: [][]byte{[]byte("delete-this")}, }, - expectedErr: status.Error(codes.InvalidArgument, "ExceptWithPrefix and Refs are mutually exclusive"), + expectedErr: status.Error(codes.InvalidArgument, "DeleteRefs: ExceptWithPrefix and Refs are mutually exclusive"), }, { desc: "Empty prefix", @@ -326,7 +326,7 @@ func TestDeleteRefs_validation(t *testing.T) { Repository: repo, ExceptWithPrefix: [][]byte{[]byte("exclude-this"), {}}, }, - expectedErr: status.Error(codes.InvalidArgument, "empty prefix for exclusion"), + expectedErr: status.Error(codes.InvalidArgument, "DeleteRefs: empty prefix for exclusion"), }, { desc: "Empty ref", @@ -334,7 +334,7 @@ func TestDeleteRefs_validation(t *testing.T) { Repository: repo, Refs: [][]byte{[]byte("delete-this"), {}}, }, - expectedErr: status.Error(codes.InvalidArgument, "empty ref"), + expectedErr: status.Error(codes.InvalidArgument, "DeleteRefs: empty ref"), }, } diff --git a/internal/gitaly/service/ref/find_all_tags.go b/internal/gitaly/service/ref/find_all_tags.go index 9bfca9e87..5df28db8c 100644 --- a/internal/gitaly/service/ref/find_all_tags.go +++ b/internal/gitaly/service/ref/find_all_tags.go @@ -43,7 +43,7 @@ func (s *server) FindAllTags(in *gitalypb.FindAllTagsRequest, stream gitalypb.Re func (s *server) findAllTags(ctx context.Context, repo *localrepo.Repo, sortField string, stream gitalypb.RefService_FindAllTagsServer, opts *paginationOpts) error { objectReader, cancel, err := s.catfileCache.ObjectReader(ctx, repo) if err != nil { - return fmt.Errorf("creating object reader: %w", err) + return fmt.Errorf("error creating object reader: %v", err) } defer cancel() @@ -57,7 +57,7 @@ func (s *server) findAllTags(ctx context.Context, repo *localrepo.Repo, sortFiel catfileObjectsIter, err := gitpipe.CatfileObject(ctx, objectReader, forEachRefIter) if err != nil { - return fmt.Errorf("creating cat-file object iterator: %w", err) + return err } chunker := chunk.New(&tagSender{stream: stream}) @@ -172,7 +172,7 @@ func (s *server) validateFindAllTagsRequest(request *gitalypb.FindAllTagsRequest } if _, err := s.locator.GetRepoPath(repository); err != nil { - return fmt.Errorf("invalid git directory: %w", err) + return fmt.Errorf("invalid git directory: %v", err) } return nil diff --git a/internal/gitaly/service/ref/find_refs_by_oid.go b/internal/gitaly/service/ref/find_refs_by_oid.go index 52c0a4c95..b6284723b 100644 --- a/internal/gitaly/service/ref/find_refs_by_oid.go +++ b/internal/gitaly/service/ref/find_refs_by_oid.go @@ -43,7 +43,7 @@ func (s *server) FindRefsByOID(ctx context.Context, in *gitalypb.FindRefsByOIDRe if strings.Contains(err.Error(), "exit status 129") { return nil, helper.ErrInvalidArgument(err) } - return nil, helper.ErrInternal(err) + return nil, err } return &gitalypb.FindRefsByOIDResponse{ diff --git a/internal/gitaly/service/ref/find_tag.go b/internal/gitaly/service/ref/find_tag.go index 982f49920..ab340caea 100644 --- a/internal/gitaly/service/ref/find_tag.go +++ b/internal/gitaly/service/ref/find_tag.go @@ -48,7 +48,7 @@ func parseTagLine(ctx context.Context, objectReader catfile.ObjectReader, tagLin case "tag": tag, err := catfile.GetTag(ctx, objectReader, git.Revision(tagID), refName) if err != nil { - return nil, fmt.Errorf("getting annotated tag: %w", err) + return nil, fmt.Errorf("getting annotated tag: %v", err) } catfile.TrimTagMessage(tag) @@ -56,7 +56,7 @@ func parseTagLine(ctx context.Context, objectReader catfile.ObjectReader, tagLin case "commit": commit, err := catfile.GetCommit(ctx, objectReader, git.Revision(tagID)) if err != nil { - return nil, fmt.Errorf("getting commit catfile: %w", err) + return nil, fmt.Errorf("getting commit catfile: %v", err) } tag.TargetCommit = commit return tag, nil @@ -77,12 +77,12 @@ func (s *server) findTag(ctx context.Context, repo git.RepositoryExecutor, tagNa git.WithRefTxHook(repo), ) if err != nil { - return nil, fmt.Errorf("for-each-ref error: %w", err) + return nil, fmt.Errorf("for-each-ref error: %v", err) } objectReader, cancel, err := s.catfileCache.ObjectReader(ctx, repo) if err != nil { - return nil, fmt.Errorf("creating object reader: %w", err) + return nil, err } defer cancel() @@ -92,7 +92,7 @@ func (s *server) findTag(ctx context.Context, repo git.RepositoryExecutor, tagNa if scanner.Scan() { tag, err = parseTagLine(ctx, objectReader, scanner.Text()) if err != nil { - return nil, fmt.Errorf("parsing tag: %w", err) + return nil, err } } else { detailedErr, err := helper.ErrWithDetails( @@ -126,7 +126,7 @@ func (s *server) validateFindTagRequest(in *gitalypb.FindTagRequest) error { } if _, err := s.locator.GetRepoPath(repository); err != nil { - return fmt.Errorf("invalid git directory: %w", err) + return fmt.Errorf("invalid git directory: %v", err) } if in.GetTagName() == nil { diff --git a/internal/gitaly/service/ref/list_refs.go b/internal/gitaly/service/ref/list_refs.go index 60d347c80..24037f287 100644 --- a/internal/gitaly/service/ref/list_refs.go +++ b/internal/gitaly/service/ref/list_refs.go @@ -25,7 +25,7 @@ func (s *server) ListRefs(in *gitalypb.ListRefsRequest, stream gitalypb.RefServi var err error headOID, err = repo.ResolveRevision(ctx, git.Revision("HEAD")) if err != nil && !errors.Is(err, git.ErrReferenceNotFound) { - return helper.ErrInternalf("resolving HEAD: %w", err) + return helper.ErrInternal(err) } } @@ -44,7 +44,7 @@ func (s *server) ListRefs(in *gitalypb.ListRefsRequest, stream gitalypb.RefServi git.ValueFlag{Name: "--sort", Value: sorting}, } - return helper.ErrInternal(s.findRefs(ctx, writer, repo, patterns, opts)) + return s.findRefs(ctx, writer, repo, patterns, opts) } func validateListRefsRequest(in *gitalypb.ListRefsRequest) error { diff --git a/internal/gitaly/service/ref/refexists.go b/internal/gitaly/service/ref/refexists.go index 38a2477b5..60a006b84 100644 --- a/internal/gitaly/service/ref/refexists.go +++ b/internal/gitaly/service/ref/refexists.go @@ -2,6 +2,7 @@ package ref import ( "context" + "fmt" "strings" "gitlab.com/gitlab-org/gitaly/v15/internal/command" @@ -20,7 +21,7 @@ func (s *server) RefExists(ctx context.Context, in *gitalypb.RefExistsRequest) ( ref := string(in.Ref) if !isValidRefName(ref) { - return nil, helper.ErrInvalidArgumentf("invalid refname") + return nil, helper.ErrInvalidArgument(fmt.Errorf("invalid refname")) } exists, err := s.refExists(ctx, in.Repository, ref) diff --git a/internal/gitaly/service/ref/refnames.go b/internal/gitaly/service/ref/refnames.go index 34c86984d..1b9e95880 100644 --- a/internal/gitaly/service/ref/refnames.go +++ b/internal/gitaly/service/ref/refnames.go @@ -21,7 +21,7 @@ func (s *server) FindAllBranchNames(in *gitalypb.FindAllBranchNamesRequest, stre chunker := chunk.New(&findAllBranchNamesSender{stream: stream}) - return helper.ErrInternal(s.listRefNames(stream.Context(), chunker, "refs/heads", in.Repository, nil)) + return s.listRefNames(stream.Context(), chunker, "refs/heads", in.Repository, nil) } type findAllBranchNamesSender struct { @@ -45,7 +45,8 @@ func (s *server) FindAllTagNames(in *gitalypb.FindAllTagNamesRequest, stream git } chunker := chunk.New(&findAllTagNamesSender{stream: stream}) - return helper.ErrInternal(s.listRefNames(stream.Context(), chunker, "refs/tags", in.Repository, nil)) + + return s.listRefNames(stream.Context(), chunker, "refs/tags", in.Repository, nil) } type findAllTagNamesSender struct { diff --git a/internal/gitaly/service/ref/refnames_containing.go b/internal/gitaly/service/ref/refnames_containing.go index 80a888316..92e692095 100644 --- a/internal/gitaly/service/ref/refnames_containing.go +++ b/internal/gitaly/service/ref/refnames_containing.go @@ -25,7 +25,11 @@ func (s *server) ListBranchNamesContainingCommit(in *gitalypb.ListBranchNamesCon chunker := chunk.New(&branchNamesContainingCommitSender{stream: stream}) ctx := stream.Context() - return helper.ErrInternal(s.listRefNames(ctx, chunker, "refs/heads", in.Repository, containingArgs(in))) + if err := s.listRefNames(ctx, chunker, "refs/heads", in.Repository, containingArgs(in)); err != nil { + return helper.ErrInternal(err) + } + + return nil } type containingRequest interface { @@ -67,7 +71,11 @@ func (s *server) ListTagNamesContainingCommit(in *gitalypb.ListTagNamesContainin chunker := chunk.New(&tagNamesContainingCommitSender{stream: stream}) ctx := stream.Context() - return helper.ErrInternal(s.listRefNames(ctx, chunker, "refs/tags", in.Repository, containingArgs(in))) + if err := s.listRefNames(ctx, chunker, "refs/tags", in.Repository, containingArgs(in)); err != nil { + return helper.ErrInternal(err) + } + + return nil } type tagNamesContainingCommitSender struct { diff --git a/internal/gitaly/service/ref/refs.go b/internal/gitaly/service/ref/refs.go index 711ecdd14..2827764ac 100644 --- a/internal/gitaly/service/ref/refs.go +++ b/internal/gitaly/service/ref/refs.go @@ -115,7 +115,7 @@ func (s *server) findLocalBranches(in *gitalypb.FindLocalBranchesRequest, stream objectReader, cancel, err := s.catfileCache.ObjectReader(ctx, repo) if err != nil { - return fmt.Errorf("creating object reader: %w", err) + return err } defer cancel() @@ -127,10 +127,7 @@ func (s *server) findLocalBranches(in *gitalypb.FindLocalBranchesRequest, stream git.Flag{Name: "--sort=" + parseSortKey(in.GetSortBy())}, } - if err := s.findRefs(ctx, writer, repo, []string{"refs/heads"}, opts); err != nil { - return fmt.Errorf("finding refs: %w", err) - } - return nil + return s.findRefs(ctx, writer, repo, []string{"refs/heads"}, opts) } func (s *server) FindAllBranches(in *gitalypb.FindAllBranchesRequest, stream gitalypb.RefService_FindAllBranchesServer) error { @@ -157,7 +154,7 @@ func (s *server) findAllBranches(in *gitalypb.FindAllBranchesRequest, stream git if in.MergedOnly { defaultBranch, err := repo.GetDefaultBranch(stream.Context()) if err != nil { - return fmt.Errorf("default branch name: %w", err) + return err } args = append(args, git.Flag{Name: fmt.Sprintf("--merged=%s", defaultBranch.String())}) @@ -174,7 +171,7 @@ func (s *server) findAllBranches(in *gitalypb.FindAllBranchesRequest, stream git ctx := stream.Context() objectReader, cancel, err := s.catfileCache.ObjectReader(ctx, repo) if err != nil { - return fmt.Errorf("creating object reader: %w", err) + return err } defer cancel() @@ -183,10 +180,7 @@ func (s *server) findAllBranches(in *gitalypb.FindAllBranchesRequest, stream git writer := newFindAllBranchesWriter(stream, objectReader) - if err = s.findRefs(ctx, writer, repo, patterns, opts); err != nil { - return fmt.Errorf("finding refs: %w", err) - } - return nil + return s.findRefs(ctx, writer, repo, patterns, opts) } func buildPaginationOpts(ctx context.Context, p *gitalypb.PaginationParameter) *paginationOpts { diff --git a/internal/gitaly/service/ref/refs_test.go b/internal/gitaly/service/ref/refs_test.go index b5acb08f5..4a6aa4fd0 100644 --- a/internal/gitaly/service/ref/refs_test.go +++ b/internal/gitaly/service/ref/refs_test.go @@ -473,7 +473,7 @@ func TestFindLocalBranchesPaginationWithIncorrectToken(t *testing.T) { _, err = c.Recv() require.NotEqual(t, err, io.EOF) - testhelper.RequireGrpcError(t, helper.ErrInternalf("finding refs: could not find page token"), err) + testhelper.RequireGrpcError(t, helper.ErrInternalf("could not find page token"), err) } // Test that `s` contains the elements in `relativeOrder` in that order @@ -571,7 +571,7 @@ func TestFindLocalBranches_validate(t *testing.T) { desc: "repository doesn't exist on disk", repo: &gitalypb.Repository{StorageName: cfg.Storages[0].Name, RelativePath: "made/up/path"}, expectedErr: status.Error(codes.NotFound, testhelper.GitalyOrPraefectMessage( - `creating object reader: GetRepoPath: not a git repository: "`+cfg.Storages[0].Path+`/made/up/path"`, + `GetRepoPath: not a git repository: "`+cfg.Storages[0].Path+`/made/up/path"`, `accessor call: route repository accessor: consistent storages: repository "default"/"made/up/path" not found`, )), }, @@ -579,7 +579,7 @@ func TestFindLocalBranches_validate(t *testing.T) { desc: "unknown storage", repo: &gitalypb.Repository{StorageName: "invalid", RelativePath: repo.GetRelativePath()}, expectedErr: status.Error(codes.InvalidArgument, testhelper.GitalyOrPraefectMessage( - `creating object reader: GetStorageByName: no such storage: "invalid"`, + `GetStorageByName: no such storage: "invalid"`, "repo scoped: invalid Repository", )), }, @@ -754,7 +754,7 @@ func TestInvalidFindAllBranchesRequest(t *testing.T) { }, }, expectedErr: helper.ErrInvalidArgumentf(testhelper.GitalyOrPraefectMessage( - "creating object reader: GetStorageByName: no such storage: \"fake\"", + "GetStorageByName: no such storage: \"fake\"", "repo scoped: invalid Repository", )), }, diff --git a/internal/gitaly/service/ref/remote_branches.go b/internal/gitaly/service/ref/remote_branches.go index 53664f6b1..972661d65 100644 --- a/internal/gitaly/service/ref/remote_branches.go +++ b/internal/gitaly/service/ref/remote_branches.go @@ -1,7 +1,6 @@ package ref import ( - "errors" "fmt" "strings" @@ -35,7 +34,7 @@ func (s *server) findAllRemoteBranches(req *gitalypb.FindAllRemoteBranchesReques ctx := stream.Context() objectReader, cancel, err := s.catfileCache.ObjectReader(ctx, repo) if err != nil { - return fmt.Errorf("creating object reader: %w", err) + return err } defer cancel() @@ -43,10 +42,7 @@ func (s *server) findAllRemoteBranches(req *gitalypb.FindAllRemoteBranchesReques opts.cmdArgs = args writer := newFindAllRemoteBranchesWriter(stream, objectReader) - if err = s.findRefs(ctx, writer, repo, patterns, opts); err != nil { - return fmt.Errorf("finding refs: %w", err) - } - return nil + return s.findRefs(ctx, writer, repo, patterns, opts) } func validateFindAllRemoteBranchesRequest(req *gitalypb.FindAllRemoteBranchesRequest) error { @@ -55,7 +51,7 @@ func validateFindAllRemoteBranchesRequest(req *gitalypb.FindAllRemoteBranchesReq } if len(req.GetRemoteName()) == 0 { - return errors.New("empty RemoteName") + return fmt.Errorf("empty RemoteName") } return nil diff --git a/internal/gitaly/service/ref/remote_branches_test.go b/internal/gitaly/service/ref/remote_branches_test.go index 543264189..be2bb44cb 100644 --- a/internal/gitaly/service/ref/remote_branches_test.go +++ b/internal/gitaly/service/ref/remote_branches_test.go @@ -94,7 +94,7 @@ func TestInvalidFindAllRemoteBranchesRequest(t *testing.T) { RemoteName: "stub", }, expectedErr: status.Error(codes.InvalidArgument, testhelper.GitalyOrPraefectMessage( - `creating object reader: GetStorageByName: no such storage: "fake"`, + `GetStorageByName: no such storage: "fake"`, "repo scoped: invalid Repository", )), }, diff --git a/internal/gitaly/service/ref/tag_messages.go b/internal/gitaly/service/ref/tag_messages.go index 9c326155d..ec223bd7e 100644 --- a/internal/gitaly/service/ref/tag_messages.go +++ b/internal/gitaly/service/ref/tag_messages.go @@ -11,11 +11,13 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/v15/streamio" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) func (s *server) GetTagMessages(request *gitalypb.GetTagMessagesRequest, stream gitalypb.RefService_GetTagMessagesServer) error { if err := validateGetTagMessagesRequest(request); err != nil { - return helper.ErrInvalidArgument(err) + return status.Errorf(codes.InvalidArgument, "GetTagMessages: %v", err) } if err := s.getAndStreamTagMessages(request, stream); err != nil { return helper.ErrInternal(err) @@ -34,14 +36,14 @@ func (s *server) getAndStreamTagMessages(request *gitalypb.GetTagMessagesRequest objectReader, cancel, err := s.catfileCache.ObjectReader(ctx, repo) if err != nil { - return fmt.Errorf("creating object reader: %w", err) + return err } defer cancel() for _, tagID := range request.GetTagIds() { tag, err := catfile.GetTag(ctx, objectReader, git.Revision(tagID), "") if err != nil { - return fmt.Errorf("getting tag: %w", err) + return fmt.Errorf("failed to get tag: %v", err) } if err := stream.Send(&gitalypb.GetTagMessagesResponse{TagId: tagID}); err != nil { @@ -54,7 +56,7 @@ func (s *server) getAndStreamTagMessages(request *gitalypb.GetTagMessagesRequest msgReader := bytes.NewReader(tag.Message) if _, err = io.Copy(sw, msgReader); err != nil { - return fmt.Errorf("failed to send response: %w", err) + return fmt.Errorf("failed to send response: %v", err) } } return nil diff --git a/internal/gitaly/service/ref/tag_messages_test.go b/internal/gitaly/service/ref/tag_messages_test.go index dfd811853..fa227973b 100644 --- a/internal/gitaly/service/ref/tag_messages_test.go +++ b/internal/gitaly/service/ref/tag_messages_test.go @@ -68,7 +68,7 @@ func TestFailedGetTagMessagesRequest(t *testing.T) { TagIds: []string{"5937ac0a7beb003549fc5fd26fc247adbce4a52e"}, }, expectedErr: status.Error(codes.InvalidArgument, testhelper.GitalyOrPraefectMessage( - "empty Repository", + "GetTagMessages: empty Repository", "repo scoped: empty Repository", )), }, diff --git a/internal/gitaly/service/ref/tag_signatures.go b/internal/gitaly/service/ref/tag_signatures.go index bd0f93217..1d2ca43a4 100644 --- a/internal/gitaly/service/ref/tag_signatures.go +++ b/internal/gitaly/service/ref/tag_signatures.go @@ -63,7 +63,7 @@ func (s *server) GetTagSignatures(req *gitalypb.GetTagSignaturesRequest, stream catfileObjectIter, err := gitpipe.CatfileObject(ctx, objectReader, revlistIter) if err != nil { - return helper.ErrInternalf("creating cat-file object iterator: %w", err) + return err } for catfileObjectIter.Next() { @@ -71,7 +71,7 @@ func (s *server) GetTagSignatures(req *gitalypb.GetTagSignaturesRequest, stream raw, err := io.ReadAll(tag) if err != nil { - return helper.ErrInternalf("reading tag: %w", err) + return helper.ErrInternal(err) } signatureKey, tagText := catfile.ExtractTagSignature(raw) @@ -86,11 +86,11 @@ func (s *server) GetTagSignatures(req *gitalypb.GetTagSignaturesRequest, stream } if err := catfileObjectIter.Err(); err != nil { - return helper.ErrInternalf("cat-file iterator stop: %w", err) + return helper.ErrInternal(err) } if err := chunker.Flush(); err != nil { - return helper.ErrInternalf("flushing chunker: %w", err) + return helper.ErrInternal(err) } return nil diff --git a/internal/gitaly/service/ref/tag_signatures_test.go b/internal/gitaly/service/ref/tag_signatures_test.go index fee224281..f7aa92481 100644 --- a/internal/gitaly/service/ref/tag_signatures_test.go +++ b/internal/gitaly/service/ref/tag_signatures_test.go @@ -60,7 +60,7 @@ func TestGetTagSignatures(t *testing.T) { revisions: []string{ "b10ff336f3fbfb131431c4959915cdfd1b49c635", }, - expectedErr: status.Error(codes.Internal, "cat-file iterator stop: rev-list pipeline command: exit status 128, stderr: \"fatal: bad object b10ff336f3fbfb131431c4959915cdfd1b49c635\\n\""), + expectedErr: status.Error(codes.Internal, "rev-list pipeline command: exit status 128, stderr: \"fatal: bad object b10ff336f3fbfb131431c4959915cdfd1b49c635\\n\""), }, { desc: "commit id", |