Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitaly.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPavlo Strokov <pstrokov@gitlab.com>2022-09-27 11:12:00 +0300
committerPavlo Strokov <pstrokov@gitlab.com>2022-10-24 09:24:30 +0300
commite82bfb7c48e8b68467c4e06be5668588af37f47f (patch)
tree70360561c3eaaea85beb17d65600534755a20f89
parent90ac673d08c844a3236f0041e7f758be19a6ddef (diff)
repository: Standardization of errors creationps-errors-management
Return Internal code in case of an unexpected operation failure instead of Undefined by using helper.ErrInternal(). Add context to the error to make it more descriptive and standard. Return pre-defined ErrEmptyRepository instead of ad-hoc created error. Replace old %v with new %w for proper error wrapping. Replace usage of status.Errorf() with helper.Err<StatusCode>f() to make error creation more standard. Replace fmt.Errorf() with errors.New() if used without a reason. Remove redundant prefix from error returned by RPC call. Check error type to convert it to the corresponding error code.
-rw-r--r--internal/gitaly/service/repository/apply_gitattributes.go24
-rw-r--r--internal/gitaly/service/repository/apply_gitattributes_test.go4
-rw-r--r--internal/gitaly/service/repository/archive.go14
-rw-r--r--internal/gitaly/service/repository/backup_custom_hooks.go8
-rw-r--r--internal/gitaly/service/repository/calculate_checksum.go12
-rw-r--r--internal/gitaly/service/repository/config.go8
-rw-r--r--internal/gitaly/service/repository/create_bundle.go10
-rw-r--r--internal/gitaly/service/repository/create_bundle_from_ref_list.go10
-rw-r--r--internal/gitaly/service/repository/create_bundle_test.go2
-rw-r--r--internal/gitaly/service/repository/create_fork.go6
-rw-r--r--internal/gitaly/service/repository/create_fork_test.go4
-rw-r--r--internal/gitaly/service/repository/create_repository_from_bundle.go6
-rw-r--r--internal/gitaly/service/repository/create_repository_from_bundle_test.go2
-rw-r--r--internal/gitaly/service/repository/create_repository_from_snapshot.go8
-rw-r--r--internal/gitaly/service/repository/create_repository_from_url.go5
-rw-r--r--internal/gitaly/service/repository/create_repository_from_url_test.go4
-rw-r--r--internal/gitaly/service/repository/fetch.go10
-rw-r--r--internal/gitaly/service/repository/fetch_bundle.go8
-rw-r--r--internal/gitaly/service/repository/fetch_bundle_test.go2
-rw-r--r--internal/gitaly/service/repository/fetch_remote.go25
-rw-r--r--internal/gitaly/service/repository/fetch_remote_test.go8
-rw-r--r--internal/gitaly/service/repository/fsck.go2
-rw-r--r--internal/gitaly/service/repository/gc.go18
-rw-r--r--internal/gitaly/service/repository/gc_test.go2
-rw-r--r--internal/gitaly/service/repository/info_attributes.go10
-rw-r--r--internal/gitaly/service/repository/license.go4
-rw-r--r--internal/gitaly/service/repository/merge_base.go11
-rw-r--r--internal/gitaly/service/repository/merge_base_test.go2
-rw-r--r--internal/gitaly/service/repository/midx.go8
-rw-r--r--internal/gitaly/service/repository/midx_test.go4
-rw-r--r--internal/gitaly/service/repository/raw_changes.go19
-rw-r--r--internal/gitaly/service/repository/remove.go6
-rw-r--r--internal/gitaly/service/repository/rename.go13
-rw-r--r--internal/gitaly/service/repository/replicate.go12
-rw-r--r--internal/gitaly/service/repository/replicate_test.go5
-rw-r--r--internal/gitaly/service/repository/repository.go7
-rw-r--r--internal/gitaly/service/repository/restore_custom_hooks.go28
-rw-r--r--internal/gitaly/service/repository/restore_custom_hooks_test.go4
-rw-r--r--internal/gitaly/service/repository/search_files.go10
-rw-r--r--internal/gitaly/service/repository/size.go4
-rw-r--r--internal/gitaly/service/repository/snapshot.go12
-rw-r--r--internal/gitaly/service/repository/util.go4
-rw-r--r--internal/gitaly/service/repository/write_ref.go25
-rw-r--r--internal/gitaly/service/repository/write_ref_test.go6
-rw-r--r--internal/praefect/replicator.go17
-rw-r--r--internal/praefect/replicator_pg_test.go3
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()