diff options
author | Pavlo Strokov <pstrokov@gitlab.com> | 2022-09-05 11:29:50 +0300 |
---|---|---|
committer | Pavlo Strokov <pstrokov@gitlab.com> | 2022-09-05 11:29:50 +0300 |
commit | 4f158ae85f13c787f33568ea14feedb69c479b02 (patch) | |
tree | 7e90777c019f1b285524cb202fb45bf8afc763d4 | |
parent | ea920a303a90a5cb9d0f2aaea64ed782a9dd800d (diff) | |
parent | f9335638bb37b297de763e7be9ddbcb593e025a0 (diff) |
Merge branch 'ps-standardize-errors' into 'master'
blob: Standardize returned errors
See merge request gitlab-org/gitaly!4849
-rw-r--r-- | internal/gitaly/service/blob/blobs.go | 7 | ||||
-rw-r--r-- | internal/gitaly/service/blob/get_blob.go | 21 | ||||
-rw-r--r-- | internal/gitaly/service/blob/get_blobs.go | 38 | ||||
-rw-r--r-- | internal/gitaly/service/blob/lfs_pointers.go | 14 |
4 files changed, 43 insertions, 37 deletions
diff --git a/internal/gitaly/service/blob/blobs.go b/internal/gitaly/service/blob/blobs.go index f8b72177f..64c5f8e62 100644 --- a/internal/gitaly/service/blob/blobs.go +++ b/internal/gitaly/service/blob/blobs.go @@ -7,6 +7,7 @@ import ( "io" "strings" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/git/gitpipe" "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" @@ -19,7 +20,7 @@ import ( func verifyListBlobsRequest(req *gitalypb.ListBlobsRequest) error { if req.GetRepository() == nil { - return errors.New("empty repository") + return gitalyerrors.ErrEmptyRepository } if len(req.GetRevisions()) == 0 { return errors.New("missing revisions") @@ -141,7 +142,7 @@ func (s *server) processBlobs( catfileObjectIter, err := gitpipe.CatfileObject(ctx, objectReader, objectIter) if err != nil { - return helper.ErrInternalf("creating catfile object iterator: %w", err) + return helper.ErrInternalf("creating object iterator: %w", err) } var i uint32 @@ -237,7 +238,7 @@ func (s *server) ListAllBlobs(req *gitalypb.ListAllBlobsRequest, stream gitalypb ctx := stream.Context() if req.GetRepository() == nil { - return helper.ErrInvalidArgumentf("empty repository") + return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) } repo := s.localrepo(req.GetRepository()) diff --git a/internal/gitaly/service/blob/get_blob.go b/internal/gitaly/service/blob/get_blob.go index b3f593e68..77ad6dfae 100644 --- a/internal/gitaly/service/blob/get_blob.go +++ b/internal/gitaly/service/blob/get_blob.go @@ -1,9 +1,10 @@ package blob import ( - "fmt" + "errors" "io" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v15/internal/helper" @@ -14,15 +15,15 @@ import ( func (s *server) GetBlob(in *gitalypb.GetBlobRequest, stream gitalypb.BlobService_GetBlobServer) error { ctx := stream.Context() - repo := s.localrepo(in.GetRepository()) - if err := validateRequest(in); err != nil { - return helper.ErrInvalidArgumentf("GetBlob: %v", err) + return helper.ErrInvalidArgument(err) } + repo := s.localrepo(in.GetRepository()) + objectReader, cancel, err := s.catfileCache.ObjectReader(stream.Context(), repo) if err != nil { - return helper.ErrInternalf("GetBlob: %v", err) + return helper.ErrInternalf("create object reader: %w", err) } defer cancel() @@ -31,7 +32,7 @@ func (s *server) GetBlob(in *gitalypb.GetBlobRequest, stream gitalypb.BlobServic if catfile.IsNotFound(err) { return helper.ErrUnavailable(stream.Send(&gitalypb.GetBlobResponse{})) } - return helper.ErrInternalf("GetBlob: %v", err) + return helper.ErrInternalf("read object: %w", err) } if blob.Type != "blob" { @@ -63,15 +64,19 @@ func (s *server) GetBlob(in *gitalypb.GetBlobRequest, stream gitalypb.BlobServic _, err = io.CopyN(sw, blob, readLimit) if err != nil { - return helper.ErrUnavailablef("GetBlob: send: %v", err) + return helper.ErrUnavailablef("send: %w", err) } return nil } func validateRequest(in *gitalypb.GetBlobRequest) error { + if in.GetRepository() == nil { + return gitalyerrors.ErrEmptyRepository + } + if len(in.GetOid()) == 0 { - return fmt.Errorf("empty Oid") + return errors.New("empty Oid") } return nil } diff --git a/internal/gitaly/service/blob/get_blobs.go b/internal/gitaly/service/blob/get_blobs.go index 18694930f..114fbbfc0 100644 --- a/internal/gitaly/service/blob/get_blobs.go +++ b/internal/gitaly/service/blob/get_blobs.go @@ -2,14 +2,16 @@ package blob import ( "bytes" + "errors" + "fmt" "io" + gitalyerrors "gitlab.com/gitlab-org/gitaly/v15/internal/errors" "gitlab.com/gitlab-org/gitaly/v15/internal/git" "gitlab.com/gitlab-org/gitaly/v15/internal/git/catfile" + "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" ) var treeEntryToObjectType = map[gitalypb.TreeEntry_EntryType]gitalypb.ObjectType{ @@ -38,14 +40,14 @@ func sendGetBlobsResponse( treeEntry, err := tef.FindByRevisionAndPath(ctx, revision, string(path)) if err != nil { - return err + return helper.ErrInternalf("find by revision and path: %w", err) } response := &gitalypb.GetBlobsResponse{Revision: revision, Path: path} if treeEntry == nil || len(treeEntry.Oid) == 0 { if err := stream.Send(response); err != nil { - return status.Errorf(codes.Unavailable, "GetBlobs: send: %v", err) + return helper.ErrUnavailablef("send: %w", err) } continue @@ -59,7 +61,7 @@ func sendGetBlobsResponse( response.Type = gitalypb.ObjectType_COMMIT if err := stream.Send(response); err != nil { - return status.Errorf(codes.Unavailable, "GetBlobs: send: %v", err) + return helper.ErrUnavailablef("send: %w", err) } continue @@ -67,7 +69,7 @@ func sendGetBlobsResponse( objectInfo, err := objectInfoReader.Info(ctx, git.Revision(treeEntry.Oid)) if err != nil { - return status.Errorf(codes.Internal, "GetBlobs: %v", err) + return helper.ErrInternalf("read object info: %w", err) } response.Size = objectInfo.Size @@ -81,7 +83,7 @@ func sendGetBlobsResponse( if response.Type != gitalypb.ObjectType_BLOB { if err := stream.Send(response); err != nil { - return status.Errorf(codes.Unavailable, "GetBlobs: send: %v", err) + return helper.ErrUnavailablef("send: %w", err) } continue } @@ -114,22 +116,22 @@ func sendBlobTreeEntry( // blobObj. if readLimit == 0 { if err := stream.Send(response); err != nil { - return status.Errorf(codes.Unavailable, "GetBlobs: send: %v", err) + return helper.ErrUnavailablef("send: %w", err) } return nil } blobObj, err := objectReader.Object(ctx, git.Revision(response.Oid)) if err != nil { - return status.Errorf(codes.Internal, "GetBlobs: %v", err) + return helper.ErrInternalf("read object: %w", err) } defer func() { if _, err := io.Copy(io.Discard, blobObj); err != nil && returnedErr == nil { - returnedErr = status.Errorf(codes.Internal, "GetBlobs: discarding data: %v", err) + returnedErr = helper.ErrInternalf("discarding data: %w", err) } }() if blobObj.Type != "blob" { - return status.Errorf(codes.Internal, "blob got unexpected type %q", blobObj.Type) + return helper.ErrInternalf("blob got unexpected type %q", blobObj.Type) } sw := streamio.NewWriter(func(p []byte) error { @@ -146,7 +148,7 @@ func sendBlobTreeEntry( _, err = io.CopyN(sw, blobObj, readLimit) if err != nil { - return status.Errorf(codes.Unavailable, "GetBlobs: send: %v", err) + return helper.ErrUnavailablef("send: %w", err) } return nil @@ -154,20 +156,20 @@ func sendBlobTreeEntry( func (s *server) GetBlobs(req *gitalypb.GetBlobsRequest, stream gitalypb.BlobService_GetBlobsServer) error { if err := validateGetBlobsRequest(req); err != nil { - return err + return helper.ErrInvalidArgument(err) } repo := s.localrepo(req.GetRepository()) objectReader, cancel, err := s.catfileCache.ObjectReader(stream.Context(), repo) if err != nil { - return err + return helper.ErrInternal(fmt.Errorf("creating object reader: %w", err)) } defer cancel() objectInfoReader, cancel, err := s.catfileCache.ObjectInfoReader(stream.Context(), repo) if err != nil { - return err + return helper.ErrInternal(fmt.Errorf("creating object info reader: %w", err)) } defer cancel() @@ -176,16 +178,16 @@ func (s *server) GetBlobs(req *gitalypb.GetBlobsRequest, stream gitalypb.BlobSer func validateGetBlobsRequest(req *gitalypb.GetBlobsRequest) error { if req.Repository == nil { - return status.Errorf(codes.InvalidArgument, "GetBlobs: empty Repository") + return gitalyerrors.ErrEmptyRepository } if len(req.RevisionPaths) == 0 { - return status.Errorf(codes.InvalidArgument, "GetBlobs: empty RevisionPaths") + return errors.New("empty RevisionPaths") } for _, rp := range req.RevisionPaths { if err := git.ValidateRevision([]byte(rp.Revision)); err != nil { - return status.Errorf(codes.InvalidArgument, "GetBlobs: %v", err) + return err } } diff --git a/internal/gitaly/service/blob/lfs_pointers.go b/internal/gitaly/service/blob/lfs_pointers.go index 9c66782fb..88b7870ec 100644 --- a/internal/gitaly/service/blob/lfs_pointers.go +++ b/internal/gitaly/service/blob/lfs_pointers.go @@ -13,8 +13,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/helper" "gitlab.com/gitlab-org/gitaly/v15/internal/helper/chunk" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" "google.golang.org/protobuf/proto" ) @@ -31,14 +29,14 @@ func (s *server) ListLFSPointers(in *gitalypb.ListLFSPointersRequest, stream git ctx := stream.Context() if in.GetRepository() == nil { - return status.Error(codes.InvalidArgument, "empty repository") + return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) } if len(in.Revisions) == 0 { - return status.Error(codes.InvalidArgument, "missing revisions") + return helper.ErrInvalidArgumentf("missing revisions") } for _, revision := range in.Revisions { if strings.HasPrefix(revision, "-") && revision != "--all" && revision != "--not" { - return status.Errorf(codes.InvalidArgument, "invalid revision: %q", revision) + return helper.ErrInvalidArgumentf("invalid revision: %q", revision) } } @@ -82,7 +80,7 @@ func (s *server) ListAllLFSPointers(in *gitalypb.ListAllLFSPointersRequest, stre ctx := stream.Context() if in.GetRepository() == nil { - return status.Error(codes.InvalidArgument, "empty repository") + return helper.ErrInvalidArgument(gitalyerrors.ErrEmptyRepository) } repo := s.localrepo(in.GetRepository()) @@ -97,7 +95,7 @@ func (s *server) ListAllLFSPointers(in *gitalypb.ListAllLFSPointersRequest, stre objectReader, cancel, err := s.catfileCache.ObjectReader(ctx, repo) if err != nil { - return helper.ErrInternal(fmt.Errorf("creating object reader: %w", err)) + return helper.ErrInternalf("creating object reader: %w", err) } defer cancel() @@ -126,7 +124,7 @@ func (s *server) GetLFSPointers(req *gitalypb.GetLFSPointersRequest, stream gita ctx := stream.Context() if err := validateGetLFSPointersRequest(req); err != nil { - return status.Errorf(codes.InvalidArgument, "GetLFSPointers: %v", err) + return helper.ErrInvalidArgument(err) } repo := s.localrepo(req.GetRepository()) |