Revert "DEMO Error helper tests: show an actual behavior change in fmt v.s. non-fmt"avar/errors-improve-test-for-decorate-DEMO

This reverts commit 69aa55079fd2e1d2cfc348009682185fab441804.

2 files changed, 23 insertions, 28 deletions

diff --git a/internal/gitaly/service/operations/tags.go b/internal/gitaly/service/operations/tags.go
index 23433322c..b951cb035 100644
--- a/internal/gitaly/service/operations/tags.go
+++ b/internal/gitaly/service/operations/tags.go
@@ -8,7 +8,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/rubyserver"
 	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
-	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -37,28 +36,11 @@ func (s *Server) UserDeleteTagRuby(ctx context.Context, req *gitalypb.UserDelete
 
 func (s *Server) UserDeleteTagGo(ctx context.Context, req *gitalypb.UserDeleteTagRequest) (*gitalypb.UserDeleteTagResponse, error) {
 	if len(req.TagName) == 0 {
-		// This one gets a new Internal error code. Then try
-		// it with:
-		//
-		//     git checkout 495a384d4 -- internal/helper/error.go
-		//
-		// And it does not, but the message is mangled to:
-		//
-		//    rpc error: code = Internal desc = empty user
-		return nil, helper.ErrInvalidArgument(status.Error(codes.Internal, "empty tag name"))
+		return nil, status.Errorf(codes.InvalidArgument, "empty tag name")
 	}
 
 	if req.User == nil {
-		// This does not get a new error code, but the message
-		// is mangled to:
-		//
-		//    rpc error: code = Internal desc = empty user
-		//
-		//     git checkout 495a384d4 -- internal/helper/error.go
-		//
-		// And the message is the same, but we mangle the
-		// error code to InvalidArgument.
-		return nil, helper.ErrInvalidArgumentf("%w", status.Error(codes.Internal, "empty user"))
+		return nil, status.Errorf(codes.InvalidArgument, "empty user")
 	}
 
 	referenceName := fmt.Sprintf("refs/tags/%s", req.TagName)
diff --git a/internal/helper/error.go b/internal/helper/error.go
index db0f9703c..d8580a534 100644
--- a/internal/helper/error.go
+++ b/internal/helper/error.go
@@ -8,13 +8,26 @@ import (
 )
 
 // Unimplemented is a Go error with gRPC error code 'Unimplemented'
-var Unimplemented = status.Errorf(codes.Unimplemented, "this rpc is not implemented")
+var Unimplemented = status.Error(codes.Unimplemented, "this rpc is not implemented")
+
+type statusWrapper struct {
+	error
+	status *status.Status
+}
+
+func (sw statusWrapper) GRPCStatus() *status.Status {
+	return sw.status
+}
+
+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 {
-		return status.Errorf(code, "%v", err)
+		return statusWrapper{err, status.New(code, err.Error())}
 	}
 	return err
 }
@@ -22,25 +35,25 @@ func DecorateError(code codes.Code, err error) error {
 // 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
+// ErrInternalf wrapps a formatted error with codes.Internal, clobbering any existing grpc error
 func ErrInternalf(format string, a ...interface{}) error {
-	return DecorateError(codes.Internal, fmt.Errorf(format, a...))
+	return ErrInternal(fmt.Errorf(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
+// ErrInvalidArgumentf wraps a formatted error with codes.InvalidArgument, clobbering any existing grpc error
 func ErrInvalidArgumentf(format string, a ...interface{}) error {
-	return DecorateError(codes.InvalidArgument, fmt.Errorf(format, a...))
+	return ErrInvalidArgument(fmt.Errorf(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) }
 
-// ErrPreconditionFailedf wraps a formatted error with codes.FailedPrecondition, unless err is already a grpc error
+// ErrPreconditionFailedf wraps a formatted error with codes.FailedPrecondition, clobbering any existing grpc error
 func ErrPreconditionFailedf(format string, a ...interface{}) error {
-	return DecorateError(codes.FailedPrecondition, fmt.Errorf(format, a...))
+	return ErrPreconditionFailed(fmt.Errorf(format, a...))
 }
 
 // ErrNotFound wraps error with codes.NotFound, unless err is already a grpc error