diff options
46 files changed, 191 insertions, 225 deletions
diff --git a/internal/gitaly/service/repository/apply_gitattributes.go b/internal/gitaly/service/repository/apply_gitattributes.go index 1a860284e..4f23b63b9 100644 --- a/internal/gitaly/service/repository/apply_gitattributes.go +++ b/internal/gitaly/service/repository/apply_gitattributes.go @@ -33,12 +33,12 @@ func (s *server) applyGitattributes(ctx context.Context, repo *localrepo.Repo, o return helper.ErrInvalidArgumentf("revision does not exist") } - return err + return helper.ErrInternalf("resolve revision: %w", err) } blobObj, err := objectReader.Object(ctx, git.Revision(fmt.Sprintf("%s:.gitattributes", revision))) if err != nil && !catfile.IsNotFound(err) { - return err + return helper.ErrInternalf("read object: %w", err) } // Create /info folder if it doesn't exist @@ -51,7 +51,7 @@ func (s *server) applyGitattributes(ctx context.Context, repo *localrepo.Repo, o FileWriterConfig: safe.FileWriterConfig{FileMode: attributesFileMode}, }) if err != nil { - return fmt.Errorf("creating gitattributes lock: %w", err) + return helper.ErrInternalf("creating gitattributes lock: %w", err) } defer func() { if err := locker.Close(); err != nil { @@ -60,21 +60,21 @@ func (s *server) applyGitattributes(ctx context.Context, repo *localrepo.Repo, o }() if err := locker.Lock(); err != nil { - return fmt.Errorf("locking gitattributes: %w", err) + return helper.ErrInternalf("locking gitattributes: %w", err) } // We use the zero OID as placeholder to vote on removal of the // gitattributes file. if err := s.vote(ctx, git.ObjectHashSHA1.ZeroOID, voting.Prepared); err != nil { - return fmt.Errorf("preimage vote: %w", err) + return helper.ErrInternalf("preimage vote: %w", err) } if err := os.Remove(attributesPath); err != nil && !os.IsNotExist(err) { - return err + return helper.ErrInternalf("remove attributes file: %w", err) } if err := s.vote(ctx, git.ObjectHashSHA1.ZeroOID, voting.Committed); err != nil { - return fmt.Errorf("postimage vote: %w", err) + return helper.ErrInternalf("postimage vote: %w", err) } return nil @@ -84,7 +84,7 @@ func (s *server) applyGitattributes(ctx context.Context, repo *localrepo.Repo, o FileWriterConfig: safe.FileWriterConfig{FileMode: attributesFileMode}, }) if err != nil { - return fmt.Errorf("creating gitattributes writer: %w", err) + return helper.ErrInternalf("creating gitattributes writer: %w", err) } defer func() { if err := writer.Close(); err != nil && returnedErr == nil { @@ -95,11 +95,11 @@ func (s *server) applyGitattributes(ctx context.Context, repo *localrepo.Repo, o }() if _, err := io.Copy(writer, blobObj); err != nil { - return err + return helper.ErrInternalf("writing data: %w", err) } if err := transaction.CommitLockedFile(ctx, s.txManager, writer); err != nil { - return fmt.Errorf("committing gitattributes: %w", err) + return helper.ErrInternalf("committing gitattributes: %w", err) } return nil @@ -139,12 +139,12 @@ func (s *server) ApplyGitattributes(ctx context.Context, in *gitalypb.ApplyGitat } if err := git.ValidateRevision(in.GetRevision()); err != nil { - return nil, helper.ErrInvalidArgumentf("revision: %v", err) + return nil, helper.ErrInvalidArgumentf("revision: %w", err) } objectReader, cancel, err := s.catfileCache.ObjectReader(ctx, repo) if err != nil { - return nil, err + return nil, helper.ErrInternalf("creating object reader: %w", err) } defer cancel() diff --git a/internal/gitaly/service/repository/apply_gitattributes_test.go b/internal/gitaly/service/repository/apply_gitattributes_test.go index 9863a2e19..a6f35cf37 100644 --- a/internal/gitaly/service/repository/apply_gitattributes_test.go +++ b/internal/gitaly/service/repository/apply_gitattributes_test.go @@ -139,7 +139,7 @@ func TestApplyGitattributesWithTransaction(t *testing.T) { }, shouldExist: false, expectedErr: func() error { - return status.Error(codes.Unknown, "committing gitattributes: voting on locked file: preimage vote: transaction was aborted") + return status.Error(codes.Internal, "committing gitattributes: voting on locked file: preimage vote: transaction was aborted") }(), expectedVotes: 1, }, @@ -151,7 +151,7 @@ func TestApplyGitattributesWithTransaction(t *testing.T) { }, shouldExist: false, expectedErr: func() error { - return status.Error(codes.Unknown, "committing gitattributes: voting on locked file: preimage vote: rpc error: code = Unknown desc = foobar") + return status.Error(codes.Internal, "committing gitattributes: voting on locked file: preimage vote: rpc error: code = Unknown desc = foobar") }(), expectedVotes: 1, }, diff --git a/internal/gitaly/service/repository/archive.go b/internal/gitaly/service/repository/archive.go index be6ffdd36..a919372b1 100644 --- a/internal/gitaly/service/repository/archive.go +++ b/internal/gitaly/service/repository/archive.go @@ -85,7 +85,7 @@ func (s *server) GetArchive(in *gitalypb.GetArchiveRequest, stream gitalypb.Repo ctxlogrus.Extract(ctx).WithField("request_hash", requestHash(in)).Info("request details") - return s.handleArchive(ctx, archiveParams{ + return helper.ErrInternal(s.handleArchive(ctx, archiveParams{ writer: writer, in: in, compressArgs: compressArgs, @@ -93,7 +93,7 @@ func (s *server) GetArchive(in *gitalypb.GetArchiveRequest, stream gitalypb.Repo archivePath: path, exclude: exclude, loggingDir: s.loggingCfg.Dir, - }) + })) } func parseArchiveFormat(format gitalypb.GetArchiveRequest_Format) ([]string, string) { @@ -113,7 +113,7 @@ func parseArchiveFormat(format gitalypb.GetArchiveRequest_Format) ([]string, str func validateGetArchiveRequest(in *gitalypb.GetArchiveRequest, format string) error { if err := git.ValidateRevision([]byte(in.GetCommitId())); err != nil { - return helper.ErrInvalidArgumentf("invalid commitId: %v", err) + return helper.ErrInvalidArgumentf("invalid commitId: %w", err) } if len(format) == 0 { @@ -132,20 +132,20 @@ func (s *server) validateGetArchivePrecondition( ) error { objectReader, cancel, err := s.catfileCache.ObjectReader(ctx, repo) if err != nil { - return err + return helper.ErrInternalf("creating object reader: %w", err) } defer cancel() objectInfoReader, cancel, err := s.catfileCache.ObjectInfoReader(ctx, repo) if err != nil { - return err + return helper.ErrInternalf("creating object info reader: %w", err) } defer cancel() f := catfile.NewTreeEntryFinder(objectReader, objectInfoReader) if path != "." { if ok, err := findGetArchivePath(ctx, f, commitID, path); err != nil { - return err + return helper.ErrInternalf("find archive: %w", err) } else if !ok { return helper.ErrFailedPreconditionf("path doesn't exist") } @@ -153,7 +153,7 @@ func (s *server) validateGetArchivePrecondition( for i, exclude := range exclude { if ok, err := findGetArchivePath(ctx, f, commitID, exclude); err != nil { - return err + return helper.ErrInternalf("find archive exclude: %w", err) } else if !ok { return helper.ErrFailedPreconditionf("exclude[%d] doesn't exist", i) } diff --git a/internal/gitaly/service/repository/backup_custom_hooks.go b/internal/gitaly/service/repository/backup_custom_hooks.go index a9b49b8f0..55b5f6df2 100644 --- a/internal/gitaly/service/repository/backup_custom_hooks.go +++ b/internal/gitaly/service/repository/backup_custom_hooks.go @@ -9,8 +9,6 @@ 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" ) const customHooksDir = "custom_hooks" @@ -21,7 +19,7 @@ func (s *server) BackupCustomHooks(in *gitalypb.BackupCustomHooksRequest, stream } repoPath, err := s.locator.GetPath(in.Repository) if err != nil { - return status.Errorf(codes.Internal, "BackupCustomHooks: getting repo path failed %v", err) + return helper.ErrInternalf("getting repo path failed %w", err) } writer := streamio.NewWriter(func(p []byte) error { @@ -36,11 +34,11 @@ func (s *server) BackupCustomHooks(in *gitalypb.BackupCustomHooksRequest, stream tar := []string{"tar", "-c", "-f", "-", "-C", repoPath, customHooksDir} cmd, err := command.New(ctx, tar, command.WithStdout(writer)) if err != nil { - return status.Errorf(codes.Internal, "%v", err) + return helper.ErrInternal(err) } if err := cmd.Wait(); err != nil { - return status.Errorf(codes.Internal, "%v", err) + return helper.ErrInternal(err) } return nil diff --git a/internal/gitaly/service/repository/calculate_checksum.go b/internal/gitaly/service/repository/calculate_checksum.go index 84eb6f627..78b7fac43 100644 --- a/internal/gitaly/service/repository/calculate_checksum.go +++ b/internal/gitaly/service/repository/calculate_checksum.go @@ -11,8 +11,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "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) CalculateChecksum(ctx context.Context, in *gitalypb.CalculateChecksumRequest) (*gitalypb.CalculateChecksumResponse, error) { @@ -27,11 +25,7 @@ func (s *server) CalculateChecksum(ctx context.Context, in *gitalypb.CalculateCh cmd, err := s.gitCmdFactory.New(ctx, repo, git.SubCmd{Name: "show-ref", Flags: []git.Option{git.Flag{Name: "--head"}}}) if err != nil { - if _, ok := status.FromError(err); ok { - return nil, err - } - - return nil, status.Errorf(codes.Internal, "CalculateChecksum: gitCommand: %v", err) + return nil, helper.ErrInternalf("gitCommand: %w", err) } var checksum git.Checksum @@ -42,7 +36,7 @@ func (s *server) CalculateChecksum(ctx context.Context, in *gitalypb.CalculateCh } if err := scanner.Err(); err != nil { - return nil, status.Errorf(codes.Internal, err.Error()) + return nil, helper.ErrInternal(err) } if err := cmd.Wait(); checksum.IsZero() || err != nil { @@ -50,7 +44,7 @@ func (s *server) CalculateChecksum(ctx context.Context, in *gitalypb.CalculateCh return &gitalypb.CalculateChecksumResponse{Checksum: git.ObjectHashSHA1.ZeroOID.String()}, nil } - return nil, status.Errorf(codes.DataLoss, "CalculateChecksum: not a git repository '%s'", repoPath) + return nil, helper.ErrDataLossf("not a git repository '%s'", repoPath) } return &gitalypb.CalculateChecksumResponse{Checksum: hex.EncodeToString(checksum.Bytes())}, nil diff --git a/internal/gitaly/service/repository/config.go b/internal/gitaly/service/repository/config.go index d99cc36dc..1a0ba2464 100644 --- a/internal/gitaly/service/repository/config.go +++ b/internal/gitaly/service/repository/config.go @@ -9,8 +9,6 @@ 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" ) // GetConfig reads the repository's gitconfig file and returns its contents. @@ -31,9 +29,9 @@ func (s *server) GetConfig( gitconfig, err := os.Open(configPath) if err != nil { if os.IsNotExist(err) { - return status.Errorf(codes.NotFound, "opening gitconfig: %v", err) + return helper.ErrNotFoundf("opening gitconfig: %w", err) } - return helper.ErrInternalf("opening gitconfig: %v", err) + return helper.ErrInternalf("opening gitconfig: %w", err) } writer := streamio.NewWriter(func(p []byte) error { @@ -43,7 +41,7 @@ func (s *server) GetConfig( }) if _, err := io.Copy(writer, gitconfig); err != nil { - return helper.ErrInternalf("sending config: %v", err) + return helper.ErrInternalf("sending config: %w", err) } return nil diff --git a/internal/gitaly/service/repository/create_bundle.go b/internal/gitaly/service/repository/create_bundle.go index 3acbcca16..b7e3881ef 100644 --- a/internal/gitaly/service/repository/create_bundle.go +++ b/internal/gitaly/service/repository/create_bundle.go @@ -8,14 +8,12 @@ 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) CreateBundle(req *gitalypb.CreateBundleRequest, stream gitalypb.RepositoryService_CreateBundleServer) error { repo := req.GetRepository() if repo == nil { - return helper.ErrInvalidArgumentf("CreateBundle: %w", gitalyerrors.ErrEmptyRepository) + return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) } ctx := stream.Context() @@ -30,7 +28,7 @@ func (s *server) CreateBundle(req *gitalypb.CreateBundleRequest, stream gitalypb Flags: []git.Option{git.OutputToStdout, git.Flag{Name: "--all"}}, }) if err != nil { - return status.Errorf(codes.Internal, "CreateBundle: cmd start failed: %v", err) + return helper.ErrInternalf("cmd start failed: %w", err) } writer := streamio.NewWriter(func(p []byte) error { @@ -39,11 +37,11 @@ func (s *server) CreateBundle(req *gitalypb.CreateBundleRequest, stream gitalypb _, err = io.Copy(writer, cmd) if err != nil { - return status.Errorf(codes.Internal, "CreateBundle: stream writer failed: %v", err) + return helper.ErrInternalf("stream writer failed: %w", err) } if err := cmd.Wait(); err != nil { - return status.Errorf(codes.Internal, "CreateBundle: cmd wait failed: %v", err) + return helper.ErrInternalf("cmd wait failed: %w", err) } return nil diff --git a/internal/gitaly/service/repository/create_bundle_from_ref_list.go b/internal/gitaly/service/repository/create_bundle_from_ref_list.go index 428ea6749..4b4859a56 100644 --- a/internal/gitaly/service/repository/create_bundle_from_ref_list.go +++ b/internal/gitaly/service/repository/create_bundle_from_ref_list.go @@ -10,8 +10,6 @@ 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) CreateBundleFromRefList(stream gitalypb.RepositoryService_CreateBundleFromRefListServer) error { @@ -63,7 +61,7 @@ func (s *server) CreateBundleFromRefList(stream gitalypb.RepositoryService_Creat git.WithStderr(&stderr), ) if err != nil { - return status.Errorf(codes.Internal, "cmd start failed: %v", err) + return helper.ErrInternalf("cmd start failed: %w", err) } writer := streamio.NewWriter(func(p []byte) error { @@ -72,14 +70,14 @@ func (s *server) CreateBundleFromRefList(stream gitalypb.RepositoryService_Creat _, err = io.Copy(writer, cmd) if err != nil { - return status.Errorf(codes.Internal, "stream writer failed: %v", err) + return helper.ErrInternalf("stream writer failed: %w", err) } err = cmd.Wait() if isExitWithCode(err, 128) && bytes.HasPrefix(stderr.Bytes(), []byte("fatal: Refusing to create empty bundle.")) { - return status.Errorf(codes.FailedPrecondition, "cmd wait failed: refusing to create empty bundle") + return helper.ErrFailedPreconditionf("cmd wait failed: refusing to create empty bundle") } else if err != nil { - return status.Errorf(codes.Internal, "cmd wait failed: %v, stderr: %q", err, stderr.String()) + return helper.ErrInternalf("cmd wait failed: %w, stderr: %q", err, stderr.String()) } return nil diff --git a/internal/gitaly/service/repository/create_bundle_test.go b/internal/gitaly/service/repository/create_bundle_test.go index f8f2202a4..3afc3c19e 100644 --- a/internal/gitaly/service/repository/create_bundle_test.go +++ b/internal/gitaly/service/repository/create_bundle_test.go @@ -76,7 +76,7 @@ func TestFailedCreateBundleRequestDueToValidations(t *testing.T) { desc: "empty repository", request: &gitalypb.CreateBundleRequest{}, expErr: status.Error(codes.InvalidArgument, testhelper.GitalyOrPraefect( - "CreateBundle: empty Repository", + "empty Repository", "repo scoped: empty Repository", )), }, diff --git a/internal/gitaly/service/repository/create_fork.go b/internal/gitaly/service/repository/create_fork.go index 36265ed84..be14cbfc4 100644 --- a/internal/gitaly/service/repository/create_fork.go +++ b/internal/gitaly/service/repository/create_fork.go @@ -10,8 +10,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git" "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) CreateFork(ctx context.Context, req *gitalypb.CreateForkRequest) (*gitalypb.CreateForkResponse, error) { @@ -19,10 +17,10 @@ func (s *server) CreateFork(ctx context.Context, req *gitalypb.CreateForkRequest sourceRepository := req.SourceRepository if sourceRepository == nil { - return nil, status.Errorf(codes.InvalidArgument, "CreateFork: empty SourceRepository") + return nil, helper.ErrInvalidArgumentf("empty SourceRepository") } if targetRepository == nil { - return nil, helper.ErrInvalidArgumentf("CreateFork: %w", gitalyerrors.ErrEmptyRepository) + return nil, helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) } if err := s.createRepository(ctx, targetRepository, func(repo *gitalypb.Repository) error { diff --git a/internal/gitaly/service/repository/create_fork_test.go b/internal/gitaly/service/repository/create_fork_test.go index f6614f8d3..80fb76af9 100644 --- a/internal/gitaly/service/repository/create_fork_test.go +++ b/internal/gitaly/service/repository/create_fork_test.go @@ -295,7 +295,7 @@ func TestCreateFork_validate(t *testing.T) { desc: "repository not provided", req: &gitalypb.CreateForkRequest{Repository: nil, SourceRepository: repo}, expErr: status.Error(codes.InvalidArgument, testhelper.GitalyOrPraefect( - "CreateFork: empty Repository", + "empty Repository", "repo scoped: empty Repository", )), }, @@ -306,7 +306,7 @@ func TestCreateFork_validate(t *testing.T) { if testhelper.IsPraefectEnabled() { return status.Error(codes.AlreadyExists, "route repository creation: reserve repository id: repository already exists") } - return status.Error(codes.InvalidArgument, "CreateFork: empty SourceRepository") + return status.Error(codes.InvalidArgument, "empty SourceRepository") }(), }, } { diff --git a/internal/gitaly/service/repository/create_repository_from_bundle.go b/internal/gitaly/service/repository/create_repository_from_bundle.go index d5b5f3797..d7963ec93 100644 --- a/internal/gitaly/service/repository/create_repository_from_bundle.go +++ b/internal/gitaly/service/repository/create_repository_from_bundle.go @@ -14,19 +14,17 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/tempdir" "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) CreateRepositoryFromBundle(stream gitalypb.RepositoryService_CreateRepositoryFromBundleServer) error { firstRequest, err := stream.Recv() if err != nil { - return status.Errorf(codes.Internal, "CreateRepositoryFromBundle: first request failed: %v", err) + return helper.ErrInternalf("first request failed: %w", err) } repo := firstRequest.GetRepository() if repo == nil { - return helper.ErrInvalidArgumentf("CreateRepositoryFromBundle: %w", gitalyerrors.ErrEmptyRepository) + return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) } ctx := stream.Context() diff --git a/internal/gitaly/service/repository/create_repository_from_bundle_test.go b/internal/gitaly/service/repository/create_repository_from_bundle_test.go index af09e5924..406467928 100644 --- a/internal/gitaly/service/repository/create_repository_from_bundle_test.go +++ b/internal/gitaly/service/repository/create_repository_from_bundle_test.go @@ -253,7 +253,7 @@ func TestCreateRepositoryFromBundle_invalidArgument(t *testing.T) { require.NoError(t, stream.Send(&gitalypb.CreateRepositoryFromBundleRequest{})) _, err = stream.CloseAndRecv() - msg := testhelper.GitalyOrPraefect("CreateRepositoryFromBundle: empty Repository", "repo scoped: empty Repository") + msg := testhelper.GitalyOrPraefect("empty Repository", "repo scoped: empty Repository") testhelper.RequireGrpcError(t, err, status.Error(codes.InvalidArgument, msg)) } diff --git a/internal/gitaly/service/repository/create_repository_from_snapshot.go b/internal/gitaly/service/repository/create_repository_from_snapshot.go index 48e2529ff..63a7bb07b 100644 --- a/internal/gitaly/service/repository/create_repository_from_snapshot.go +++ b/internal/gitaly/service/repository/create_repository_from_snapshot.go @@ -12,8 +12,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/labkit/correlation" "gitlab.com/gitlab-org/labkit/tracing" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" ) // httpTransport defines a http.Transport with values that are more restrictive @@ -47,7 +45,7 @@ var httpClient = &http.Client{ func untar(ctx context.Context, path string, in *gitalypb.CreateRepositoryFromSnapshotRequest) error { req, err := http.NewRequestWithContext(ctx, "GET", in.HttpUrl, nil) if err != nil { - return status.Errorf(codes.InvalidArgument, "Bad HTTP URL: %v", err) + return helper.ErrInvalidArgumentf("Bad HTTP URL: %w", err) } if in.HttpAuth != "" { @@ -59,12 +57,12 @@ func untar(ctx context.Context, path string, in *gitalypb.CreateRepositoryFromSn rsp, err := httpClient.Do(req) if err != nil { - return status.Errorf(codes.Internal, "HTTP request failed: %v", err) + return helper.ErrInternalf("HTTP request failed: %w", err) } defer rsp.Body.Close() if rsp.StatusCode < http.StatusOK || rsp.StatusCode >= http.StatusMultipleChoices { - return status.Errorf(codes.Internal, "HTTP server: %v", rsp.Status) + return helper.ErrInternalf("HTTP server: %v", rsp.Status) } cmd, err := command.New(ctx, []string{"tar", "-C", path, "-xvf", "-"}, command.WithStdin(rsp.Body)) diff --git a/internal/gitaly/service/repository/create_repository_from_url.go b/internal/gitaly/service/repository/create_repository_from_url.go index a94506239..8f9b19a9c 100644 --- a/internal/gitaly/service/repository/create_repository_from_url.go +++ b/internal/gitaly/service/repository/create_repository_from_url.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/base64" + "errors" "fmt" "net/url" "os" @@ -73,7 +74,7 @@ func (s *server) cloneFromURLCommand( func (s *server) CreateRepositoryFromURL(ctx context.Context, req *gitalypb.CreateRepositoryFromURLRequest) (*gitalypb.CreateRepositoryFromURLResponse, error) { if err := validateCreateRepositoryFromURLRequest(req); err != nil { - return nil, helper.ErrInvalidArgumentf("CreateRepositoryFromURL: %w", err) + return nil, helper.ErrInvalidArgument(err) } if err := s.createRepository(ctx, req.GetRepository(), func(repo *gitalypb.Repository) error { @@ -123,7 +124,7 @@ func validateCreateRepositoryFromURLRequest(req *gitalypb.CreateRepositoryFromUR } if req.GetUrl() == "" { - return fmt.Errorf("empty Url") + return errors.New("empty Url") } return nil diff --git a/internal/gitaly/service/repository/create_repository_from_url_test.go b/internal/gitaly/service/repository/create_repository_from_url_test.go index 3aa9a169d..472169978 100644 --- a/internal/gitaly/service/repository/create_repository_from_url_test.go +++ b/internal/gitaly/service/repository/create_repository_from_url_test.go @@ -325,14 +325,14 @@ func TestServer_CloneFromURLCommand_validate(t *testing.T) { desc: "no repository provided", req: &gitalypb.CreateRepositoryFromURLRequest{Repository: nil}, expErr: status.Error(codes.InvalidArgument, testhelper.GitalyOrPraefect( - "CreateRepositoryFromURL: empty Repository", + "empty Repository", "repo scoped: empty Repository", )), }, { desc: "no URL provided", req: &gitalypb.CreateRepositoryFromURLRequest{Repository: &gitalypb.Repository{StorageName: cfg.Storages[0].Name, RelativePath: "new"}, Url: ""}, - expErr: status.Error(codes.InvalidArgument, "CreateRepositoryFromURL: empty Url"), + expErr: status.Error(codes.InvalidArgument, "empty Url"), }, } diff --git a/internal/gitaly/service/repository/fetch.go b/internal/gitaly/service/repository/fetch.go index 08d7fd4a3..50067eadd 100644 --- a/internal/gitaly/service/repository/fetch.go +++ b/internal/gitaly/service/repository/fetch.go @@ -53,7 +53,7 @@ func (s *server) FetchSourceBranch(ctx context.Context, req *gitalypb.FetchSourc if errors.Is(err, git.ErrReferenceNotFound) { return &gitalypb.FetchSourceBranchResponse{Result: false}, nil } - return nil, helper.ErrInternal(err) + return nil, helper.ErrInternalf("resolve revision on target: %w", err) } containsObject = true @@ -65,7 +65,7 @@ func (s *server) FetchSourceBranch(ctx context.Context, req *gitalypb.FetchSourc if errors.Is(err, git.ErrReferenceNotFound) { return &gitalypb.FetchSourceBranchResponse{Result: false}, nil } - return nil, helper.ErrInternal(err) + return nil, helper.ErrInternalf("resolve revision on source: %w", err) } // Otherwise, if the source is a remote repository, we check @@ -73,7 +73,7 @@ func (s *server) FetchSourceBranch(ctx context.Context, req *gitalypb.FetchSourc // If so, we can skip the fetch. containsObject, err = targetRepo.HasRevision(ctx, sourceOid.Revision()+"^{commit}") if err != nil { - return nil, helper.ErrInternal(err) + return nil, helper.ErrInternalf("has revision: %w", err) } } @@ -94,12 +94,12 @@ func (s *server) FetchSourceBranch(ctx context.Context, req *gitalypb.FetchSourc return &gitalypb.FetchSourceBranchResponse{Result: false}, nil } - return nil, err + return nil, helper.ErrInternalf("fetch internal: %w", err) } } if err := targetRepo.UpdateRef(ctx, git.ReferenceName(req.GetTargetRef()), sourceOid, ""); err != nil { - return nil, err + return nil, helper.ErrInternalf("update ref: %w", err) } return &gitalypb.FetchSourceBranchResponse{Result: true}, nil diff --git a/internal/gitaly/service/repository/fetch_bundle.go b/internal/gitaly/service/repository/fetch_bundle.go index 2f3fb560e..0805b47fd 100644 --- a/internal/gitaly/service/repository/fetch_bundle.go +++ b/internal/gitaly/service/repository/fetch_bundle.go @@ -23,7 +23,7 @@ const ( func (s *server) FetchBundle(stream gitalypb.RepositoryService_FetchBundleServer) error { firstRequest, err := stream.Recv() if err != nil { - return helper.ErrInternalf("first request: %v", err) + return helper.ErrInternalf("first request: %w", err) } if firstRequest.GetRepository() == nil { @@ -47,13 +47,13 @@ func (s *server) FetchBundle(stream gitalypb.RepositoryService_FetchBundleServer tmpDir, err := tempdir.New(ctx, repo.GetStorageName(), s.locator) if err != nil { - return helper.ErrInternal(err) + return helper.ErrInternalf("tmp dir creation: %w", err) } bundlePath := filepath.Join(tmpDir.Path(), "repo.bundle") file, err := os.Create(bundlePath) if err != nil { - return helper.ErrInternal(err) + return helper.ErrInternalf("bundle file creation: %w", err) } _, err = io.Copy(file, reader) @@ -70,7 +70,7 @@ func (s *server) FetchBundle(stream gitalypb.RepositoryService_FetchBundleServer } if err := repo.FetchRemote(ctx, "inmemory", opts); err != nil { - return helper.ErrInternal(err) + return helper.ErrInternalf("fetch remote: %w", err) } if updateHead { diff --git a/internal/gitaly/service/repository/fetch_bundle_test.go b/internal/gitaly/service/repository/fetch_bundle_test.go index afee7a744..b12526eab 100644 --- a/internal/gitaly/service/repository/fetch_bundle_test.go +++ b/internal/gitaly/service/repository/fetch_bundle_test.go @@ -154,7 +154,7 @@ func TestServer_FetchBundle_validation(t *testing.T) { }, }, expErr: status.Error(codes.NotFound, testhelper.GitalyOrPraefect( - `GetRepoPath: not a git repository: "`+cfg.Storages[0].Path+`/unknown"`, + `fetch remote: GetRepoPath: not a git repository: "`+cfg.Storages[0].Path+`/unknown"`, `mutator call: route repository mutator: get repository id: repository "default"/"unknown" not found`, )), }, diff --git a/internal/gitaly/service/repository/fetch_remote.go b/internal/gitaly/service/repository/fetch_remote.go index 5f7ceda05..26343fcee 100644 --- a/internal/gitaly/service/repository/fetch_remote.go +++ b/internal/gitaly/service/repository/fetch_remote.go @@ -3,6 +3,7 @@ package repository import ( "bytes" "context" + "errors" "fmt" "io" "time" @@ -15,13 +16,11 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/transaction/txinfo" "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) FetchRemote(ctx context.Context, req *gitalypb.FetchRemoteRequest) (*gitalypb.FetchRemoteResponse, error) { if err := s.validateFetchRemoteRequest(req); err != nil { - return nil, err + return nil, helper.ErrInvalidArgument(err) } var stderr bytes.Buffer @@ -69,7 +68,7 @@ func (s *server) FetchRemote(ctx context.Context, req *gitalypb.FetchRemoteReque sshCommand, cleanup, err := git.BuildSSHInvocation(ctx, req.GetSshKey(), req.GetKnownHosts()) if err != nil { - return nil, err + return nil, helper.ErrInternalf("build ssh cmd: %w", err) } defer cleanup() @@ -82,18 +81,12 @@ func (s *server) FetchRemote(ctx context.Context, req *gitalypb.FetchRemoteReque } if err := repo.FetchRemote(ctx, remoteName, opts); err != nil { - if _, ok := status.FromError(err); ok { - // this check is used because of internal call to alternates.PathAndEnv - // which may return gRPC status as an error result - return nil, err - } - errMsg := stderr.String() if errMsg != "" { - return nil, fmt.Errorf("fetch remote: %q: %w", errMsg, err) + return nil, helper.ErrInternalf("fetch remote: %q: %w", errMsg, err) } - return nil, fmt.Errorf("fetch remote: %w", err) + return nil, helper.ErrInternalf("fetch remote: %w", err) } // Ideally, we'd do the voting process via git-fetch(1) using the reference-transaction @@ -122,7 +115,7 @@ func (s *server) FetchRemote(ctx context.Context, req *gitalypb.FetchRemoteReque return s.txManager.Vote(ctx, tx, vote, voting.UnknownPhase) }); err != nil { - return nil, status.Errorf(codes.Aborted, "failed vote on refs: %v", err) + return nil, helper.ErrAbortedf("failed vote on refs: %w", err) } out := &gitalypb.FetchRemoteResponse{TagsChanged: true} @@ -152,15 +145,15 @@ func didTagsChange(r io.Reader) bool { func (s *server) validateFetchRemoteRequest(req *gitalypb.FetchRemoteRequest) error { if req.GetRepository() == nil { - return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) + return gitalyerrors.ErrEmptyRepository } if req.GetRemoteParams() == nil { - return helper.ErrInvalidArgumentf("missing remote params") + return errors.New("missing remote params") } if req.GetRemoteParams().GetUrl() == "" { - return helper.ErrInvalidArgumentf("blank or empty remote URL") + return errors.New("blank or empty remote URL") } return nil diff --git a/internal/gitaly/service/repository/fetch_remote_test.go b/internal/gitaly/service/repository/fetch_remote_test.go index 61ea7b92d..d0a29df7b 100644 --- a/internal/gitaly/service/repository/fetch_remote_test.go +++ b/internal/gitaly/service/repository/fetch_remote_test.go @@ -367,7 +367,7 @@ func TestFetchRemote_force(t *testing.T) { Url: remoteURL, }, }, - expectedErr: status.Error(codes.Unknown, "fetch remote: exit status 1"), + expectedErr: status.Error(codes.Internal, "fetch remote: exit status 1"), expectedRefs: map[git.ReferenceName]git.ObjectID{ "refs/heads/master": branchOID, "refs/tags/v1.0.0": tagOID, @@ -399,7 +399,7 @@ func TestFetchRemote_force(t *testing.T) { // The master branch has been updated to the diverging branch, but the // command still fails because we do fetch tags by default, and the tag did // diverge. - expectedErr: status.Error(codes.Unknown, "fetch remote: exit status 1"), + expectedErr: status.Error(codes.Internal, "fetch remote: exit status 1"), expectedRefs: map[git.ReferenceName]git.ObjectID{ "refs/heads/master": divergingBranchOID, "refs/tags/v1.0.0": tagOID, @@ -553,7 +553,7 @@ func TestFetchRemote_inputValidation(t *testing.T) { }, Timeout: 1000, }, - code: codes.Unknown, + code: codes.Internal, errMsg: "invalid/repo/path.git/' not found", }, { @@ -565,7 +565,7 @@ func TestFetchRemote_inputValidation(t *testing.T) { }, Timeout: 1000, }, - code: codes.Unknown, + code: codes.Internal, errMsg: "'/dev/null' does not appear to be a git repository", }, } diff --git a/internal/gitaly/service/repository/fsck.go b/internal/gitaly/service/repository/fsck.go index b705e8a93..7742020f9 100644 --- a/internal/gitaly/service/repository/fsck.go +++ b/internal/gitaly/service/repository/fsck.go @@ -24,7 +24,7 @@ func (s *server) Fsck(ctx context.Context, req *gitalypb.FsckRequest) (*gitalypb git.WithStderr(&stderr), ) if err != nil { - return nil, err + return nil, helper.ErrInternal(err) } if err = cmd.Wait(); err != nil { diff --git a/internal/gitaly/service/repository/gc.go b/internal/gitaly/service/repository/gc.go index a06a411eb..15ed71188 100644 --- a/internal/gitaly/service/repository/gc.go +++ b/internal/gitaly/service/repository/gc.go @@ -18,7 +18,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/stats" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" - "google.golang.org/grpc/status" ) func (s *server) GarbageCollect(ctx context.Context, in *gitalypb.GarbageCollectRequest) (*gitalypb.GarbageCollectResponse, error) { @@ -38,7 +37,7 @@ func (s *server) GarbageCollect(ctx context.Context, in *gitalypb.GarbageCollect } if err := s.cleanupKeepArounds(ctx, repo); err != nil { - return nil, err + return nil, helper.ErrInternalf("cleanup keep-arounds: %w", err) } // Perform housekeeping to cleanup stale lockfiles that may block GC @@ -47,7 +46,7 @@ func (s *server) GarbageCollect(ctx context.Context, in *gitalypb.GarbageCollect } if err := s.gc(ctx, in); err != nil { - return nil, err + return nil, helper.ErrInternalf("garbage collect: %w", err) } if err := housekeeping.WriteCommitGraph(ctx, repo, housekeeping.WriteCommitGraphConfig{ @@ -75,17 +74,14 @@ func (s *server) gc(ctx context.Context, in *gitalypb.GarbageCollectRequest) err ) if err != nil { if git.IsInvalidArgErr(err) { - return helper.ErrInvalidArgumentf("GarbageCollect: gitCommand: %v", err) + return helper.ErrInvalidArgumentf("gitCommand: %w", err) } - if _, ok := status.FromError(err); ok { - return err - } - return helper.ErrInternal(fmt.Errorf("GarbageCollect: gitCommand: %v", err)) + return helper.ErrInternal(fmt.Errorf("gitCommand: %w", err)) } if err := cmd.Wait(); err != nil { - return helper.ErrInternal(fmt.Errorf("GarbageCollect: cmd wait: %v", err)) + return helper.ErrInternal(fmt.Errorf("cmd wait: %w", err)) } return nil @@ -111,7 +107,7 @@ func (s *server) cleanupKeepArounds(ctx context.Context, repo *localrepo.Repo) e return nil } if err != nil { - return err + return fmt.Errorf("list refs/keep-around: %w", err) } for _, entry := range refEntries { @@ -140,7 +136,7 @@ func (s *server) cleanupKeepArounds(ctx context.Context, repo *localrepo.Repo) e } if err := s.fixRef(ctx, repo, objectInfoReader, path, refName, info.Name()); err != nil { - return err + return fmt.Errorf("fix ref: %w", err) } } diff --git a/internal/gitaly/service/repository/gc_test.go b/internal/gitaly/service/repository/gc_test.go index 87944ce3d..57738b8ba 100644 --- a/internal/gitaly/service/repository/gc_test.go +++ b/internal/gitaly/service/repository/gc_test.go @@ -185,7 +185,7 @@ func TestGarbageCollectDeletesRefsLocks(t *testing.T) { //nolint:staticcheck c, err := client.GarbageCollect(ctx, req) testhelper.RequireGrpcCode(t, err, codes.Internal) - require.Contains(t, err.Error(), "GarbageCollect: cmd wait") + require.Contains(t, err.Error(), "cmd wait") assert.Nil(t, c) // Sanity checks diff --git a/internal/gitaly/service/repository/info_attributes.go b/internal/gitaly/service/repository/info_attributes.go index cf2c86031..9a76425a1 100644 --- a/internal/gitaly/service/repository/info_attributes.go +++ b/internal/gitaly/service/repository/info_attributes.go @@ -9,8 +9,6 @@ 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) GetInfoAttributes(in *gitalypb.GetInfoAttributesRequest, stream gitalypb.RepositoryService_GetInfoAttributesServer) error { @@ -29,7 +27,7 @@ func (s *server) GetInfoAttributes(in *gitalypb.GetInfoAttributesRequest, stream return stream.Send(&gitalypb.GetInfoAttributesResponse{}) } - return status.Errorf(codes.Internal, "GetInfoAttributes failure to read info attributes: %v", err) + return helper.ErrInternalf("failure to read info attributes: %w", err) } sw := streamio.NewWriter(func(p []byte) error { @@ -38,6 +36,8 @@ func (s *server) GetInfoAttributes(in *gitalypb.GetInfoAttributesRequest, stream }) }) - _, err = io.Copy(sw, f) - return err + if _, err = io.Copy(sw, f); err != nil { + return helper.ErrInternalf("send attributes: %w", err) + } + return nil } diff --git a/internal/gitaly/service/repository/license.go b/internal/gitaly/service/repository/license.go index 2f9d9dc55..bda414441 100644 --- a/internal/gitaly/service/repository/license.go +++ b/internal/gitaly/service/repository/license.go @@ -49,7 +49,7 @@ func (s *server) FindLicense(ctx context.Context, req *gitalypb.FindLicenseReque hasHeadRevision, err := repo.HasRevision(ctx, "HEAD") if err != nil { - return nil, helper.ErrInternalf("cannot check HEAD revision: %v", err) + return nil, helper.ErrInternalf("cannot check HEAD revision: %w", err) } if !hasHeadRevision { return &gitalypb.FindLicenseResponse{}, nil @@ -130,7 +130,7 @@ func (s *server) FindLicense(ctx context.Context, req *gitalypb.FindLicenseReque client, err := s.ruby.RepositoryServiceClient(ctx) if err != nil { - return nil, err + return nil, helper.ErrInternalf("sidecar connection: %w", err) } clientCtx, err := rubyserver.SetHeaders(ctx, s.locator, req.GetRepository()) if err != nil { diff --git a/internal/gitaly/service/repository/merge_base.go b/internal/gitaly/service/repository/merge_base.go index 21486caf6..9703e0d39 100644 --- a/internal/gitaly/service/repository/merge_base.go +++ b/internal/gitaly/service/repository/merge_base.go @@ -9,8 +9,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/text" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" ) func (s *server) FindMergeBase(ctx context.Context, req *gitalypb.FindMergeBaseRequest) (*gitalypb.FindMergeBaseResponse, error) { @@ -23,7 +21,7 @@ func (s *server) FindMergeBase(ctx context.Context, req *gitalypb.FindMergeBaseR } if len(revisions) < 2 { - return nil, status.Errorf(codes.InvalidArgument, "FindMergeBase: at least 2 revisions are required") + return nil, helper.ErrInvalidArgumentf("at least 2 revisions are required") } cmd, err := s.gitCmdFactory.New(ctx, req.GetRepository(), @@ -33,15 +31,12 @@ func (s *server) FindMergeBase(ctx context.Context, req *gitalypb.FindMergeBaseR }, ) if err != nil { - if _, ok := status.FromError(err); ok { - return nil, err - } - return nil, status.Errorf(codes.Internal, "FindMergeBase: cmd: %v", err) + return nil, helper.ErrInternalf("cmd: %w", err) } mergeBase, err := io.ReadAll(cmd) if err != nil { - return nil, err + return nil, helper.ErrInternalf("read output: %w", err) } mergeBaseStr := text.ChompBytes(mergeBase) diff --git a/internal/gitaly/service/repository/merge_base_test.go b/internal/gitaly/service/repository/merge_base_test.go index 20d7c5c30..a0e4a4a2d 100644 --- a/internal/gitaly/service/repository/merge_base_test.go +++ b/internal/gitaly/service/repository/merge_base_test.go @@ -104,7 +104,7 @@ func TestFailedFindMergeBaseRequestDueToValidations(t *testing.T) { Repository: repo, Revisions: [][]byte{[]byte("372ab6950519549b14d220271ee2322caa44d4eb")}, }, - expErr: status.Error(codes.InvalidArgument, "FindMergeBase: at least 2 revisions are required"), + expErr: status.Error(codes.InvalidArgument, "at least 2 revisions are required"), }, } { _, err := client.FindMergeBase(ctx, tc.req) diff --git a/internal/gitaly/service/repository/midx.go b/internal/gitaly/service/repository/midx.go index e2d899474..961e21d56 100644 --- a/internal/gitaly/service/repository/midx.go +++ b/internal/gitaly/service/repository/midx.go @@ -18,7 +18,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/stats" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" - "google.golang.org/grpc/status" ) const ( @@ -35,19 +34,16 @@ func (s *server) MidxRepack(ctx context.Context, in *gitalypb.MidxRepackRequest) repo := s.localrepo(repoProto) if err := repo.SetConfig(ctx, "core.multiPackIndex", "true", s.txManager); err != nil { - if _, ok := status.FromError(err); ok { - return nil, err - } return nil, helper.ErrInternalf("setting config: %w", err) } for _, cmd := range []midxSubCommand{s.midxWrite, s.midxExpire, s.midxRepack} { if err := s.safeMidxCommand(ctx, repoProto, cmd); err != nil { if git.IsInvalidArgErr(err) { - return nil, helper.ErrInvalidArgumentf("MidxRepack: %w", err) + return nil, helper.ErrInvalidArgument(err) } - return nil, helper.ErrInternal(fmt.Errorf("...%v", err)) + return nil, helper.ErrInternal(fmt.Errorf("...%w", err)) } } diff --git a/internal/gitaly/service/repository/midx_test.go b/internal/gitaly/service/repository/midx_test.go index d262bca32..b2652566c 100644 --- a/internal/gitaly/service/repository/midx_test.go +++ b/internal/gitaly/service/repository/midx_test.go @@ -300,12 +300,12 @@ func TestMidxRepack_validationChecks(t *testing.T) { { desc: "invalid storage", req: &gitalypb.MidxRepackRequest{Repository: &gitalypb.Repository{StorageName: "invalid"}}, - expErr: status.Error(codes.InvalidArgument, `GetStorageByName: no such storage: "invalid"`), + expErr: status.Error(codes.InvalidArgument, `setting config: GetStorageByName: no such storage: "invalid"`), }, { desc: "not existing repository", req: &gitalypb.MidxRepackRequest{Repository: &gitalypb.Repository{StorageName: cfg.Storages[0].Name, RelativePath: "invalid"}}, - expErr: status.Error(codes.NotFound, fmt.Sprintf(`GetRepoPath: not a git repository: "%s/invalid"`, cfg.Storages[0].Path)), + expErr: status.Error(codes.NotFound, fmt.Sprintf(`setting config: GetRepoPath: not a git repository: "%s/invalid"`, cfg.Storages[0].Path)), }, } { t.Run(tc.desc, func(t *testing.T) { diff --git a/internal/gitaly/service/repository/raw_changes.go b/internal/gitaly/service/repository/raw_changes.go index d76d72a44..db560493d 100644 --- a/internal/gitaly/service/repository/raw_changes.go +++ b/internal/gitaly/service/repository/raw_changes.go @@ -2,6 +2,7 @@ package repository import ( "context" + "errors" "fmt" "io" "regexp" @@ -26,7 +27,7 @@ func (s *server) GetRawChanges(req *gitalypb.GetRawChangesRequest, stream gitaly objectInfoReader, cancel, err := s.catfileCache.ObjectInfoReader(stream.Context(), repo) if err != nil { - return helper.ErrInternal(err) + return helper.ErrInternalf("creating object info reader: %w", err) } defer cancel() @@ -35,7 +36,7 @@ func (s *server) GetRawChanges(req *gitalypb.GetRawChangesRequest, stream gitaly } if err := s.getRawChanges(stream, repo, objectInfoReader, req.GetFromRevision(), req.GetToRevision()); err != nil { - return helper.ErrInternal(err) + return helper.ErrInternalf("read raw changes: %w", err) } return nil @@ -74,7 +75,7 @@ func (s *server) getRawChanges(stream gitalypb.RepositoryService_GetRawChangesSe Args: []string{from, to}, }) if err != nil { - return fmt.Errorf("start git diff: %v", err) + return fmt.Errorf("start git diff: %w", err) } p := rawdiff.NewParser(diffCmd) @@ -86,21 +87,21 @@ func (s *server) getRawChanges(stream gitalypb.RepositoryService_GetRawChangesSe break // happy path } if err != nil { - return fmt.Errorf("read diff: %v", err) + return fmt.Errorf("read diff: %w", err) } change, err := changeFromDiff(ctx, objectInfoReader, d) if err != nil { - return fmt.Errorf("build change from diff line: %v", err) + return fmt.Errorf("build change from diff line: %w", err) } if err := chunker.Send(change); err != nil { - return fmt.Errorf("send response: %v", err) + return fmt.Errorf("send response: %w", err) } } if err := diffCmd.Wait(); err != nil { - return fmt.Errorf("wait git diff: %v", err) + return fmt.Errorf("wait git diff: %w", err) } return chunker.Flush() @@ -159,7 +160,7 @@ func changeFromDiff(ctx context.Context, objectInfoReader catfile.ObjectInfoRead if blobMode != submoduleTreeEntryMode { info, err := objectInfoReader.Info(ctx, git.Revision(shortBlobID)) if err != nil { - return nil, fmt.Errorf("find %q: %v", shortBlobID, err) + return nil, fmt.Errorf("find %q: %w", shortBlobID, err) } resp.BlobId = info.Oid.String() @@ -171,7 +172,7 @@ func changeFromDiff(ctx context.Context, objectInfoReader catfile.ObjectInfoRead func setOperationAndPaths(d *rawdiff.Diff, resp *gitalypb.GetRawChangesResponse_RawChange) error { if len(d.Status) == 0 { - return fmt.Errorf("empty diff status") + return errors.New("empty diff status") } resp.NewPathBytes = []byte(d.SrcPath) diff --git a/internal/gitaly/service/repository/remove.go b/internal/gitaly/service/repository/remove.go index 3e786110a..10a661f7f 100644 --- a/internal/gitaly/service/repository/remove.go +++ b/internal/gitaly/service/repository/remove.go @@ -33,7 +33,7 @@ func (s *server) RemoveRepository(ctx context.Context, in *gitalypb.RemoveReposi } if err := os.MkdirAll(tempDir, 0o755); err != nil { - return nil, helper.ErrInternal(err) + return nil, helper.ErrInternalf("create tmp dir: %w", err) } base := filepath.Base(path) @@ -81,7 +81,7 @@ func (s *server) RemoveRepository(ctx context.Context, in *gitalypb.RemoveReposi } if err := s.voteOnAction(ctx, repo, voting.Prepared); err != nil { - return nil, helper.ErrInternalf("vote on rename: %v", err) + return nil, helper.ErrInternalf("vote on rename: %w", err) } // We move the repository into our temporary directory first before we start to @@ -96,7 +96,7 @@ func (s *server) RemoveRepository(ctx context.Context, in *gitalypb.RemoveReposi } if err := s.voteOnAction(ctx, repo, voting.Committed); err != nil { - return nil, helper.ErrInternalf("vote on finalizing: %v", err) + return nil, helper.ErrInternalf("vote on finalizing: %w", err) } return &gitalypb.RemoveRepositoryResponse{}, nil diff --git a/internal/gitaly/service/repository/rename.go b/internal/gitaly/service/repository/rename.go index cb5c5d172..ca9c4ecbb 100644 --- a/internal/gitaly/service/repository/rename.go +++ b/internal/gitaly/service/repository/rename.go @@ -3,7 +3,6 @@ package repository import ( "context" "errors" - "fmt" "os" "path/filepath" @@ -49,7 +48,7 @@ func (s *server) renameRepository(ctx context.Context, sourceRepo, targetRepo *g } if err := os.MkdirAll(filepath.Dir(targetPath), 0o770); err != nil { - return fmt.Errorf("create target parent dir: %w", err) + return helper.ErrInternalf("create target parent dir: %w", err) } // We're locking both the source repository path and the target repository path for @@ -57,7 +56,7 @@ func (s *server) renameRepository(ctx context.Context, sourceRepo, targetRepo *g // meanwhile, and so that the target repo doesn't get created concurrently either. sourceLocker, err := safe.NewLockingFileWriter(sourcePath) if err != nil { - return fmt.Errorf("creating source repo locker: %w", err) + return helper.ErrInternalf("creating source repo locker: %w", err) } defer func() { if err := sourceLocker.Close(); err != nil { @@ -67,7 +66,7 @@ func (s *server) renameRepository(ctx context.Context, sourceRepo, targetRepo *g targetLocker, err := safe.NewLockingFileWriter(targetPath) if err != nil { - return fmt.Errorf("creating target repo locker: %w", err) + return helper.ErrInternalf("creating target repo locker: %w", err) } defer func() { if err := targetLocker.Close(); err != nil { @@ -77,10 +76,10 @@ func (s *server) renameRepository(ctx context.Context, sourceRepo, targetRepo *g // We're now entering the critical section where both the source and target path are locked. if err := sourceLocker.Lock(); err != nil { - return fmt.Errorf("locking source repo: %w", err) + return helper.ErrInternalf("locking source repo: %w", err) } if err := targetLocker.Lock(); err != nil { - return fmt.Errorf("locking target repo: %w", err) + return helper.ErrInternalf("locking target repo: %w", err) } // We need to re-check whether the target path exists in case somebody has removed it before @@ -90,7 +89,7 @@ func (s *server) renameRepository(ctx context.Context, sourceRepo, targetRepo *g } if err := os.Rename(sourcePath, targetPath); err != nil { - return fmt.Errorf("moving repository into place: %w", err) + return helper.ErrInternalf("moving repository into place: %w", err) } return nil diff --git a/internal/gitaly/service/repository/replicate.go b/internal/gitaly/service/repository/replicate.go index 13e798dca..537be74ad 100644 --- a/internal/gitaly/service/repository/replicate.go +++ b/internal/gitaly/service/repository/replicate.go @@ -30,7 +30,7 @@ import ( ) // ErrInvalidSourceRepository is returned when attempting to replicate from an invalid source repository. -var ErrInvalidSourceRepository = status.Error(codes.NotFound, "invalid source repository") +var ErrInvalidSourceRepository = errors.New("invalid source repository") func (s *server) ReplicateRepository(ctx context.Context, in *gitalypb.ReplicateRepositoryRequest) (*gitalypb.ReplicateRepositoryResponse, error) { if err := validateReplicateRepository(in); err != nil { @@ -45,10 +45,10 @@ func (s *server) ReplicateRepository(ctx context.Context, in *gitalypb.Replicate if !storage.IsGitDirectory(repoPath) { if err = s.create(ctx, in, repoPath); err != nil { if errors.Is(err, ErrInvalidSourceRepository) { - return nil, ErrInvalidSourceRepository + return nil, helper.ErrNotFound(ErrInvalidSourceRepository) } - return nil, helper.ErrInternal(err) + return nil, helper.ErrInternalf("create repository: %w", err) } } @@ -70,7 +70,7 @@ func (s *server) ReplicateRepository(ctx context.Context, in *gitalypb.Replicate return nil, helper.ErrInternalf("checking for repo existence: %w", err) } if !request.GetExists() { - return nil, ErrInvalidSourceRepository + return nil, helper.ErrNotFound(ErrInvalidSourceRepository) } outgoingCtx := metadata.IncomingToOutgoing(ctx) @@ -119,7 +119,7 @@ func (s *server) create(ctx context.Context, in *gitalypb.ReplicateRepositoryReq } if err = os.Rename(repoPath, filepath.Join(tempDir.Path(), filepath.Base(repoPath))); err != nil { - return fmt.Errorf("error deleting invalid repo: %v", err) + return fmt.Errorf("error deleting invalid repo: %w", err) } ctxlogrus.Extract(ctx).WithField("repo_path", repoPath).Warn("removed invalid repository") @@ -166,7 +166,7 @@ func (s *server) extractSnapshot(ctx context.Context, source, target *gitalypb.R firstBytes, err := stream.Recv() if err != nil { if st, ok := status.FromError(err); ok { - if st.Code() == codes.NotFound && strings.HasPrefix(st.Message(), "GetRepoPath: not a git repository:") { + if st.Code() == codes.NotFound && strings.Contains(st.Message(), "GetRepoPath: not a git repository:") { return ErrInvalidSourceRepository } } diff --git a/internal/gitaly/service/repository/replicate_test.go b/internal/gitaly/service/repository/replicate_test.go index 99ab26dcb..40608dc20 100644 --- a/internal/gitaly/service/repository/replicate_test.go +++ b/internal/gitaly/service/repository/replicate_test.go @@ -25,6 +25,7 @@ import ( gitalyhook "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/hook" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/storage" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction" + "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/text" "gitlab.com/gitlab-org/gitaly/v15/internal/metadata" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" @@ -405,7 +406,7 @@ func TestReplicateRepository_BadRepository(t *testing.T) { desc: "source invalid", invalidSource: true, error: func(tb testing.TB, actual error) { - testhelper.RequireGrpcError(tb, ErrInvalidSourceRepository, actual) + testhelper.RequireGrpcError(tb, helper.ErrNotFound(ErrInvalidSourceRepository), actual) }, }, { @@ -413,7 +414,7 @@ func TestReplicateRepository_BadRepository(t *testing.T) { invalidSource: true, invalidTarget: true, error: func(tb testing.TB, actual error) { - testhelper.RequireGrpcError(tb, ErrInvalidSourceRepository, actual) + testhelper.RequireGrpcError(tb, helper.ErrNotFound(ErrInvalidSourceRepository), actual) }, }, } { diff --git a/internal/gitaly/service/repository/repository.go b/internal/gitaly/service/repository/repository.go index 02686fa2c..cd1e77e11 100644 --- a/internal/gitaly/service/repository/repository.go +++ b/internal/gitaly/service/repository/repository.go @@ -7,15 +7,8 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/storage" "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" ) -// Deprecated -func (s *server) Exists(ctx context.Context, in *gitalypb.RepositoryExistsRequest) (*gitalypb.RepositoryExistsResponse, error) { - return nil, status.Error(codes.Unimplemented, "this rpc is not implemented") -} - func (s *server) RepositoryExists(ctx context.Context, in *gitalypb.RepositoryExistsRequest) (*gitalypb.RepositoryExistsResponse, error) { if in.GetRepository() == nil { return nil, helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) diff --git a/internal/gitaly/service/repository/restore_custom_hooks.go b/internal/gitaly/service/repository/restore_custom_hooks.go index 104055449..3231641aa 100644 --- a/internal/gitaly/service/repository/restore_custom_hooks.go +++ b/internal/gitaly/service/repository/restore_custom_hooks.go @@ -18,8 +18,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/transaction/voting" "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) RestoreCustomHooks(stream gitalypb.RepositoryService_RestoreCustomHooksServer) error { @@ -29,12 +27,12 @@ func (s *server) RestoreCustomHooks(stream gitalypb.RepositoryService_RestoreCus firstRequest, err := stream.Recv() if err != nil { - return status.Errorf(codes.Internal, "RestoreCustomHooks: first request failed %v", err) + return helper.ErrInternalf("first request failed %w", err) } repo := firstRequest.GetRepository() if repo == nil { - return helper.ErrInvalidArgumentf("RestoreCustomHooks: %w", gitalyerrors.ErrEmptyRepository) + return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) } reader := streamio.NewReader(func() ([]byte, error) { @@ -50,7 +48,7 @@ func (s *server) RestoreCustomHooks(stream gitalypb.RepositoryService_RestoreCus repoPath, err := s.locator.GetPath(repo) if err != nil { - return status.Errorf(codes.Internal, "RestoreCustomHooks: getting repo path failed %v", err) + return helper.ErrInternalf("getting repo path failed %w", err) } cmdArgs := []string{ @@ -64,11 +62,11 @@ func (s *server) RestoreCustomHooks(stream gitalypb.RepositoryService_RestoreCus ctx := stream.Context() cmd, err := command.New(ctx, append([]string{"tar"}, cmdArgs...), command.WithStdin(reader)) if err != nil { - return status.Errorf(codes.Internal, "RestoreCustomHooks: Could not untar custom hooks tar %v", err) + return helper.ErrInternalf("Could not untar custom hooks tar %w", err) } if err := cmd.Wait(); err != nil { - return status.Errorf(codes.Internal, "RestoreCustomHooks: cmd wait failed: %v", err) + return helper.ErrInternalf("cmd wait failed: %w", err) } return stream.SendAndClose(&gitalypb.RestoreCustomHooksResponse{}) @@ -77,14 +75,14 @@ func (s *server) RestoreCustomHooks(stream gitalypb.RepositoryService_RestoreCus func (s *server) restoreCustomHooksWithVoting(stream gitalypb.RepositoryService_RestoreCustomHooksServer) error { firstRequest, err := stream.Recv() if err != nil { - return helper.ErrInternalf("RestoreCustomHooks: first request failed %w", err) + return helper.ErrInternalf("first request failed %w", err) } ctx := stream.Context() repo := firstRequest.GetRepository() if repo == nil { - return helper.ErrInvalidArgumentf("RestoreCustomHooks: %w", gitalyerrors.ErrEmptyRepository) + return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) } v := voting.NewVoteHash() @@ -97,16 +95,16 @@ func (s *server) restoreCustomHooksWithVoting(stream gitalypb.RepositoryService_ customHooksPath := filepath.Join(repoPath, customHooksDir) if err = os.MkdirAll(customHooksPath, os.ModePerm); err != nil { - return helper.ErrInternalf("RestoreCustomHooks: making custom hooks directory %w", err) + return helper.ErrInternalf("making custom hooks directory %w", err) } lockDir, err := safe.NewLockingDirectory(customHooksPath) if err != nil { - return helper.ErrInternalf("RestoreCustomHooks: creating locking directory: %w", err) + return helper.ErrInternalf("creating locking directory: %w", err) } if err := lockDir.Lock(); err != nil { - return helper.ErrInternalf("RestoreCustomHooks: locking directory failed: %w", err) + return helper.ErrInternalf("locking directory failed: %w", err) } defer func() { @@ -151,11 +149,11 @@ func (s *server) restoreCustomHooksWithVoting(stream gitalypb.RepositoryService_ cmd, err := command.New(ctx, append([]string{"tar"}, cmdArgs...), command.WithStdin(reader)) if err != nil { - return helper.ErrInternalf("RestoreCustomHooks: Could not untar custom hooks tar %w", err) + return helper.ErrInternalf("Could not untar custom hooks tar %w", err) } if err := cmd.Wait(); err != nil { - return helper.ErrInternalf("RestoreCustomHooks: cmd wait failed: %w", err) + return helper.ErrInternalf("cmd wait failed: %w", err) } if err := voteCustomHooks(ctx, s.txManager, &v, voting.Committed); err != nil { @@ -163,7 +161,7 @@ func (s *server) restoreCustomHooksWithVoting(stream gitalypb.RepositoryService_ } if err := lockDir.Unlock(); err != nil { - return helper.ErrInternalf("RestoreCustomHooks: committing lock dir %w", err) + return helper.ErrInternalf("committing lock dir %w", err) } return stream.SendAndClose(&gitalypb.RestoreCustomHooksResponse{}) diff --git a/internal/gitaly/service/repository/restore_custom_hooks_test.go b/internal/gitaly/service/repository/restore_custom_hooks_test.go index 0b5257dd7..436c4900e 100644 --- a/internal/gitaly/service/repository/restore_custom_hooks_test.go +++ b/internal/gitaly/service/repository/restore_custom_hooks_test.go @@ -106,11 +106,11 @@ func testFailedRestoreCustomHooksDueToValidations(t *testing.T, ctx context.Cont stream, err := client.RestoreCustomHooks(ctx) require.NoError(t, err) - require.NoError(t, stream.Send(&gitalypb.RestoreCustomHooksRequest{})) + require.NoError(t, stream.Send(&gitalypb.RestoreCustomHooksRequest{Repository: nil})) _, err = stream.CloseAndRecv() testhelper.RequireGrpcError(t, err, status.Error(codes.InvalidArgument, testhelper.GitalyOrPraefect( - "RestoreCustomHooks: empty Repository", + "empty Repository", "repo scoped: empty Repository", ))) } diff --git a/internal/gitaly/service/repository/search_files.go b/internal/gitaly/service/repository/search_files.go index a7daadef6..0b0e9ba2a 100644 --- a/internal/gitaly/service/repository/search_files.go +++ b/internal/gitaly/service/repository/search_files.go @@ -44,11 +44,11 @@ func (s *server) SearchFilesByContent(req *gitalypb.SearchFilesByContentRequest, git.Flag{Name: "-e"}, }, Args: []string{req.GetQuery(), string(req.GetRef())}}) if err != nil { - return helper.ErrInternalf("SearchFilesByContent: cmd start failed: %v", err) + return helper.ErrInternalf("cmd start failed: %w", err) } if err = sendSearchFilesResultChunked(cmd, stream); err != nil { - return helper.ErrInternalf("SearchFilesByContent: sending chunked response failed: %v", err) + return helper.ErrInternalf("sending chunked response failed: %w", err) } return nil @@ -104,12 +104,12 @@ func (s *server) SearchFilesByName(req *gitalypb.SearchFilesByNameRequest, strea var filter *regexp.Regexp if req.GetFilter() != "" { if len(req.GetFilter()) > searchFilesFilterMaxLength { - return helper.ErrInvalidArgumentf("SearchFilesByName: filter exceeds maximum length") + return helper.ErrInvalidArgumentf("filter exceeds maximum length") } var err error filter, err = regexp.Compile(req.GetFilter()) if err != nil { - return helper.ErrInvalidArgumentf("SearchFilesByName: filter did not compile: %v", err) + return helper.ErrInvalidArgumentf("filter did not compile: %w", err) } } @@ -128,7 +128,7 @@ func (s *server) SearchFilesByName(req *gitalypb.SearchFilesByNameRequest, strea git.Flag{Name: "-z"}, }, Args: []string{string(req.GetRef()), req.GetQuery()}}) if err != nil { - return helper.ErrInternalf("SearchFilesByName: cmd start failed: %v", err) + return helper.ErrInternalf("cmd start failed: %w", err) } files, err := parseLsTree(cmd, filter, int(req.GetOffset()), int(req.GetLimit())) diff --git a/internal/gitaly/service/repository/size.go b/internal/gitaly/service/repository/size.go index 8e7af1566..a872fa6a6 100644 --- a/internal/gitaly/service/repository/size.go +++ b/internal/gitaly/service/repository/size.go @@ -52,7 +52,7 @@ func (s *server) RepositorySize(ctx context.Context, in *gitalypb.RepositorySize if featureflag.RevlistForRepoSize.IsEnabled(ctx) { newSizeBytes, err = calculateSizeWithRevlist(ctx, repo) if err != nil { - return nil, fmt.Errorf("calculating repository size with git-rev-list: %w", err) + return nil, helper.ErrInternalf("calculating repository size with git-rev-list: %w", err) } logger.WithField("repo_size_revlist_bytes", newSizeBytes).Info("repository size calculated") @@ -71,7 +71,7 @@ func (s *server) RepositorySize(ctx context.Context, in *gitalypb.RepositorySize s.housekeepingManager, ) if err != nil { - return nil, fmt.Errorf("calculating repository size with git-cat-file: %w", err) + return nil, helper.ErrInternalf("calculating repository size with git-cat-file: %w", err) } logger.WithField("repo_size_catfile_bytes", newSizeBytes).Info("repository size calculated") diff --git a/internal/gitaly/service/repository/snapshot.go b/internal/gitaly/service/repository/snapshot.go index ffb54b717..7ce174baf 100644 --- a/internal/gitaly/service/repository/snapshot.go +++ b/internal/gitaly/service/repository/snapshot.go @@ -76,7 +76,7 @@ func (s *server) GetSnapshot(in *gitalypb.GetSnapshotRequest, stream gitalypb.Re } if err := builder.Close(); err != nil { - return helper.ErrInternal(fmt.Errorf("building snapshot failed: %v", err)) + return helper.ErrInternal(fmt.Errorf("building snapshot failed: %w", err)) } return nil @@ -101,7 +101,7 @@ func (s *server) addAlternateFiles(ctx context.Context, repository *gitalypb.Rep for _, altObjDir := range altObjDirs { if err := walkAndAddToBuilder(altObjDir, builder); err != nil { - return fmt.Errorf("walking alternates file: %v", err) + return fmt.Errorf("walking alternates file: %w", err) } } @@ -111,7 +111,7 @@ func (s *server) addAlternateFiles(ctx context.Context, repository *gitalypb.Rep func walkAndAddToBuilder(alternateObjDir string, builder *archive.TarBuilder) error { matchWalker := archive.NewMatchWalker(objectFiles, func(path string, info os.FileInfo, err error) error { if err != nil { - return fmt.Errorf("error walking %v: %v", path, err) + return fmt.Errorf("error walking %v: %w", path, err) } relPath, err := filepath.Rel(alternateObjDir, path) @@ -121,21 +121,21 @@ func walkAndAddToBuilder(alternateObjDir string, builder *archive.TarBuilder) er file, err := os.Open(path) if err != nil { - return fmt.Errorf("opening file %s: %v", path, err) + return fmt.Errorf("opening file %s: %w", path, err) } defer file.Close() objectPath := filepath.Join("objects", relPath) if err := builder.VirtualFileWithContents(objectPath, file); err != nil { - return fmt.Errorf("expected file %v to exist: %v", path, err) + return fmt.Errorf("expected file %v to exist: %w", path, err) } return nil }) if err := filepath.Walk(alternateObjDir, matchWalker.Walk); err != nil { - return fmt.Errorf("error when traversing: %v", err) + return fmt.Errorf("error when traversing: %w", err) } return nil diff --git a/internal/gitaly/service/repository/util.go b/internal/gitaly/service/repository/util.go index 9f11c3707..c4807e896 100644 --- a/internal/gitaly/service/repository/util.go +++ b/internal/gitaly/service/repository/util.go @@ -22,10 +22,10 @@ import ( func (s *server) removeOriginInRepo(ctx context.Context, repository *gitalypb.Repository) error { cmd, err := s.gitCmdFactory.New(ctx, repository, git.SubCmd{Name: "remote", Args: []string{"remove", "origin"}}, git.WithRefTxHook(repository)) if err != nil { - return fmt.Errorf("remote cmd start: %v", err) + return fmt.Errorf("remote cmd start: %w", err) } if err := cmd.Wait(); err != nil { - return fmt.Errorf("remote cmd wait: %v", err) + return fmt.Errorf("remote cmd wait: %w", err) } return nil diff --git a/internal/gitaly/service/repository/write_ref.go b/internal/gitaly/service/repository/write_ref.go index 1e86f3fcd..a04ec6c6d 100644 --- a/internal/gitaly/service/repository/write_ref.go +++ b/internal/gitaly/service/repository/write_ref.go @@ -3,6 +3,7 @@ package repository import ( "bytes" "context" + "errors" "fmt" gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" @@ -18,6 +19,9 @@ func (s *server) WriteRef(ctx context.Context, req *gitalypb.WriteRefRequest) (* return nil, helper.ErrInvalidArgument(err) } if err := s.writeRef(ctx, req); err != nil { + if errors.Is(err, git.ErrReferenceNotFound) { + return nil, helper.ErrNotFound(err) + } return nil, helper.ErrInternal(err) } @@ -29,13 +33,16 @@ func (s *server) writeRef(ctx context.Context, req *gitalypb.WriteRefRequest) er if string(req.Ref) == "HEAD" { if err := repo.SetDefaultBranch(ctx, s.txManager, git.ReferenceName(req.GetRevision())); err != nil { - return fmt.Errorf("setting default branch: %v", err) + return fmt.Errorf("setting default branch: %w", err) } return nil } - return updateRef(ctx, repo, req) + if err := updateRef(ctx, repo, req); err != nil { + return fmt.Errorf("update ref: %w", err) + } + return nil } func updateRef(ctx context.Context, repo *localrepo.Repo, req *gitalypb.WriteRefRequest) error { @@ -73,15 +80,15 @@ func updateRef(ctx context.Context, repo *localrepo.Repo, req *gitalypb.WriteRef u, err := updateref.New(ctx, repo) if err != nil { - return fmt.Errorf("error when running creating new updater: %v", err) + return fmt.Errorf("error when running creating new updater: %w", err) } if err = u.Update(git.ReferenceName(req.GetRef()), newObjectID, oldObjectID); err != nil { - return fmt.Errorf("error when creating update-ref command: %v", err) + return fmt.Errorf("error when creating update-ref command: %w", err) } if err = u.Commit(); err != nil { - return fmt.Errorf("error when running update-ref command: %v", err) + return fmt.Errorf("error when running update-ref command: %w", err) } return nil @@ -92,19 +99,19 @@ func validateWriteRefRequest(req *gitalypb.WriteRefRequest) error { return gitalyerrors.ErrEmptyRepository } if err := git.ValidateRevision(req.Ref); err != nil { - return fmt.Errorf("invalid ref: %v", err) + return fmt.Errorf("invalid ref: %w", err) } if err := git.ValidateRevision(req.Revision); err != nil { - return fmt.Errorf("invalid revision: %v", err) + return fmt.Errorf("invalid revision: %w", err) } if len(req.OldRevision) > 0 { if err := git.ValidateRevision(req.OldRevision); err != nil { - return fmt.Errorf("invalid OldRevision: %v", err) + return fmt.Errorf("invalid OldRevision: %w", err) } } if !bytes.Equal(req.Ref, []byte("HEAD")) && !bytes.HasPrefix(req.Ref, []byte("refs/")) { - return fmt.Errorf("ref has to be a full reference") + return errors.New("ref has to be a full reference") } return nil } diff --git a/internal/gitaly/service/repository/write_ref_test.go b/internal/gitaly/service/repository/write_ref_test.go index c6b38d713..2429b3b8f 100644 --- a/internal/gitaly/service/repository/write_ref_test.go +++ b/internal/gitaly/service/repository/write_ref_test.go @@ -217,7 +217,7 @@ func TestWriteRef_missingRevisions(t *testing.T) { Ref: []byte("refs/heads/main"), Revision: []byte("refs/heads/missing"), }, - expectedErr: helper.ErrInternalf("resolving new revision: reference not found"), + expectedErr: helper.ErrNotFoundf("update ref: resolving new revision: reference not found"), }, { desc: "revision refers to missing object", @@ -226,7 +226,7 @@ func TestWriteRef_missingRevisions(t *testing.T) { Ref: []byte("refs/heads/main"), Revision: bytes.Repeat([]byte("1"), gittest.DefaultObjectHash.EncodedLen()), }, - expectedErr: helper.ErrInternalf("resolving new revision: reference not found"), + expectedErr: helper.ErrNotFoundf("update ref: resolving new revision: reference not found"), }, { desc: "old revision refers to missing reference", @@ -236,7 +236,7 @@ func TestWriteRef_missingRevisions(t *testing.T) { Revision: []byte(commitID), OldRevision: bytes.Repeat([]byte("1"), gittest.DefaultObjectHash.EncodedLen()), }, - expectedErr: helper.ErrInternalf("resolving old revision: reference not found"), + expectedErr: helper.ErrNotFoundf("update ref: resolving old revision: reference not found"), }, } { t.Run(tc.desc, func(t *testing.T) { diff --git a/internal/praefect/replicator.go b/internal/praefect/replicator.go index d7d66a95e..b49b9beb2 100644 --- a/internal/praefect/replicator.go +++ b/internal/praefect/replicator.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "strings" "sync" "time" @@ -19,6 +20,8 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" "gitlab.com/gitlab-org/labkit/correlation" "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "google.golang.org/protobuf/proto" ) @@ -79,13 +82,15 @@ func (dr defaultReplicator) Replicate(ctx context.Context, event datastore.Repli Source: sourceRepository, Repository: targetRepository, }); err != nil { - if errors.Is(err, repository.ErrInvalidSourceRepository) { - if err := dr.rs.DeleteInvalidRepository(ctx, event.Job.RepositoryID, event.Job.SourceNodeStorage); err != nil { - return fmt.Errorf("delete invalid repository: %w", err) - } + if st, ok := status.FromError(err); ok { + if st.Code() == codes.NotFound && strings.Contains(st.Message(), repository.ErrInvalidSourceRepository.Error()) { + if err := dr.rs.DeleteInvalidRepository(ctx, event.Job.RepositoryID, event.Job.SourceNodeStorage); err != nil { + return fmt.Errorf("delete invalid repository: %w", err) + } - logger.Info("invalid repository record removed") - return nil + logger.Info("invalid repository record removed") + return nil + } } return fmt.Errorf("failed to create repository: %w", err) diff --git a/internal/praefect/replicator_pg_test.go b/internal/praefect/replicator_pg_test.go index 2ee5ef568..27c694538 100644 --- a/internal/praefect/replicator_pg_test.go +++ b/internal/praefect/replicator_pg_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/client" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service/repository" + "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/praefect/datastore" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testdb" @@ -41,7 +42,7 @@ func TestReplicatorInvalidSourceRepository(t *testing.T) { srv := grpc.NewServer() gitalypb.RegisterRepositoryServiceServer(srv, &mockRepositoryService{ ReplicateRepositoryFunc: func(context.Context, *gitalypb.ReplicateRepositoryRequest) (*gitalypb.ReplicateRepositoryResponse, error) { - return nil, repository.ErrInvalidSourceRepository + return nil, helper.ErrNotFoundf("validate: %w", repository.ErrInvalidSourceRepository) }, }) defer srv.Stop() |