Merge branch 'pks-helper-formatting-grpc-error-codes' into 'master'

helper: Improve gRPC error interfaces and retain error codes in formatting directives

See merge request gitlab-org/gitaly!3708

15 files changed, 141 insertions, 105 deletions

diff --git a/cmd/gitaly-git2go/conflicts/conflicts.go b/cmd/gitaly-git2go/conflicts/conflicts.go
index 625eb241d..c5a79f85e 100644
--- a/cmd/gitaly-git2go/conflicts/conflicts.go
+++ b/cmd/gitaly-git2go/conflicts/conflicts.go
@@ -55,7 +55,7 @@ func Merge(repo *git.Repository, conflict git.IndexConflict) (*git.MergeFileResu
 
 		blob, err := repo.LookupBlob(entry.Id)
 		if err != nil {
-			return nil, helper.ErrPreconditionFailedf("could not get conflicting blob: %w", err)
+			return nil, helper.ErrFailedPreconditionf("could not get conflicting blob: %w", err)
 		}
 
 		input.Path = entry.Path
diff --git a/internal/gitaly/service/blob/get_blob.go b/internal/gitaly/service/blob/get_blob.go
index 526a34b11..177a9756a 100644
--- a/internal/gitaly/service/blob/get_blob.go
+++ b/internal/gitaly/service/blob/get_blob.go
@@ -9,8 +9,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/v14/streamio"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
 )
 
 func (s *server) GetBlob(in *gitalypb.GetBlobRequest, stream gitalypb.BlobService_GetBlobServer) error {
@@ -19,20 +17,20 @@ func (s *server) GetBlob(in *gitalypb.GetBlobRequest, stream gitalypb.BlobServic
 	repo := s.localrepo(in.GetRepository())
 
 	if err := validateRequest(in); err != nil {
-		return status.Errorf(codes.InvalidArgument, "GetBlob: %v", err)
+		return helper.ErrInvalidArgumentf("GetBlob: %v", err)
 	}
 
 	c, err := s.catfileCache.BatchProcess(stream.Context(), repo)
 	if err != nil {
-		return status.Errorf(codes.Internal, "GetBlob: %v", err)
+		return helper.ErrInternalf("GetBlob: %v", err)
 	}
 
 	objectInfo, err := c.Info(ctx, git.Revision(in.Oid))
 	if err != nil && !catfile.IsNotFound(err) {
-		return status.Errorf(codes.Internal, "GetBlob: %v", err)
+		return helper.ErrInternalf("GetBlob: %v", err)
 	}
 	if catfile.IsNotFound(err) || objectInfo.Type != "blob" {
-		return helper.DecorateError(codes.Unavailable, stream.Send(&gitalypb.GetBlobResponse{}))
+		return helper.ErrUnavailable(stream.Send(&gitalypb.GetBlobResponse{}))
 	}
 
 	readLimit := objectInfo.Size
@@ -45,12 +43,12 @@ func (s *server) GetBlob(in *gitalypb.GetBlobRequest, stream gitalypb.BlobServic
 	}
 
 	if readLimit == 0 {
-		return helper.DecorateError(codes.Unavailable, stream.Send(firstMessage))
+		return helper.ErrUnavailable(stream.Send(firstMessage))
 	}
 
 	blobObj, err := c.Blob(ctx, git.Revision(objectInfo.Oid))
 	if err != nil {
-		return status.Errorf(codes.Internal, "GetBlob: %v", err)
+		return helper.ErrInternalf("GetBlob: %v", err)
 	}
 
 	sw := streamio.NewWriter(func(p []byte) error {
@@ -65,7 +63,7 @@ func (s *server) GetBlob(in *gitalypb.GetBlobRequest, stream gitalypb.BlobServic
 
 	_, err = io.CopyN(sw, blobObj.Reader, readLimit)
 	if err != nil {
-		return status.Errorf(codes.Unavailable, "GetBlob: send: %v", err)
+		return helper.ErrUnavailablef("GetBlob: send: %v", err)
 	}
 
 	return nil
diff --git a/internal/gitaly/service/commit/list_files.go b/internal/gitaly/service/commit/list_files.go
index 0ab072789..ba7688f0d 100644
--- a/internal/gitaly/service/commit/list_files.go
+++ b/internal/gitaly/service/commit/list_files.go
@@ -1,7 +1,6 @@
 package commit
 
 import (
-	"fmt"
 	"io"
 
 	"github.com/golang/protobuf/proto"
@@ -12,8 +11,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper/chunk"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
 )
 
 func (s *server) ListFiles(in *gitalypb.ListFilesRequest, stream gitalypb.CommitService_ListFilesServer) error {
@@ -34,11 +31,11 @@ func (s *server) ListFiles(in *gitalypb.ListFilesRequest, stream gitalypb.Commit
 	if len(revision) == 0 {
 		defaultBranch, err := defaultBranchName(stream.Context(), repo)
 		if err != nil {
-			return helper.DecorateError(codes.NotFound, fmt.Errorf("revision not found %q", revision))
+			return helper.ErrNotFoundf("revision not found %q", revision)
 		}
 
 		if len(defaultBranch) == 0 {
-			return status.Errorf(codes.FailedPrecondition, "repository does not have a default branch")
+			return helper.ErrFailedPreconditionf("repository does not have a default branch")
 		}
 
 		revision = string(defaultBranch)
diff --git a/internal/gitaly/service/commit/tree_entry.go b/internal/gitaly/service/commit/tree_entry.go
index 6d0ab14f6..aea2f9e18 100644
--- a/internal/gitaly/service/commit/tree_entry.go
+++ b/internal/gitaly/service/commit/tree_entry.go
@@ -10,8 +10,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/v14/streamio"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
 )
 
 func sendTreeEntry(stream gitalypb.CommitService_TreeEntryServer, c catfile.Batch, revision, path string, limit, maxSize int64) error {
@@ -23,7 +21,7 @@ func sendTreeEntry(stream gitalypb.CommitService_TreeEntryServer, c catfile.Batc
 	}
 
 	if treeEntry == nil || len(treeEntry.Oid) == 0 {
-		return status.Errorf(codes.NotFound, "not found: %s", path)
+		return helper.ErrNotFoundf("not found: %s", path)
 	}
 
 	if treeEntry.Type == gitalypb.TreeEntry_COMMIT {
@@ -33,7 +31,7 @@ func sendTreeEntry(stream gitalypb.CommitService_TreeEntryServer, c catfile.Batc
 			Oid:  treeEntry.Oid,
 		}
 		if err := stream.Send(response); err != nil {
-			return status.Errorf(codes.Unavailable, "TreeEntry: send: %v", err)
+			return helper.ErrUnavailablef("TreeEntry: send: %v", err)
 		}
 
 		return nil
@@ -51,17 +49,16 @@ func sendTreeEntry(stream gitalypb.CommitService_TreeEntryServer, c catfile.Batc
 			Size: treeInfo.Size,
 			Mode: treeEntry.Mode,
 		}
-		return helper.DecorateError(codes.Unavailable, stream.Send(response))
+		return helper.ErrUnavailable(stream.Send(response))
 	}
 
 	objectInfo, err := c.Info(ctx, git.Revision(treeEntry.Oid))
 	if err != nil {
-		return status.Errorf(codes.Internal, "TreeEntry: %v", err)
+		return helper.ErrInternalf("TreeEntry: %v", err)
 	}
 
 	if strings.ToLower(treeEntry.Type.String()) != objectInfo.Type {
-		return status.Errorf(
-			codes.Internal,
+		return helper.ErrInternalf(
 			"TreeEntry: mismatched object type: tree-oid=%s object-oid=%s entry-type=%s object-type=%s",
 			treeEntry.Oid, objectInfo.Oid, treeEntry.Type.String(), objectInfo.Type,
 		)
@@ -70,8 +67,7 @@ func sendTreeEntry(stream gitalypb.CommitService_TreeEntryServer, c catfile.Batc
 	dataLength := objectInfo.Size
 
 	if maxSize > 0 && dataLength > maxSize {
-		return status.Errorf(
-			codes.FailedPrecondition,
+		return helper.ErrFailedPreconditionf(
 			"TreeEntry: object size (%d) is bigger than the maximum allowed size (%d)",
 			dataLength, maxSize,
 		)
@@ -88,7 +84,7 @@ func sendTreeEntry(stream gitalypb.CommitService_TreeEntryServer, c catfile.Batc
 		Mode: treeEntry.Mode,
 	}
 	if dataLength == 0 {
-		return helper.DecorateError(codes.Unavailable, stream.Send(response))
+		return helper.ErrUnavailable(stream.Send(response))
 	}
 
 	blobObj, err := c.Blob(ctx, git.Revision(objectInfo.Oid))
@@ -100,7 +96,7 @@ func sendTreeEntry(stream gitalypb.CommitService_TreeEntryServer, c catfile.Batc
 		response.Data = p
 
 		if err := stream.Send(response); err != nil {
-			return status.Errorf(codes.Unavailable, "TreeEntry: send: %v", err)
+			return helper.ErrUnavailablef("TreeEntry: send: %v", err)
 		}
 
 		// Use a new response so we don't send other fields (Size, ...) over and over
@@ -115,7 +111,7 @@ func sendTreeEntry(stream gitalypb.CommitService_TreeEntryServer, c catfile.Batc
 
 func (s *server) TreeEntry(in *gitalypb.TreeEntryRequest, stream gitalypb.CommitService_TreeEntryServer) error {
 	if err := validateRequest(in); err != nil {
-		return status.Errorf(codes.InvalidArgument, "TreeEntry: %v", err)
+		return helper.ErrInvalidArgumentf("TreeEntry: %v", err)
 	}
 
 	repo := s.localrepo(in.GetRepository())
diff --git a/internal/gitaly/service/conflicts/list_conflict_files.go b/internal/gitaly/service/conflicts/list_conflict_files.go
index a64d38ff6..2fe1447dc 100644
--- a/internal/gitaly/service/conflicts/list_conflict_files.go
+++ b/internal/gitaly/service/conflicts/list_conflict_files.go
@@ -25,12 +25,12 @@ func (s *server) ListConflictFiles(request *gitalypb.ListConflictFilesRequest, s
 
 	ours, err := repo.ResolveRevision(ctx, git.Revision(request.OurCommitOid+"^{commit}"))
 	if err != nil {
-		return helper.ErrPreconditionFailedf("could not lookup 'our' OID: %s", err)
+		return helper.ErrFailedPreconditionf("could not lookup 'our' OID: %s", err)
 	}
 
 	theirs, err := repo.ResolveRevision(ctx, git.Revision(request.TheirCommitOid+"^{commit}"))
 	if err != nil {
-		return helper.ErrPreconditionFailedf("could not lookup 'their' OID: %s", err)
+		return helper.ErrFailedPreconditionf("could not lookup 'their' OID: %s", err)
 	}
 
 	repoPath, err := s.locator.GetPath(request.Repository)
@@ -55,11 +55,11 @@ func (s *server) ListConflictFiles(request *gitalypb.ListConflictFilesRequest, s
 
 	for _, conflict := range conflicts.Conflicts {
 		if !request.AllowTreeConflicts && (conflict.Their.Path == "" || conflict.Our.Path == "") {
-			return helper.ErrPreconditionFailedf("conflict side missing")
+			return helper.ErrFailedPreconditionf("conflict side missing")
 		}
 
 		if !utf8.Valid(conflict.Content) {
-			return helper.ErrPreconditionFailed(errors.New("unsupported encoding"))
+			return helper.ErrFailedPrecondition(errors.New("unsupported encoding"))
 		}
 
 		conflictFiles = append(conflictFiles, &gitalypb.ConflictFile{
diff --git a/internal/gitaly/service/operations/merge.go b/internal/gitaly/service/operations/merge.go
index c56c76920..846c578e5 100644
--- a/internal/gitaly/service/operations/merge.go
+++ b/internal/gitaly/service/operations/merge.go
@@ -97,7 +97,7 @@ func (s *Server) UserMergeBranch(stream gitalypb.OperationService_UserMergeBranc
 		return err
 	}
 	if !secondRequest.Apply {
-		return helper.ErrPreconditionFailedf("merge aborted by client")
+		return helper.ErrFailedPreconditionf("merge aborted by client")
 	}
 
 	if err := s.updateReferenceWithHooks(ctx, firstRequest.Repository, firstRequest.User, nil, referenceName, mergeOID, revision); err != nil {
@@ -170,7 +170,7 @@ func (s *Server) UserFFBranch(ctx context.Context, in *gitalypb.UserFFBranchRequ
 		return nil, err
 	}
 	if !ancestor {
-		return nil, helper.ErrPreconditionFailedf("not fast forward")
+		return nil, helper.ErrFailedPreconditionf("not fast forward")
 	}
 
 	if err := s.updateReferenceWithHooks(ctx, in.Repository, in.User, nil, referenceName, commitID, revision); err != nil {
@@ -266,7 +266,7 @@ func (s *Server) UserMergeToRef(ctx context.Context, request *gitalypb.UserMerge
 	var oldTargetOID git.ObjectID
 	if targetRef, err := repo.GetReference(ctx, git.ReferenceName(request.TargetRef)); err == nil {
 		if targetRef.IsSymbolic {
-			return nil, helper.ErrPreconditionFailedf("target reference is symbolic: %q", request.TargetRef)
+			return nil, helper.ErrFailedPreconditionf("target reference is symbolic: %q", request.TargetRef)
 		}
 
 		oid, err := git.NewObjectIDFromHex(targetRef.Target)
@@ -304,7 +304,7 @@ func (s *Server) UserMergeToRef(ctx context.Context, request *gitalypb.UserMerge
 		if errors.Is(err, git2go.ErrInvalidArgument) {
 			return nil, helper.ErrInvalidArgument(err)
 		}
-		return nil, helper.ErrPreconditionFailedf("Failed to create merge commit for source_sha %s and target_sha %s at %s",
+		return nil, helper.ErrFailedPreconditionf("Failed to create merge commit for source_sha %s and target_sha %s at %s",
 			sourceOID, oid, string(request.TargetRef))
 	}
 
@@ -316,8 +316,7 @@ func (s *Server) UserMergeToRef(ctx context.Context, request *gitalypb.UserMerge
 	// ... and move branch from target ref to the merge commit. The Ruby
 	// implementation doesn't invoke hooks, so we don't either.
 	if err := repo.UpdateRef(ctx, git.ReferenceName(request.TargetRef), mergeOID, oldTargetOID); err != nil {
-		//nolint:stylecheck
-		return nil, helper.ErrPreconditionFailed(fmt.Errorf("Could not update %s. Please refresh and try again", string(request.TargetRef)))
+		return nil, helper.ErrFailedPreconditionf("Could not update %s. Please refresh and try again", string(request.TargetRef))
 	}
 
 	return &gitalypb.UserMergeToRefResponse{
diff --git a/internal/gitaly/service/operations/rebase.go b/internal/gitaly/service/operations/rebase.go
index a3fde32a8..af530206e 100644
--- a/internal/gitaly/service/operations/rebase.go
+++ b/internal/gitaly/service/operations/rebase.go
@@ -86,7 +86,7 @@ func (s *Server) UserRebaseConfirmable(stream gitalypb.OperationService_UserReba
 	}
 
 	if !secondRequest.GetApply() {
-		return helper.ErrPreconditionFailedf("rebase aborted by client")
+		return helper.ErrFailedPreconditionf("rebase aborted by client")
 	}
 
 	if err := s.updateReferenceWithHooks(
diff --git a/internal/gitaly/service/repository/archive.go b/internal/gitaly/service/repository/archive.go
index 2f0197e72..51e1d4394 100644
--- a/internal/gitaly/service/repository/archive.go
+++ b/internal/gitaly/service/repository/archive.go
@@ -151,7 +151,7 @@ func (s *server) validateGetArchivePrecondition(
 		if ok, err := findGetArchivePath(ctx, f, commitID, path); err != nil {
 			return err
 		} else if !ok {
-			return helper.ErrPreconditionFailedf("path doesn't exist")
+			return helper.ErrFailedPreconditionf("path doesn't exist")
 		}
 	}
 
@@ -159,7 +159,7 @@ func (s *server) validateGetArchivePrecondition(
 		if ok, err := findGetArchivePath(ctx, f, commitID, exclude); err != nil {
 			return err
 		} else if !ok {
-			return helper.ErrPreconditionFailedf("exclude[%d] doesn't exist", i)
+			return helper.ErrFailedPreconditionf("exclude[%d] doesn't exist", i)
 		}
 	}
 
diff --git a/internal/gitaly/service/repository/create_from_bundle.go b/internal/gitaly/service/repository/create_from_bundle.go
index 5fa6d7e62..e5005d711 100644
--- a/internal/gitaly/service/repository/create_from_bundle.go
+++ b/internal/gitaly/service/repository/create_from_bundle.go
@@ -2,7 +2,6 @@ package repository
 
 import (
 	"bytes"
-	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -36,7 +35,7 @@ func (s *server) CreateRepositoryFromBundle(stream gitalypb.RepositoryService_Cr
 	}
 
 	if !isDirEmpty(repoPath) {
-		return helper.ErrPreconditionFailed(errors.New("CreateRepositoryFromBundle: target directory is non-empty"))
+		return helper.ErrFailedPreconditionf("CreateRepositoryFromBundle: target directory is non-empty")
 	}
 
 	firstRead := false
diff --git a/internal/gitaly/service/repository/rename.go b/internal/gitaly/service/repository/rename.go
index f76507dd5..2557cbc2b 100644
--- a/internal/gitaly/service/repository/rename.go
+++ b/internal/gitaly/service/repository/rename.go
@@ -26,7 +26,7 @@ func (s *server) RenameRepository(ctx context.Context, in *gitalypb.RenameReposi
 	}
 
 	if _, err = os.Stat(toFullPath); !os.IsNotExist(err) {
-		return nil, helper.ErrPreconditionFailed(errors.New("destination already exists"))
+		return nil, helper.ErrFailedPreconditionf("destination already exists")
 	}
 
 	if err = os.MkdirAll(filepath.Dir(toFullPath), 0755); err != nil {
diff --git a/internal/gitaly/service/repository/search_files.go b/internal/gitaly/service/repository/search_files.go
index a95e7d9ca..2eb2d5b9e 100644
--- a/internal/gitaly/service/repository/search_files.go
+++ b/internal/gitaly/service/repository/search_files.go
@@ -14,8 +14,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/helper/lines"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/v14/streamio"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
 )
 
 const (
@@ -30,12 +28,12 @@ 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 helper.ErrInvalidArgument(err)
 	}
 
 	repo := req.GetRepository()
 	if repo == nil {
-		return status.Errorf(codes.InvalidArgument, "SearchFilesByContent: empty Repository")
+		return helper.ErrInvalidArgumentf("SearchFilesByContent: empty Repository")
 	}
 
 	ctx := stream.Context()
@@ -51,11 +49,11 @@ func (s *server) SearchFilesByContent(req *gitalypb.SearchFilesByContentRequest,
 			git.Flag{Name: "-e"}}, Args: []string{req.GetQuery(), string(req.GetRef())}})
 
 	if err != nil {
-		return status.Errorf(codes.Internal, "SearchFilesByContent: cmd start failed: %v", err)
+		return helper.ErrInternalf("SearchFilesByContent: cmd start failed: %v", err)
 	}
 
 	if err = sendSearchFilesResultChunked(cmd, stream); err != nil {
-		return status.Errorf(codes.Internal, "SearchFilesByContent: sending chunked response failed: %v", err)
+		return helper.ErrInternalf("SearchFilesByContent: sending chunked response failed: %v", err)
 	}
 
 	return nil
@@ -105,7 +103,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 helper.ErrInvalidArgument(err)
 	}
 
 	var filter *regexp.Regexp
@@ -122,7 +120,7 @@ func (s *server) SearchFilesByName(req *gitalypb.SearchFilesByNameRequest, strea
 
 	repo := req.GetRepository()
 	if repo == nil {
-		return status.Errorf(codes.InvalidArgument, "SearchFilesByName: empty Repository")
+		return helper.ErrInvalidArgumentf("SearchFilesByName: empty Repository")
 	}
 
 	ctx := stream.Context()
@@ -134,7 +132,7 @@ func (s *server) SearchFilesByName(req *gitalypb.SearchFilesByNameRequest, strea
 			git.Flag{Name: "--name-status"},
 			git.Flag{Name: "-r"}}, Args: []string{string(req.GetRef()), req.GetQuery()}})
 	if err != nil {
-		return status.Errorf(codes.Internal, "SearchFilesByName: cmd start failed: %v", err)
+		return helper.ErrInternalf("SearchFilesByName: cmd start failed: %v", err)
 	}
 
 	lr := func(objs [][]byte) error {
diff --git a/internal/helper/error.go b/internal/helper/error.go
index 53d6fe208..c42029266 100644
--- a/internal/helper/error.go
+++ b/internal/helper/error.go
@@ -1,6 +1,7 @@
 package helper
 
 import (
+	"errors"
 	"fmt"
 
 	"google.golang.org/grpc/codes"
@@ -20,41 +21,77 @@ func (sw statusWrapper) Unwrap() error {
 	return sw.error
 }
 
-// DecorateError unless it's already a grpc error.
-//  If given nil it will return nil.
-func DecorateError(code codes.Code, err error) error {
-	if err != nil && GrpcCode(err) == codes.Unknown {
+// ErrCanceled wraps err with codes.Canceled, unless err is already a gRPC error.
+func ErrCanceled(err error) error { return wrapError(codes.Canceled, err) }
+
+// ErrInternal wraps err with codes.Internal, unless err is already a gRPC error.
+func ErrInternal(err error) error { return wrapError(codes.Internal, err) }
+
+// ErrInvalidArgument wraps err with codes.InvalidArgument, unless err is already a gRPC error.
+func ErrInvalidArgument(err error) error { return wrapError(codes.InvalidArgument, err) }
+
+// ErrNotFound wraps error with codes.NotFound, unless err is already a gRPC error.
+func ErrNotFound(err error) error { return wrapError(codes.NotFound, err) }
+
+// ErrFailedPrecondition wraps err with codes.FailedPrecondition, unless err is already a gRPC
+// error.
+func ErrFailedPrecondition(err error) error { return wrapError(codes.FailedPrecondition, err) }
+
+// ErrUnavailable wraps err with codes.Unavailable, unless err is already a gRPC error.
+func ErrUnavailable(err error) error { return wrapError(codes.Unavailable, err) }
+
+// wrapError wraps the given error with the error code unless it's already a gRPC error. If given
+// nil it will return nil.
+func wrapError(code codes.Code, err error) error {
+	if GrpcCode(err) == codes.Unknown {
 		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, clobbering any existing grpc error
+// ErrInternalf wraps a formatted error with codes.Internal, unless the formatted error is a
+// wrapped gRPC error.
 func ErrInternalf(format string, a ...interface{}) error {
-	return ErrInternal(fmt.Errorf(format, a...))
+	return formatError(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, clobbering any existing grpc error
+// ErrInvalidArgumentf wraps a formatted error with codes.InvalidArgument, unless the formatted
+// error is a wrapped gRPC error.
 func ErrInvalidArgumentf(format string, a ...interface{}) error {
-	return ErrInvalidArgument(fmt.Errorf(format, a...))
+	return formatError(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) }
+// ErrNotFoundf wraps a formatted error with codes.NotFound, unless the
+// formatted error is a wrapped gRPC error.
+func ErrNotFoundf(format string, a ...interface{}) error {
+	return formatError(codes.NotFound, format, a...)
+}
+
+// ErrFailedPreconditionf wraps a formatted error with codes.FailedPrecondition, unless the
+// formatted error is a wrapped gRPC error.
+func ErrFailedPreconditionf(format string, a ...interface{}) error {
+	return formatError(codes.FailedPrecondition, format, a...)
+}
 
-// ErrPreconditionFailedf wraps a formatted error with codes.FailedPrecondition, clobbering any existing grpc error
-func ErrPreconditionFailedf(format string, a ...interface{}) error {
-	return ErrPreconditionFailed(fmt.Errorf(format, a...))
+// ErrUnavailablef wraps a formatted error with codes.Unavailable, unless the
+// formatted error is a wrapped gRPC error.
+func ErrUnavailablef(format string, a ...interface{}) error {
+	return formatError(codes.Unavailable, 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) }
+// formatError will create a new error from the given format string. If the error string contains a
+// %w verb and its corresponding error has a gRPC error code, then the returned error will keep this
+// gRPC error code instead of using the one provided as an argument.
+func formatError(code codes.Code, format string, a ...interface{}) error {
+	err := fmt.Errorf(format, a...)
+
+	nestedCode := GrpcCode(errors.Unwrap(err))
+	if nestedCode != codes.OK && nestedCode != codes.Unknown {
+		code = nestedCode
+	}
+
+	return statusWrapper{err, status.New(code, err.Error())}
+}
 
 // GrpcCode emulates the old grpc.Code function: it translates errors into codes.Code values.
 func GrpcCode(err error) codes.Code {
diff --git a/internal/helper/errors_test.go b/internal/helper/error_test.go
index 7927d41f6..614b7b8b6 100644
--- a/internal/helper/errors_test.go
+++ b/internal/helper/error_test.go
@@ -23,6 +23,11 @@ func TestError(t *testing.T) {
 		code   codes.Code
 	}{
 		{
+			desc:   "Canceled",
+			errorf: ErrCanceled,
+			code:   codes.Canceled,
+		},
+		{
 			desc:   "Internal",
 			errorf: ErrInternal,
 			code:   codes.Internal,
@@ -33,8 +38,8 @@ func TestError(t *testing.T) {
 			code:   codes.InvalidArgument,
 		},
 		{
-			desc:   "PreconditionFailed",
-			errorf: ErrPreconditionFailed,
+			desc:   "FailedPrecondition",
+			errorf: ErrFailedPrecondition,
 			code:   codes.FailedPrecondition,
 		},
 		{
@@ -42,6 +47,11 @@ func TestError(t *testing.T) {
 			errorf: ErrNotFound,
 			code:   codes.NotFound,
 		},
+		{
+			desc:   "Unavailable",
+			errorf: ErrUnavailable,
+			code:   codes.Unavailable,
+		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
 			// tc.code and our canary test code must not
@@ -77,13 +87,6 @@ func TestErrorF_withWFormat(t *testing.T) {
 	testErrorfFormat(t, "expected %w", "expected %s")
 }
 
-// oldPreconditionFailedf shows ErrPreconditionFailedf looked like
-// before 777a12cfd. We're testing the nature of a regression in that
-// change.
-func oldPreconditionFailedf(format string, a ...interface{}) error {
-	return DecorateError(codes.FailedPrecondition, fmt.Errorf(format, a...))
-}
-
 func testErrorfFormat(t *testing.T, errorFormat, errorFormatEqual string) {
 	isFormatW := strings.Contains(errorFormat, "%w")
 	errorMessage := "sentinel error"
@@ -108,43 +111,53 @@ func testErrorfFormat(t *testing.T, errorFormat, errorFormatEqual string) {
 			code:   codes.InvalidArgument,
 		},
 		{
-			desc:   "PreconditionFailedf",
-			errorf: ErrPreconditionFailedf,
+			desc:   "FailedPreconditionf",
+			errorf: ErrFailedPreconditionf,
 			code:   codes.FailedPrecondition,
 		},
 		{
-			desc:   "oldPreconditionFailedf",
-			errorf: oldPreconditionFailedf,
-			code:   codes.FailedPrecondition,
+			desc:   "NotFoundf",
+			errorf: ErrNotFoundf,
+			code:   codes.NotFound,
+		},
+		{
+			desc:   "ErrUnavailablef",
+			errorf: ErrUnavailablef,
+			code:   codes.Unavailable,
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
-			// tc.code and our canary test code must not
-			// clash!
-			require.NotEqual(t, tc.code, inputGRPCCode)
+			require.NotEqual(t, tc.code, inputGRPCCode, "canary test code and tc.code may not be the same")
 
-			// When not re-throwing an error we get the
-			// GRPC error code corresponding to the
-			// function's name. Just like the non-f functions.
+			// When not re-throwing an error we get the GRPC error code corresponding to
+			// the function's name. Just like the non-f functions.
 			err := tc.errorf(errorFormat, input)
 			require.EqualError(t, err, fmt.Sprintf(errorFormatEqual, errorMessage))
 			require.False(t, errors.Is(err, inputGRPC))
 			require.Equal(t, tc.code, status.Code(err))
 
-			// When re-throwing an error an existing GRPC
-			// error code will get clobbered, not the one
-			// corresponding to the function's name. This
-			// is unlike the non-f functions.
+			// When wrapping an existing gRPC error, then the error code will stay the
+			// same.
 			err = tc.errorf(errorFormat, inputGRPCFmt)
 			require.False(t, errors.Is(err, input))
-			require.Equal(t, tc.code, status.Code(err))
+			if isFormatW {
+				require.Equal(t, inputGRPCCode, status.Code(err))
+			} else {
+				require.Equal(t, tc.code, status.Code(err))
+			}
 			require.NotEqual(t, tc.code, status.Code(inputGRPC))
 
+			// The same as above, except that we test with an error returned by
+			// `status.Error()`.
 			errX := tc.errorf(errorFormat, inputGRPC)
 			require.Equal(t, errors.Is(errX, inputGRPC), isFormatW) // .True() for non-f
 			require.False(t, errors.Is(errX, input))
-			require.NotEqual(t, inputGRPCCode, status.Code(errX)) // .Equal() for non-f
-			require.NotEqual(t, tc.code, status.Code(inputGRPC))
+			if isFormatW {
+				require.Equal(t, inputGRPCCode, status.Code(errX))
+			} else {
+				require.Equal(t, tc.code, status.Code(errX))
+			}
+			require.Equal(t, inputGRPCCode, status.Code(inputGRPC))
 		})
 	}
 }
diff --git a/internal/praefect/coordinator.go b/internal/praefect/coordinator.go
index 108f1e926..67e5461d7 100644
--- a/internal/praefect/coordinator.go
+++ b/internal/praefect/coordinator.go
@@ -32,7 +32,7 @@ import (
 
 // ErrRepositoryReadOnly is returned when the repository is in read-only mode. This happens
 // if the primary does not have the latest changes.
-var ErrRepositoryReadOnly = helper.ErrPreconditionFailedf("repository is in read-only mode")
+var ErrRepositoryReadOnly = helper.ErrFailedPreconditionf("repository is in read-only mode")
 
 type transactionsCondition func(context.Context) bool
 
diff --git a/internal/praefect/service/transaction/server.go b/internal/praefect/service/transaction/server.go
index 086449532..5de3f3599 100644
--- a/internal/praefect/service/transaction/server.go
+++ b/internal/praefect/service/transaction/server.go
@@ -8,7 +8,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v14/internal/praefect/transactions"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/transaction/voting"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
-	"google.golang.org/grpc/codes"
 )
 
 type Server struct {
@@ -36,7 +35,7 @@ func (s *Server) VoteTransaction(ctx context.Context, in *gitalypb.VoteTransacti
 		case errors.Is(err, transactions.ErrNotFound):
 			return nil, helper.ErrNotFound(err)
 		case errors.Is(err, transactions.ErrTransactionCanceled):
-			return nil, helper.DecorateError(codes.Canceled, err)
+			return nil, helper.ErrCanceled(err)
 		case errors.Is(err, transactions.ErrTransactionStopped):
 			return &gitalypb.VoteTransactionResponse{
 				State: gitalypb.VoteTransactionResponse_STOP,
@@ -65,7 +64,7 @@ func (s *Server) StopTransaction(ctx context.Context, in *gitalypb.StopTransacti
 		case errors.Is(err, transactions.ErrNotFound):
 			return nil, helper.ErrNotFound(err)
 		case errors.Is(err, transactions.ErrTransactionCanceled):
-			return nil, helper.DecorateError(codes.Canceled, err)
+			return nil, helper.ErrCanceled(err)
 		case errors.Is(err, transactions.ErrTransactionStopped):
 			return &gitalypb.StopTransactionResponse{}, nil
 		default: