Merge branch 'pks-introduce-structerr' into 'master'

Introduce new `structerr` package

See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5099

Merged-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: Toon Claes <toon@gitlab.com>
Approved-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>

11 files changed, 912 insertions, 50 deletions

diff --git a/internal/gitaly/server/server.go b/internal/gitaly/server/server.go
index c0c99ca8c..fe1b87891 100644
--- a/internal/gitaly/server/server.go
+++ b/internal/gitaly/server/server.go
@@ -29,6 +29,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/middleware/sentryhandler"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/middleware/statushandler"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/praefect/protoregistry"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/structerr"
 	grpccorrelation "gitlab.com/gitlab-org/labkit/correlation/grpc"
 	grpctracing "gitlab.com/gitlab-org/labkit/tracing/grpc"
 	"google.golang.org/grpc"
@@ -94,6 +95,7 @@ func New(
 			commandstatshandler.FieldsProducer,
 			grpcstats.FieldsProducer,
 			featureflag.FieldsProducer,
+			structerr.FieldsProducer,
 		),
 	)
 
diff --git a/internal/gitaly/service/commit/check_objects_exist.go b/internal/gitaly/service/commit/check_objects_exist.go
index 8a557df8b..2d56448fb 100644
--- a/internal/gitaly/service/commit/check_objects_exist.go
+++ b/internal/gitaly/service/commit/check_objects_exist.go
@@ -9,6 +9,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/service"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/helper/chunk"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/structerr"
 	"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
 	"google.golang.org/protobuf/proto"
 )
@@ -49,7 +50,8 @@ func (s *server) CheckObjectsExist(
 		// so we only fetch the next request at the end of this loop.
 		for _, revision := range request.GetRevisions() {
 			if err := git.ValidateRevision(revision); err != nil {
-				return helper.ErrInvalidArgumentf("invalid revision %q: %w", revision, err)
+				return structerr.NewInvalidArgument("invalid revision: %w", err).
+					WithMetadata("revision", string(revision))
 			}
 		}
 
diff --git a/internal/gitaly/service/commit/check_objects_exist_test.go b/internal/gitaly/service/commit/check_objects_exist_test.go
index dd6fb6320..91a142164 100644
--- a/internal/gitaly/service/commit/check_objects_exist_test.go
+++ b/internal/gitaly/service/commit/check_objects_exist_test.go
@@ -7,10 +7,12 @@ import (
 	"io"
 	"testing"
 
+	"github.com/sirupsen/logrus/hooks/test"
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/gittest"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testserver"
 	"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -20,7 +22,8 @@ func TestCheckObjectsExist(t *testing.T) {
 	t.Parallel()
 
 	ctx := testhelper.Context(t)
-	cfg, client := setupCommitService(t, ctx)
+	logger, hook := test.NewNullLogger()
+	cfg, client := setupCommitService(t, ctx, testserver.WithLogger(logger))
 
 	repo, repoPath := gittest.CreateRepository(t, ctx, cfg)
 
@@ -35,10 +38,11 @@ func TestCheckObjectsExist(t *testing.T) {
 	)
 
 	for _, tc := range []struct {
-		desc            string
-		requests        []*gitalypb.CheckObjectsExistRequest
-		expectedResults map[string]bool
-		expectedErr     error
+		desc                  string
+		requests              []*gitalypb.CheckObjectsExistRequest
+		expectedResults       map[string]bool
+		expectedErr           error
+		expectedErrorMetadata any
 	}{
 		{
 			desc:     "no repository provided",
@@ -166,7 +170,10 @@ func TestCheckObjectsExist(t *testing.T) {
 					},
 				},
 			},
-			expectedErr: helper.ErrInvalidArgumentf("invalid revision %q: revision can't start with '-'", "-not-a-rev"),
+			expectedErr: helper.ErrInvalidArgumentf("invalid revision: revision can't start with '-'"),
+			expectedErrorMetadata: map[string]any{
+				"revision": "-not-a-rev",
+			},
 		},
 		{
 			desc: "input with whitespace",
@@ -178,7 +185,10 @@ func TestCheckObjectsExist(t *testing.T) {
 					},
 				},
 			},
-			expectedErr: helper.ErrInvalidArgumentf("invalid revision %q: revision can't contain whitespace", fmt.Sprintf("%s\n%s", commitID1, commitID2)),
+			expectedErr: helper.ErrInvalidArgumentf("invalid revision: revision can't contain whitespace"),
+			expectedErrorMetadata: map[string]any{
+				"revision": fmt.Sprintf("%s\n%s", commitID1, commitID2),
+			},
 		},
 		{
 			desc: "chunked invalid input",
@@ -195,10 +205,15 @@ func TestCheckObjectsExist(t *testing.T) {
 					},
 				},
 			},
-			expectedErr: helper.ErrInvalidArgumentf("invalid revision %q: revision can't start with '-'", "-not-a-rev"),
+			expectedErr: helper.ErrInvalidArgumentf("invalid revision: revision can't start with '-'"),
+			expectedErrorMetadata: map[string]any{
+				"revision": "-not-a-rev",
+			},
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
+			hook.Reset()
+
 			client, err := client.CheckObjectsExist(ctx)
 			require.NoError(t, err)
 
