diff --git a/internal/gitaly/service/repository/create.go b/internal/gitaly/service/repository/create.go index b9fc42d3a274b849f4538219a54e72e3c6696248..16daec9e5a0d62fc083e06885348f7b9fe286d38 100644 --- a/internal/gitaly/service/repository/create.go +++ b/internal/gitaly/service/repository/create.go @@ -13,11 +13,11 @@ import ( func (s *server) CreateRepository(ctx context.Context, req *gitalypb.CreateRepositoryRequest) (*gitalypb.CreateRepositoryResponse, error) { diskPath, err := s.locator.GetPath(req.GetRepository()) if err != nil { - return nil, helper.ErrInvalidArgumentf("locate repository: %w", err) + return nil, helper.ErrInvalidArgumentf("locate repository: %v", err) } if err := os.MkdirAll(diskPath, 0770); err != nil { - return nil, helper.ErrInternalf("create directories: %w", err) + return nil, helper.ErrInternalf("create directories: %v", err) } stderr := &bytes.Buffer{} @@ -32,11 +32,11 @@ func (s *server) CreateRepository(ctx context.Context, req *gitalypb.CreateRepos }, ) if err != nil { - return nil, helper.ErrInternalf("create git init: %w", err) + return nil, helper.ErrInternalf("create git init: %v", err) } if err := cmd.Wait(); err != nil { - return nil, helper.ErrInternalf("git init stderr: %q, err: %w", stderr, err) + return nil, helper.ErrInternalf("git init stderr: %q, err: %v", stderr, err) } return &gitalypb.CreateRepositoryResponse{}, nil diff --git a/internal/gitaly/service/repository/create_bundle.go b/internal/gitaly/service/repository/create_bundle.go index 4f2d631c5e8d9e31c3ddabe76c4b3a62358ad7ec..2f57ddbc2c81e7aaf198d686f2d722cfbe92fd97 100644 --- a/internal/gitaly/service/repository/create_bundle.go +++ b/internal/gitaly/service/repository/create_bundle.go @@ -20,7 +20,7 @@ func (s *server) CreateBundle(req *gitalypb.CreateBundleRequest, stream gitalypb ctx := stream.Context() if _, err := s.Cleanup(ctx, &gitalypb.CleanupRequest{Repository: req.GetRepository()}); err != nil { - return helper.ErrInternalf("running Cleanup on repository: %w", err) + return helper.ErrInternalf("running Cleanup on repository: %v", err) } cmd, err := git.SafeCmd(ctx, repo, nil, git.SubCmd{ diff --git a/internal/gitaly/service/repository/midx.go b/internal/gitaly/service/repository/midx.go index ec04e3dc6418a698ba3099a0f586978c9e54f711..299a229ed79364b26111317c31cf9778d19b9851 100644 --- a/internal/gitaly/service/repository/midx.go +++ b/internal/gitaly/service/repository/midx.go @@ -30,7 +30,7 @@ func (s *server) MidxRepack(ctx context.Context, in *gitalypb.MidxRepackRequest) for _, cmd := range []midxSubCommand{midxWrite, midxExpire, s.midxRepack} { if err := s.safeMidxCommand(ctx, repo, cmd); err != nil { if git.IsInvalidArgErr(err) { - return nil, helper.ErrInvalidArgumentf("MidxRepack: %w", err) + return nil, helper.ErrInvalidArgumentf("MidxRepack: %v", err) } return nil, helper.ErrInternal(fmt.Errorf("...%v", err)) diff --git a/internal/gitaly/service/repository/raw_changes.go b/internal/gitaly/service/repository/raw_changes.go index dab1d1fb1d24fbd5bb0c1e5685d1b044ce7c0b79..95229d1f5ffbe93808e90cbb15c57f95ded279b9 100644 --- a/internal/gitaly/service/repository/raw_changes.go +++ b/internal/gitaly/service/repository/raw_changes.go @@ -14,13 +14,19 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/internal/helper/chunk" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) func (s *server) GetRawChanges(req *gitalypb.GetRawChangesRequest, stream gitalypb.RepositoryService_GetRawChangesServer) error { repo := req.Repository batch, err := catfile.New(stream.Context(), repo) if err != nil { - return helper.ErrInternal(err) + _, ok := status.FromError(err) + if !ok { + return status.Errorf(codes.Internal, "%s", err.Error()) + } + return err } if err := validateRawChangesRequest(req, batch); err != nil { diff --git a/internal/gitaly/service/repository/replicate.go b/internal/gitaly/service/repository/replicate.go index 992398fb56d9a88d3ef82b7fb2135a5f28cd1928..230356b95e2c610f32529dc8312b952d4d4c5b52 100644 --- a/internal/gitaly/service/repository/replicate.go +++ b/internal/gitaly/service/repository/replicate.go @@ -34,7 +34,11 @@ func (s *server) ReplicateRepository(ctx context.Context, in *gitalypb.Replicate repoPath, err := s.locator.GetPath(in.GetRepository()) if err != nil { - return nil, helper.ErrInternal(err) + _, ok := status.FromError(err) + if !ok { + return nil, status.Errorf(codes.Internal, "%s", err.Error()) + } + return nil, err } if !storage.IsGitDirectory(repoPath) { diff --git a/internal/gitaly/service/repository/repository.go b/internal/gitaly/service/repository/repository.go index 6ceb3a101c51bd6fa2e2f555b5751449e674e2b8..30e696b2dc233eec47ceb76bdc7dcd8ddb5aac23 100644 --- a/internal/gitaly/service/repository/repository.go +++ b/internal/gitaly/service/repository/repository.go @@ -7,11 +7,13 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/internal/storage" "gitlab.com/gitlab-org/gitaly/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, helper.Unimplemented + return nil, status.Error(codes.Unimplemented, "this rpc is not implemented") } func (s *server) RepositoryExists(ctx context.Context, in *gitalypb.RepositoryExistsRequest) (*gitalypb.RepositoryExistsResponse, error) { diff --git a/internal/gitaly/service/repository/search_files.go b/internal/gitaly/service/repository/search_files.go index 958880097234d5dbc1d75da52843edfbd84450e7..7f2540177165fd41519e614218c21659961a3117 100644 --- a/internal/gitaly/service/repository/search_files.go +++ b/internal/gitaly/service/repository/search_files.go @@ -30,7 +30,7 @@ var contentDelimiter = []byte("--\n") func (s *server) SearchFilesByContent(req *gitalypb.SearchFilesByContentRequest, stream gitalypb.RepositoryService_SearchFilesByContentServer) error { if err := validateSearchFilesRequest(req); err != nil { - return helper.DecorateError(codes.InvalidArgument, err) + return status.Errorf(codes.InvalidArgument, err.Error()) } repo := req.GetRepository() @@ -106,7 +106,7 @@ func sendSearchFilesResultChunked(cmd *command.Command, stream gitalypb.Reposito func (s *server) SearchFilesByName(req *gitalypb.SearchFilesByNameRequest, stream gitalypb.RepositoryService_SearchFilesByNameServer) error { if err := validateSearchFilesRequest(req); err != nil { - return helper.DecorateError(codes.InvalidArgument, err) + return status.Errorf(codes.InvalidArgument, err.Error()) } var filter *regexp.Regexp diff --git a/internal/gitaly/service/repository/snapshot.go b/internal/gitaly/service/repository/snapshot.go index 87a5634792a1f9baa136906b7839940222d05fa4..d628b53a279f20bf68c71ecacc3acc610b7b0245 100644 --- a/internal/gitaly/service/repository/snapshot.go +++ b/internal/gitaly/service/repository/snapshot.go @@ -67,7 +67,7 @@ func (s *server) GetSnapshot(in *gitalypb.GetSnapshotRequest, stream gitalypb.Re builder.FileIfExist("shallow") if err := s.addAlternateFiles(stream.Context(), in.GetRepository(), builder); err != nil { - return helper.ErrInternalf("add alternates: %w", err) + return helper.ErrInternalf("add alternates: %v", err) } if err := builder.Close(); err != nil { diff --git a/internal/helper/error.go b/internal/helper/error.go index 664989a58c6faac270b9620b1232ef29ef03ab6d..a9f3730ccba52538d9d8577acbe77ab9a6d3b685 100644 --- a/internal/helper/error.go +++ b/internal/helper/error.go @@ -1,15 +1,10 @@ package helper import ( - "fmt" - "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) -// Unimplemented is a Go error with gRPC error code 'Unimplemented' -var Unimplemented = status.Error(codes.Unimplemented, "this rpc is not implemented") - type statusWrapper struct { error status *status.Status @@ -27,37 +22,31 @@ func (sw statusWrapper) Unwrap() error { // If given nil it will return nil. func DecorateError(code codes.Code, err error) error { if err != nil && GrpcCode(err) == codes.Unknown { + panic("we should not be relying on wrapped errors!") return statusWrapper{err, status.New(code, err.Error())} } return err } -// ErrInternal wraps err with codes.Internal, unless err is already a grpc error -func ErrInternal(err error) error { return DecorateError(codes.Internal, err) } - -// ErrInternalf wrapps a formatted error with codes.Internal, unless err is already a grpc error +func ErrInternal(err error) error { return status.Errorf(codes.Internal, "%s", err.Error()) } func ErrInternalf(format string, a ...interface{}) error { - return ErrInternal(fmt.Errorf(format, a...)) + return status.Errorf(codes.Internal, format, a...) } -// ErrInvalidArgument wraps err with codes.InvalidArgument, unless err is already a grpc error -func ErrInvalidArgument(err error) error { return DecorateError(codes.InvalidArgument, err) } - -// ErrInvalidArgumentf wraps a formatted error with codes.InvalidArgument, unless err is already a grpc error +func ErrInvalidArgument(err error) error { return status.Errorf(codes.InvalidArgument, err.Error()) } func ErrInvalidArgumentf(format string, a ...interface{}) error { - return ErrInvalidArgument(fmt.Errorf(format, a...)) + return status.Errorf(codes.InvalidArgument, format, a...) } -// ErrPreconditionFailed wraps err with codes.FailedPrecondition, unless err is already a grpc error -func ErrPreconditionFailed(err error) error { return DecorateError(codes.FailedPrecondition, err) } +func ErrPreconditionFailed(err error) error { + return status.Errorf(codes.FailedPrecondition, "%s", err.Error()) +} -// ErrPreconditionFailedf wraps a formatted error with codes.FailedPrecondition, unless err is already a grpc error func ErrPreconditionFailedf(format string, a ...interface{}) error { - return ErrPreconditionFailed(fmt.Errorf(format, a...)) + return status.Errorf(codes.FailedPrecondition, format, a...) } -// ErrNotFound wraps error with codes.NotFound, unless err is already a grpc error -func ErrNotFound(err error) error { return DecorateError(codes.NotFound, err) } +func ErrNotFound(err error) error { return status.Errorf(codes.NotFound, "%s", err.Error()) } // GrpcCode emulates the old grpc.Code function: it translates errors into codes.Code values. func GrpcCode(err error) codes.Code {