@@ -229,6 +244,23 @@ func TestCheckObjectsExist(t *testing.T) {
 
 			testhelper.RequireGrpcError(t, tc.expectedErr, err)
 			require.Equal(t, tc.expectedResults, results)
+
+			if tc.expectedErrorMetadata == nil {
+				tc.expectedErrorMetadata = map[string]any{}
+			}
+
+			metadata := map[string]any{}
+			for _, entry := range hook.AllEntries() {
+				errorMetadata, ok := entry.Data["error.metadata"]
+				if !ok {
+					continue
+				}
+
+				for key, value := range errorMetadata.(map[string]any) {
+					metadata[key] = value
+				}
+			}
+			require.Equal(t, tc.expectedErrorMetadata, metadata)
 		})
 	}
 }
diff --git a/internal/gitaly/service/commit/testhelper_test.go b/internal/gitaly/service/commit/testhelper_test.go
index 2adeda0a7..a2276cebe 100644
--- a/internal/gitaly/service/commit/testhelper_test.go
+++ b/internal/gitaly/service/commit/testhelper_test.go
@@ -28,33 +28,40 @@ func TestMain(m *testing.M) {
 }
 
 // setupCommitService makes a basic configuration and starts the service with the client.
-func setupCommitService(tb testing.TB, ctx context.Context) (config.Cfg, gitalypb.CommitServiceClient) {
+func setupCommitService(
+	tb testing.TB,
+	ctx context.Context,
+	opts ...testserver.GitalyServerOpt,
+) (config.Cfg, gitalypb.CommitServiceClient) {
 	cfg, _, _, client := setupCommitServiceCreateRepo(tb, ctx, func(tb testing.TB, ctx context.Context, cfg config.Cfg) (*gitalypb.Repository, string) {
 		return nil, ""
-	})
+	}, opts...)
 	return cfg, client
 }
 
 // setupCommitServiceWithRepo makes a basic configuration, creates a test repository and starts the service with the client.
 func setupCommitServiceWithRepo(
-	tb testing.TB, ctx context.Context,
+	tb testing.TB,
+	ctx context.Context,
+	opts ...testserver.GitalyServerOpt,
 ) (config.Cfg, *gitalypb.Repository, string, gitalypb.CommitServiceClient) {
 	return setupCommitServiceCreateRepo(tb, ctx, func(tb testing.TB, ctx context.Context, cfg config.Cfg) (*gitalypb.Repository, string) {
 		repo, repoPath := gittest.CreateRepository(tb, ctx, cfg, gittest.CreateRepositoryConfig{
 			Seed: gittest.SeedGitLabTest,
 		})
 		return repo, repoPath
-	})
+	}, opts...)
 }
 
 func setupCommitServiceCreateRepo(
 	tb testing.TB,
 	ctx context.Context,
 	createRepo func(testing.TB, context.Context, config.Cfg) (*gitalypb.Repository, string),
+	opts ...testserver.GitalyServerOpt,
 ) (config.Cfg, *gitalypb.Repository, string, gitalypb.CommitServiceClient) {
 	cfg := testcfg.Build(tb)
 
-	cfg.SocketPath = startTestServices(tb, cfg)
+	cfg.SocketPath = startTestServices(tb, cfg, opts...)
 	client := newCommitServiceClient(tb, cfg.SocketPath)
 
 	repo, repoPath := createRepo(tb, ctx, cfg)
@@ -62,7 +69,7 @@ func setupCommitServiceCreateRepo(
 	return cfg, repo, repoPath, client
 }
 
-func startTestServices(tb testing.TB, cfg config.Cfg) string {
+func startTestServices(tb testing.TB, cfg config.Cfg, opts ...testserver.GitalyServerOpt) string {
 	tb.Helper()
 	return testserver.RunGitalyServer(tb, cfg, nil, func(srv *grpc.Server, deps *service.Dependencies) {
 		gitalypb.RegisterCommitServiceServer(srv, NewServer(
@@ -82,7 +89,7 @@ func startTestServices(tb testing.TB, cfg config.Cfg) string {
 			deps.GetGit2goExecutor(),
 			deps.GetHousekeepingManager(),
 		))
-	})
+	}, opts...)
 }
 
 func newCommitServiceClient(tb testing.TB, serviceSocketPath string) gitalypb.CommitServiceClient {
diff --git a/internal/middleware/limithandler/concurrency_limiter.go b/internal/middleware/limithandler/concurrency_limiter.go
index 3bffe7d04..adda2a935 100644
--- a/internal/middleware/limithandler/concurrency_limiter.go
+++ b/internal/middleware/limithandler/concurrency_limiter.go
@@ -11,6 +11,7 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/helper"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/structerr"
 	"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
 	"google.golang.org/protobuf/types/known/durationpb"
 )
@@ -149,22 +150,12 @@ func (c *ConcurrencyLimiter) Limit(ctx context.Context, lockKey string, f Limite
 	log := ctxlogrus.Extract(ctx).WithField("limiting_key", lockKey)
 	if err := c.queueInc(ctx); err != nil {
 		if errors.Is(err, ErrMaxQueueSize) {
-			detailedErr, errGeneratingDetailedErr := helper.ErrWithDetails(
-				helper.ErrResourceExhausted(ErrMaxQueueSize),
+			return nil, structerr.NewResourceExhausted("%w", ErrMaxQueueSize).WithDetail(
 				&gitalypb.LimitError{
 					ErrorMessage: err.Error(),
 					RetryAfter:   durationpb.New(0),
 				},
 			)
-			if errGeneratingDetailedErr != nil {
-				log.WithField("max_queue_size_error", err).
-					WithError(errGeneratingDetailedErr).
-					Error("failed to generate detailed error")
-
-				return nil, helper.ErrResourceExhausted(ErrMaxQueueSize)
-			}
-
-			return nil, detailedErr
 		}
 
 		log.WithError(err).Error("unexpected error when queueing request")
@@ -186,22 +177,10 @@ func (c *ConcurrencyLimiter) Limit(ctx context.Context, lockKey string, f Limite
 		if errors.Is(err, ErrMaxQueueTime) {
 			c.monitor.Dropped(ctx, "max_time")
 
-			detailedErr, errGeneratingDetailedErr := helper.ErrWithDetails(
-				helper.ErrResourceExhausted(ErrMaxQueueTime),
-				&gitalypb.LimitError{
-					ErrorMessage: err.Error(),
-					RetryAfter:   durationpb.New(0),
-				},
-			)
-			if errGeneratingDetailedErr != nil {
-				log.WithField("max_queue_wait_error", err).
-					WithError(errGeneratingDetailedErr).
-					Error("failed to generate detailed error")
-
-				return nil, helper.ErrResourceExhausted(ErrMaxQueueTime)
-			}
-
-			return nil, detailedErr
+			return nil, structerr.NewResourceExhausted("%w", ErrMaxQueueTime).WithDetail(&gitalypb.LimitError{
+				ErrorMessage: err.Error(),
+				RetryAfter:   durationpb.New(0),
+			})
 		}
 
 		log.WithError(err).Error("unexpected error when dequeueing request")
diff --git a/internal/middleware/limithandler/concurrency_limiter_test.go b/internal/middleware/limithandler/concurrency_limiter_test.go
index 86d37902f..b1cb031ca 100644
--- a/internal/middleware/limithandler/concurrency_limiter_test.go
+++ b/internal/middleware/limithandler/concurrency_limiter_test.go
@@ -4,6 +4,7 @@ package limithandler
 
 import (
 	"context"
+	"errors"
 	"strconv"
 	"sync"
 	"testing"
@@ -12,10 +13,10 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/helper"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/structerr"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
 	"google.golang.org/protobuf/types/known/durationpb"
 )
 
@@ -296,9 +297,9 @@ func TestConcurrencyLimiter_queueLimit(t *testing.T) {
 	err := <-errChan
 	assert.Error(t, err)
 
-	s, ok := status.FromError(err)
-	require.True(t, ok)
-	details := s.Details()
+	var structErr structerr.Error
+	require.True(t, errors.As(err, &structErr))
+	details := structErr.Details()
 	require.Len(t, details, 1)
 
 	limitErr, ok := details[0].(*gitalypb.LimitError)
@@ -369,9 +370,9 @@ func TestLimitConcurrency_queueWaitTime(t *testing.T) {
 	<-dequeuedCh
 	err := <-errChan
 
-	s, ok := status.FromError(err)
-	require.True(t, ok)
-	details := s.Details()
+	var structErr structerr.Error
+	require.True(t, errors.As(err, &structErr))
+	details := structErr.Details()
 	require.Len(t, details, 1)
 
 	limitErr, ok := details[0].(*gitalypb.LimitError)
diff --git a/internal/structerr/error.go b/internal/structerr/error.go
new file mode 100644
index 000000000..05c6e9f39
--- /dev/null
+++ b/internal/structerr/error.go
@@ -0,0 +1,270 @@
+package structerr
+
+import (
+	"errors"
+	"fmt"
+
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+	"google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/types/known/anypb"
+)
+
+type metadataItem struct {
+	key   string
+	value any
+}
+
+// Error is a structured error that contains additional details.
+type Error struct {
+	err    error
+	code   codes.Code
+	detail proto.Message
+	// metadata is the array of metadata items added to this error. Note that we explicitly
+	// don't use a map here so that we don't have any allocation overhead here in the general
+	// case where there is no metadata.
+	metadata []metadataItem
+}
+
+func newError(code codes.Code, format string, a ...any) Error {
+	for i, arg := range a {
+		err, ok := arg.(error)
+		if !ok {
+			continue
+		}
+
+		if errors.As(err, &(Error{})) {
+			// We need to explicitly handle this, otherwise `status.FromError()` would
+			// return these because we implement `GRPCStatus()`.
+			continue
+		}
+
+		// Convert any gRPC status we see here to an `Error`.
+		if st, ok := status.FromError(err); ok {
+			a[i] = Error{
+				err:  errors.New(st.Message()),
+				code: st.Code(),
+			}
+		}
+	}
+
+	formattedErr := fmt.Errorf(format, a...)
+
+	// When we wrap an Error, we retain its error code. The intent of this is to retain the most
+	// specific error code we have in the general case.
+	var wrappedErr Error
+	if errors.As(formattedErr, &wrappedErr) {
+		code = wrappedErr.code
+	}
+
+	return Error{
+		err:  formattedErr,
+		code: code,
+	}
+}
+
+// New returns a new Error with the default error code, which is Internal. When this function is
+// used to wrap another Error, then the error code of that wrapped Error will be retained. The
+// intent of this is to always retain the most specific error code in the general case.
+func New(format string, a ...any) Error {
+	return newError(codes.Internal, format, a...)
+}
+
+// NewAborted constructs a new error code with the Aborted error code. Please refer to New for
+// further details.
+func NewAborted(format string, a ...any) Error {
+	return newError(codes.Aborted, format, a...)
+}
+
+// NewAlreadyExists constructs a new error code with the AlreadyExists error code. Please refer to
+// New for further details.
+func NewAlreadyExists(format string, a ...any) Error {
+	return newError(codes.AlreadyExists, format, a...)
+}
+
+// NewCanceled constructs a new error code with the Canceled error code. Please refer to New for
+// further details.
+func NewCanceled(format string, a ...any) Error {
+	return newError(codes.Canceled, format, a...)
+}
+
+// NewDataLoss constructs a new error code with the DataLoss error code. Please refer to New for
+// further details.
+func NewDataLoss(format string, a ...any) Error {
+	return newError(codes.DataLoss, format, a...)
+}
+
+// NewDeadlineExceeded constructs a new error code with the DeadlineExceeded error code. Please
+// refer to New for further details.
+func NewDeadlineExceeded(format string, a ...any) Error {
+	return newError(codes.DeadlineExceeded, format, a...)
+}
+
+// NewFailedPrecondition constructs a new error code with the FailedPrecondition error code. Please
+// refer to New for further details.
+func NewFailedPrecondition(format string, a ...any) Error {
+	return newError(codes.FailedPrecondition, format, a...)
+}
+
+// NewInternal constructs a new error code with the Internal error code. Please refer to New for
+// further details.
+func NewInternal(format string, a ...any) Error {
+	return newError(codes.Internal, format, a...)
+}
+
+// NewInvalidArgument constructs a new error code with the InvalidArgument error code. Please refer
+// to New for further details.
+func NewInvalidArgument(format string, a ...any) Error {
+	return newError(codes.InvalidArgument, format, a...)
+}
+
+// NewNotFound constructs a new error code with the NotFound error code. Please refer to New for
+// further details.
+func NewNotFound(format string, a ...any) Error {
+	return newError(codes.NotFound, format, a...)
+}
+
+// NewPermissionDenied constructs a new error code with the PermissionDenied error code. Please
+// refer to New for further details.
+func NewPermissionDenied(format string, a ...any) Error {
+	return newError(codes.PermissionDenied, format, a...)
+}
+
+// NewResourceExhausted constructs a new error code with the ResourceExhausted error code. Please
+// refer to New for further details.
+func NewResourceExhausted(format string, a ...any) Error {
+	return newError(codes.ResourceExhausted, format, a...)
+}
+
+// NewUnavailable constructs a new error code with the Unavailable error code. Please refer to New
+// for further details.
+func NewUnavailable(format string, a ...any) Error {
+	return newError(codes.Unavailable, format, a...)
+}
+
+// NewUnauthenticated constructs a new error code with the Unauthenticated error code. Please refer
+// to New for further details.
+func NewUnauthenticated(format string, a ...any) Error {
+	return newError(codes.Unauthenticated, format, a...)
+}
+
+// NewUnimplemented constructs a new error code with the Unimplemented error code. Please refer to
+// New for further details.
+func NewUnimplemented(format string, a ...any) Error {
+	return newError(codes.Unimplemented, format, a...)
+}
+
+// NewUnknown constructs a new error code with the Unknown error code. Please refer to New for
+// further details.
+func NewUnknown(format string, a ...any) Error {
+	return newError(codes.Unknown, format, a...)
+}
+
+// Error returns the error message of the Error.
+func (e Error) Error() string {
+	return e.err.Error()
+}
+
+// Unwrap returns the wrapped error if any, otherwise it returns nil.
+func (e Error) Unwrap() error {
+	return errors.Unwrap(e.err)
+}
+
+// Code returns the error code of the Error.
+func (e Error) Code() codes.Code {
+	return e.code
+}
+
+// GRPCStatus returns the gRPC status of this error.
+func (e Error) GRPCStatus() *status.Status {
+	st := status.New(e.Code(), e.Error())
+
+	if details := e.Details(); len(details) > 0 {
+		proto := st.Proto()
+
+		for _, detail := range details {
+			marshaled, err := anypb.New(detail)
+			if err != nil {
+				return status.New(codes.Internal, fmt.Sprintf("marshaling error details: %v", err))
+			}
+
+			proto.Details = append(proto.Details, marshaled)
+		}
+
+		st = status.FromProto(proto)
+	}
+
+	return st
+}
+
+// errorChain returns the complete chain of `structerr.Error`s wrapped by this error, including the
+// error itself.
+func (e Error) errorChain() []Error {
+	var result []Error
+	for err := error(e); err != nil; err = errors.Unwrap(err) {
+		if structErr, ok := err.(Error); ok {
+			result = append(result, structErr)
+		}
+	}
+	return result
+}
+
+// Metadata returns the Error's metadata. The metadata will contain the combination of all added
+// metadata of this error as well as any wrapped Errors.
+//
+// When the same metada key exists multiple times in the error chain, then the value that is
+// highest up the callchain will be returned. This is done because in general, the higher up the
+// callchain one is the more context is available.
+func (e Error) Metadata() map[string]any {
+	result := map[string]any{}
+
+	for _, err := range e.errorChain() {
+		for _, m := range err.metadata {
+			if _, exists := result[m.key]; !exists {
+				result[m.key] = m.value
+			}
+		}
+	}
+
+	return result
+}
+
+// WithMetadata adds an additional metadata item to the Error. The metadata has the intent to
+// provide more context around errors to the consumer of the Error. Calling this function multiple
+// times with the same key will override any previous values.
+func (e Error) WithMetadata(key string, value any) Error {
+	for i, metadataItem := range e.metadata {
+		// In case the key already exists we override it.
+		if metadataItem.key == key {
+			e.metadata[i].value = value
+			return e
+		}
+	}
+
+	// Otherwise we append a new metadata item.
+	e.metadata = append(e.metadata, metadataItem{
+		key: key, value: value,
+	})
+	return e
+}
+
+// Details returns the chain error details set by this and any wrapped Error. The returned array
+// contains error details ordered from top-level error details to bottom-level error details.
+func (e Error) Details() []proto.Message {
+	var details []proto.Message
+
+	for _, err := range e.errorChain() {
+		if err.detail != nil {
+			details = append(details, err.detail)
+		}
+	}
+
+	return details
+}
+
+// WithDetail sets the Error detail that provides additional structured information about the error
+// via gRPC so that callers can programmatically determine the exact circumstances of an error.
+func (e Error) WithDetail(detail proto.Message) Error {
+	e.detail = detail
+	return e
+}
diff --git a/internal/structerr/error_test.go b/internal/structerr/error_test.go
new file mode 100644
index 000000000..697eda49a
--- /dev/null
+++ b/internal/structerr/error_test.go
@@ -0,0 +1,397 @@
+package structerr
+
+import (
+	"errors"
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+	"google.golang.org/grpc/test/grpc_testing"
+	"google.golang.org/protobuf/proto"
+)
+
+// unusedErrorCode is any error code that we don't have any constructors for yet. This is used
+// to verify that we correctly wrap errors that already have a different gRPC error code than the
+// one under test.
+const unusedErrorCode = codes.OutOfRange
+
+func TestNew(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range []struct {
+		desc         string
+		constructor  func(format string, a ...any) Error
+		expectedCode codes.Code
+	}{
+		{
+			desc:         "New",
+			constructor:  New,
+			expectedCode: codes.Internal,
+		},
+		{
+			desc:         "NewAborted",
+			constructor:  NewAborted,
+			expectedCode: codes.Aborted,
+		},
+		{
+			desc:         "NewAlreadyExist",
+			constructor:  NewAlreadyExists,
+			expectedCode: codes.AlreadyExists,
+		},
+		{
+			desc:         "NewCanceled",
+			constructor:  NewCanceled,
+			expectedCode: codes.Canceled,
+		},
+		{
+			desc:         "NewDataLoss",
+			constructor:  NewDataLoss,
+			expectedCode: codes.DataLoss,
+		},
+		{
+			desc:         "NewDeadlineExceeded",
+			constructor:  NewDeadlineExceeded,
+			expectedCode: codes.DeadlineExceeded,
+		},
+		{
+			desc:         "NewFailedPrecondition",
+			constructor:  NewFailedPrecondition,
+			expectedCode: codes.FailedPrecondition,
+		},
+		{
+			desc:         "NewInternal",
+			constructor:  NewInternal,
+			expectedCode: codes.Internal,
+		},
+		{
+			desc:         "NewInvalidArgument",
+			constructor:  NewInvalidArgument,
+			expectedCode: codes.InvalidArgument,
+		},
+		{
+			desc:         "NewNotFound",
+			constructor:  NewNotFound,
+			expectedCode: codes.NotFound,
+		},
+		{
+			desc:         "NewPermissionDenied",
+			constructor:  NewPermissionDenied,
+			expectedCode: codes.PermissionDenied,
+		},
+		{
+			desc:         "NewResourceExhausted",
+			constructor:  NewResourceExhausted,
+			expectedCode: codes.ResourceExhausted,
+		},
+		{
+			desc:         "NewUnavailable",
+			constructor:  NewUnavailable,
+			expectedCode: codes.Unavailable,
+		},
+		{
+			desc:         "NewUnauthenticated",
+			constructor:  NewUnauthenticated,
+			expectedCode: codes.Unauthenticated,
+		},
+		{
+			desc:         "NewUnimplemented",
+			constructor:  NewUnimplemented,
+			expectedCode: codes.Unimplemented,
+		},
+		{
+			desc:         "NewUnknown",
+			constructor:  NewUnknown,
+			expectedCode: codes.Unknown,
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			require.NotEqual(t, tc.expectedCode, unusedErrorCode)
+
+			t.Run("without wrapping", func(t *testing.T) {
+				err := tc.constructor("top-level: %v", errors.New("nested"))
+				require.EqualError(t, err, "top-level: nested")
+				require.Equal(t, tc.expectedCode, status.Code(err))
+
+				s, ok := status.FromError(err)
+				require.True(t, ok)
+				require.Equal(t, status.New(tc.expectedCode, "top-level: nested"), s)
+			})
+
+			t.Run("wrapping normal error", func(t *testing.T) {
+				err := tc.constructor("top-level: %w", errors.New("nested"))
+				require.EqualError(t, err, "top-level: nested")
+				require.Equal(t, tc.expectedCode, status.Code(err))
+
+				s, ok := status.FromError(err)
+				require.True(t, ok)
+				require.Equal(t, status.New(tc.expectedCode, "top-level: nested"), s)
+			})
+
+			t.Run("wrapping structerr with %v", func(t *testing.T) {
+				err := tc.constructor("top-level: %v", newError(unusedErrorCode, "nested"))
+				require.EqualError(t, err, "top-level: nested")
+				// We should be reporting the error code of the newly created error.
+				require.Equal(t, tc.expectedCode, status.Code(err))
+
+				s, ok := status.FromError(err)
+				require.True(t, ok)
+				require.Equal(t, status.New(tc.expectedCode, "top-level: nested"), s)
+			})
+
+			t.Run("wrapping structerr with %w", func(t *testing.T) {
+				err := tc.constructor("top-level: %w", newError(unusedErrorCode, "nested"))
+				require.EqualError(t, err, "top-level: nested")
+				// We should be reporting the error code of the nested error.
+				require.Equal(t, unusedErrorCode, status.Code(err))
+
+				s, ok := status.FromError(err)
+				require.True(t, ok)
+				require.Equal(t, status.New(unusedErrorCode, "top-level: nested"), s)
+			})
+
+			t.Run("wrapping status.Error", func(t *testing.T) {
+				err := tc.constructor("top-level: %w", status.Error(unusedErrorCode, "nested"))
+				require.EqualError(t, err, "top-level: nested")
+				// We should be reporting the error code of the wrapped gRPC status.
+				require.Equal(t, unusedErrorCode, status.Code(err))
+
+				s, ok := status.FromError(err)
+				require.True(t, ok)
+				require.Equal(t, status.New(unusedErrorCode, "top-level: nested"), s)
+			})
+
+			t.Run("mixed normal and structerr chain", func(t *testing.T) {
+				err := tc.constructor("first: %w", fmt.Errorf("second: %w", newError(unusedErrorCode, "third")))
+				require.EqualError(t, err, "first: second: third")
+				// We should be reporting the error code of the nested error.
+				require.Equal(t, unusedErrorCode, status.Code(err))
+
+				s, ok := status.FromError(err)
+				require.True(t, ok)
+				require.Equal(t, status.New(unusedErrorCode, "first: second: third"), s)
+			})
+		})
+	}
+}
+
+func TestError_Metadata(t *testing.T) {
+	t.Parallel()
+
+	t.Run("without metadata", func(t *testing.T) {
+		err := New("message")
+		require.Equal(t, Error{
+			err:  errors.New("message"),
+			code: codes.Internal,
+		}, err)
+		require.Equal(t, map[string]any{}, err.Metadata())
+	})
+
+	t.Run("single metadata key", func(t *testing.T) {
+		err := New("message").WithMetadata("key", "value")
+		require.Equal(t, Error{
+			err:  errors.New("message"),
+			code: codes.Internal,
+			metadata: []metadataItem{
+				{key: "key", value: "value"},
+			},
+		}, err)
+		require.Equal(t, map[string]any{
+			"key": "value",
+		}, err.Metadata())
+	})
+
+	t.Run("multiple metadata keys", func(t *testing.T) {
+		err := New("message").WithMetadata("first", 1).WithMetadata("second", 2)
+		require.Equal(t, Error{
+			err:  errors.New("message"),
+			code: codes.Internal,
+			metadata: []metadataItem{
+				{key: "first", value: 1},
+				{key: "second", value: 2},
+			},
+		}, err)
+		require.Equal(t, map[string]any{
+			"first":  1,
+			"second": 2,
+		}, err.Metadata())
+	})
+
+	t.Run("overriding metadata keys", func(t *testing.T) {
+		err := New("message").WithMetadata("first", "initial").WithMetadata("first", "overridden")
+		require.Equal(t, Error{
+			err:  errors.New("message"),
+			code: codes.Internal,
+			metadata: []metadataItem{
+				{key: "first", value: "overridden"},
+			},
+		}, err)
+		require.Equal(t, map[string]any{
+			"first": "overridden",
+		}, err.Metadata())
+	})
+
+	t.Run("chained metadata", func(t *testing.T) {
+		nestedErr := New("nested").WithMetadata("nested", "value")
+		toplevelErr := New("top-level: %w", nestedErr).WithMetadata("toplevel", "value")
+		require.Equal(t, Error{
+			err:  fmt.Errorf("top-level: %w", nestedErr),
+			code: codes.Internal,
+			metadata: []metadataItem{
+				{key: "toplevel", value: "value"},
+			},
+		}, toplevelErr)
+		require.Equal(t, map[string]any{
+			"nested":   "value",
+			"toplevel": "value",
+		}, toplevelErr.Metadata())
+	})
+
+	t.Run("chained metadata overriding each other", func(t *testing.T) {
+		nestedErr := New("nested").WithMetadata("key", "nested")
+		toplevelErr := New("top-level: %w", nestedErr).WithMetadata("key", "top-level")
+		require.Equal(t, Error{
+			err:  fmt.Errorf("top-level: %w", nestedErr),
+			code: codes.Internal,
+			metadata: []metadataItem{
+				{key: "key", value: "top-level"},
+			},
+		}, toplevelErr)
+		require.Equal(t, map[string]any{
+			"key": "top-level",
+		}, toplevelErr.Metadata())
+	})
+
+	t.Run("chained metadata with internal overrides", func(t *testing.T) {
+		nestedErr := New("nested").WithMetadata("nested", "initial").WithMetadata("nested", "overridden")
+		toplevelErr := New("top-level: %w", nestedErr).WithMetadata("toplevel", "initial").WithMetadata("toplevel", "overridden")
+		require.Equal(t, Error{
+			err:  fmt.Errorf("top-level: %w", nestedErr),
+			code: codes.Internal,
+			metadata: []metadataItem{
+				{key: "toplevel", value: "overridden"},
+			},
+		}, toplevelErr)
+		require.Equal(t, map[string]any{
+			"toplevel": "overridden",
+			"nested":   "overridden",
+		}, toplevelErr.Metadata())
+	})
+
+	t.Run("chained metadata with mixed error types", func(t *testing.T) {
+		bottomErr := New("bottom").WithMetadata("bottom", "value")
+		midlevelErr := fmt.Errorf("mid: %w", bottomErr)
+		toplevelErr := New("top: %w", midlevelErr).WithMetadata("toplevel", "value")
+
+		require.Equal(t, Error{
+			err:  fmt.Errorf("top: %w", midlevelErr),
+			code: codes.Internal,
+			metadata: []metadataItem{
+				{key: "toplevel", value: "value"},
+			},
+		}, toplevelErr)
+		require.Equal(t, map[string]any{
+			"bottom":   "value",
+			"toplevel": "value",
+		}, toplevelErr.Metadata())
+	})
+}
+
+func TestError_Details(t *testing.T) {
+	t.Parallel()
+
+	initialPayload := &grpc_testing.Payload{
+		Body: []byte("bottom"),
+	}
+	overridingPayload := &grpc_testing.Payload{
+		Body: []byte("top"),
+	}
+
+	for _, tc := range []struct {
+		desc            string
+		createError     func() Error
+		expectedErr     error
+		expectedDetails []proto.Message
+		expectedMessage string
+	}{
+		{
+			desc: "without details",
+			createError: func() Error {
+				return New("message")
+			},
+			expectedErr: Error{
+				err:  errors.New("message"),
+				code: codes.Internal,
+			},
+			expectedMessage: "message",
+		},
+		{
+			desc: "single detail",
+			createError: func() Error {
+				return New("message").WithDetail(initialPayload)
+			},
+			expectedErr: Error{
+				err:    errors.New("message"),
+				code:   codes.Internal,
+				detail: initialPayload,
+			},
+			expectedDetails: []proto.Message{
+				initialPayload,
+			},
+			expectedMessage: "message",
+		},
+		{
+			desc: "overridden detail",
+			createError: func() Error {
+				return New("message").WithDetail(initialPayload).WithDetail(overridingPayload)
+			},
+			expectedErr: Error{
+				err:    errors.New("message"),
+				code:   codes.Internal,
+				detail: overridingPayload,
+			},
+			expectedDetails: []proto.Message{
+				overridingPayload,
+			},
+			expectedMessage: "message",
+		},
+		{
+			desc: "chained details",
+			createError: func() Error {
+				nestedErr := New("nested").WithDetail(initialPayload)
+				return New("top-level: %w", nestedErr).WithDetail(overridingPayload)
+			},
+			expectedErr: Error{
+				err:    fmt.Errorf("top-level: %w", New("nested").WithDetail(initialPayload)),
+				code:   codes.Internal,
+				detail: overridingPayload,
+			},
+			expectedDetails: []proto.Message{
+				overridingPayload,
+				initialPayload,
+			},
+			expectedMessage: "top-level: nested",
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			err := tc.createError()
+			require.Equal(t, tc.expectedErr, err)
+			testhelper.ProtoEqual(t, tc.expectedDetails, err.Details())
+
+			// `proto.Details()` returns an `[]any` slice, so we need to convert here or
+			// otherwise the comparison would fail.
+			anyDetails := make([]any, 0, len(tc.expectedDetails))
+			for _, detail := range tc.expectedDetails {
+				anyDetails = append(anyDetails, detail)
+			}
+
+			s, ok := status.FromError(err)
+			require.True(t, ok)
+			require.Equal(t, codes.Internal, s.Code())
+			require.Equal(t, tc.expectedMessage, s.Message())
+			testhelper.ProtoEqual(t, anyDetails, s.Details())
+		})
+	}
+}
diff --git a/internal/structerr/fields_producer.go b/internal/structerr/fields_producer.go
new file mode 100644
index 000000000..d4d6d16b4
--- /dev/null
+++ b/internal/structerr/fields_producer.go
@@ -0,0 +1,26 @@
+package structerr
+
+import (
+	"context"
+	"errors"
+
+	"github.com/sirupsen/logrus"
+)
+
+// FieldsProducer extracts metadata from err if it contains a `structerr.Error` and exposes it as
+// logged fields. This function is supposed to be used with `log.MessageProducer()`.
+func FieldsProducer(_ context.Context, err error) logrus.Fields {
+	var structErr Error
+	if errors.As(err, &structErr) {
+		metadata := structErr.Metadata()
+		if len(metadata) == 0 {
+			return nil
+		}
+
+		return logrus.Fields{
+			"error.metadata": metadata,
+		}
+	}
+
+	return nil
+}
diff --git a/internal/structerr/fields_producer_test.go b/internal/structerr/fields_producer_test.go
new file mode 100644
index 000000000..ef6bbee45
--- /dev/null
+++ b/internal/structerr/fields_producer_test.go
@@ -0,0 +1,135 @@
+package structerr
+
+import (
+	"context"
+	"errors"
+	"net"
+	"testing"
+
+	grpcmw "github.com/grpc-ecosystem/go-grpc-middleware"
+	grpcmwlogrus "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus"
+	"github.com/sirupsen/logrus"
+	"github.com/sirupsen/logrus/hooks/test"
+	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/grpcstats"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/log"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/credentials/insecure"
+	"google.golang.org/grpc/status"
+	"google.golang.org/grpc/test/grpc_testing"
+)
+
+type mockService struct {
+	grpc_testing.UnimplementedTestServiceServer
+	err error
+}
+
+func (m *mockService) UnaryCall(
+	context.Context, *grpc_testing.SimpleRequest,
+) (*grpc_testing.SimpleResponse, error) {
+	return &grpc_testing.SimpleResponse{}, m.err
+}
+
+func TestFieldsProducer(t *testing.T) {
+	t.Parallel()
+
+	ctx := testhelper.Context(t)
+	logger, loggerHook := test.NewNullLogger()
+	logger.SetLevel(logrus.ErrorLevel)
+
+	listener, err := net.Listen("tcp", "localhost:0")
+	require.NoError(t, err)
+
+	service := &mockService{}
+	server := grpc.NewServer(
+		grpc.StatsHandler(log.PerRPCLogHandler{
+			Underlying: &grpcstats.PayloadBytes{},
+		}),
+		grpcmw.WithUnaryServerChain(
+			grpcmwlogrus.UnaryServerInterceptor(
+				logrus.NewEntry(logger),
+				grpcmwlogrus.WithMessageProducer(
+					log.MessageProducer(
+						log.PropagationMessageProducer(grpcmwlogrus.DefaultMessageProducer),
+						FieldsProducer,
+					),
+				),
+			),
+		),
+	)
+	grpc_testing.RegisterTestServiceServer(server, service)
+
+	go testhelper.MustServe(t, server, listener)
+	defer server.Stop()
+
+	conn, err := grpc.Dial(listener.Addr().String(),
+		grpc.WithTransportCredentials(insecure.NewCredentials()),
+	)
+	require.NoError(t, err)
+	defer testhelper.MustClose(t, conn)
+
+	client := grpc_testing.NewTestServiceClient(conn)
+
+	for _, tc := range []struct {
+		desc             string
+		returnedErr      error
+		expectedErr      error
+		expectedMetadata []map[string]any
+	}{
+		{
+			desc:        "no error",
+			returnedErr: nil,
+		},
+		{
+			desc:        "plain error",
+			returnedErr: errors.New("message"),
+			expectedErr: status.Error(codes.Unknown, "message"),
+		},
+		{
+			desc:        "structured error",
+			returnedErr: New("message"),
+			expectedErr: status.Error(codes.Internal, "message"),
+		},
+		{
+			desc:        "structured error with metadata",
+			returnedErr: New("message").WithMetadata("key", "value"),
+			expectedErr: status.Error(codes.Internal, "message"),
+			expectedMetadata: []map[string]any{
+				{
+					"key": "value",
+				},
+			},
+		},
+		{
+			desc:        "structured error with nested metadata",
+			returnedErr: New("message: %w", New("nested").WithMetadata("nested", "value")).WithMetadata("key", "value"),
+			expectedErr: status.Error(codes.Internal, "message: nested"),
+			expectedMetadata: []map[string]any{
+				{
+					"key":    "value",
+					"nested": "value",
+				},
+			},
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			loggerHook.Reset()
+
+			service.err = tc.returnedErr
+
+			_, err := client.UnaryCall(ctx, &grpc_testing.SimpleRequest{})
+			testhelper.ProtoEqual(t, tc.expectedErr, err)
+
+			var metadata []map[string]any
+			for _, entry := range loggerHook.AllEntries() {
+				if untypedMetadata := entry.Data["error.metadata"]; untypedMetadata != nil {
+					require.IsType(t, untypedMetadata, map[string]any{})
+					metadata = append(metadata, untypedMetadata.(map[string]any))
+				}
+			}
+			require.Equal(t, tc.expectedMetadata, metadata)
+		})
+	}
+}
diff --git a/internal/structerr/testhelper_test.go b/internal/structerr/testhelper_test.go
new file mode 100644
index 000000000..b9934fecc
--- /dev/null
+++ b/internal/structerr/testhelper_test.go
@@ -0,0 +1,11 @@
+package structerr
+
+import (
+	"testing"
+
+	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
+)
+
+func TestMain(m *testing.M) {
+	testhelper.Run(m)
+}