Merge branch 'pks-update-ref-improved-errors' into 'master'

Introduced generic error metadata and improve updateref errors

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

Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: karthik nayak <knayak@gitlab.com>
Reviewed-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Reviewed-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Reviewed-by: karthik nayak <knayak@gitlab.com>

15 files changed, 241 insertions, 79 deletions

diff --git a/internal/git/updateref/update_with_hooks.go b/internal/git/updateref/update_with_hooks.go
index 6efa375c6..4c8201916 100644
--- a/internal/git/updateref/update_with_hooks.go
+++ b/internal/git/updateref/update_with_hooks.go
@@ -125,7 +125,7 @@ func (e Error) Unwrap() error {
 }
 
 func (e Error) Error() string {
-	return fmt.Sprintf("Could not update %s. Please refresh and try again.", e.Reference)
+	return fmt.Sprintf("reference update: %v", e.Cause.Error())
 }
 
 // NewUpdaterWithHooks creates a new instance of a struct that will update a Git reference.
diff --git a/internal/git/updateref/updateref.go b/internal/git/updateref/updateref.go
index 0627451d4..aab4fddc4 100644
--- a/internal/git/updateref/updateref.go
+++ b/internal/git/updateref/updateref.go
@@ -21,7 +21,15 @@ type AlreadyLockedError struct {
 }
 
 func (e AlreadyLockedError) Error() string {
-	return fmt.Sprintf("reference is already locked: %q", e.ReferenceName)
+	return "reference is already locked"
+}
+
+// ErrorMetadata implements the `structerr.ErrorMetadater` interface and provides the name of the reference that was
+// locked already.
+func (e AlreadyLockedError) ErrorMetadata() []structerr.MetadataItem {
+	return []structerr.MetadataItem{
+		{Key: "reference", Value: e.ReferenceName},
+	}
 }
 
 // InvalidReferenceFormatError indicates a reference name was invalid.
@@ -31,7 +39,15 @@ type InvalidReferenceFormatError struct {
 }
 
 func (e InvalidReferenceFormatError) Error() string {
-	return fmt.Sprintf("invalid reference format: %q", e.ReferenceName)
+	return "invalid reference format"
+}
+
+// ErrorMetadata implements the `structerr.ErrorMetadater` interface and provides the name of the reference that was
+// invalid.
+func (e InvalidReferenceFormatError) ErrorMetadata() []structerr.MetadataItem {
+	return []structerr.MetadataItem{
+		{Key: "reference", Value: e.ReferenceName},
+	}
 }
 
 // FileDirectoryConflictError is returned when an operation would causes a file-directory conflict
@@ -44,7 +60,16 @@ type FileDirectoryConflictError struct {
 }
 
 func (e FileDirectoryConflictError) Error() string {
-	return fmt.Sprintf("%q conflicts with %q", e.ConflictingReferenceName, e.ExistingReferenceName)
+	return "file directory conflict"
+}
+
+// ErrorMetadata implements the `structerr.ErrorMetadater` interface and provides the name of preexisting and
+// conflicting reference names.
+func (e FileDirectoryConflictError) ErrorMetadata() []structerr.MetadataItem {
+	return []structerr.MetadataItem{
+		{Key: "conflicting_reference", Value: e.ConflictingReferenceName},
+		{Key: "existing_reference", Value: e.ExistingReferenceName},
+	}
 }
 
 // InTransactionConflictError is returned when attempting to modify two references in the same transaction
@@ -58,7 +83,16 @@ type InTransactionConflictError struct {
 }
 
 func (e InTransactionConflictError) Error() string {
-	return fmt.Sprintf("%q and %q conflict in the same transaction", e.FirstReferenceName, e.SecondReferenceName)
+	return "conflicting reference updates in the same transaction"
+}
+
+// ErrorMetadata implements the `structerr.ErrorMetadater` interface and provides the name of the first and second
+// conflicting reference names.
+func (e InTransactionConflictError) ErrorMetadata() []structerr.MetadataItem {
+	return []structerr.MetadataItem{
+		{Key: "first_reference", Value: e.FirstReferenceName},
+		{Key: "second_reference", Value: e.SecondReferenceName},
+	}
 }
 
 // NonExistentObjectError is returned when attempting to point a reference to an object that does not
@@ -71,7 +105,16 @@ type NonExistentObjectError struct {
 }
 
 func (e NonExistentObjectError) Error() string {
-	return fmt.Sprintf("pointed reference %q to a non-existent object %q", e.ReferenceName, e.ObjectID)
+	return "target object missing"
+}
+
+// ErrorMetadata implements the `structerr.ErrorMetadater` interface and provides the missing object as well as the
+// reference that should have been updated to point to it.
+func (e NonExistentObjectError) ErrorMetadata() []structerr.MetadataItem {
+	return []structerr.MetadataItem{
+		{Key: "reference", Value: e.ReferenceName},
+		{Key: "missing_object", Value: e.ObjectID},
+	}
 }
 
 // NonCommitObjectError is returned when attempting to point a branch to an object that is not an object.
@@ -83,7 +126,41 @@ type NonCommitObjectError struct {
 }
 
 func (e NonCommitObjectError) Error() string {
-	return fmt.Sprintf("pointed branch %q to a non-commit object %q", e.ReferenceName, e.ObjectID)
+	return "target object not a commit"
+}
+
+// ErrorMetadata implements the `structerr.ErrorMetadater` interface and provides the object that is not a commit as
+// well as the reference that should have been updated to point to it.
+func (e NonCommitObjectError) ErrorMetadata() []structerr.MetadataItem {
+	return []structerr.MetadataItem{
+		{Key: "reference", Value: e.ReferenceName},
+		{Key: "non_commit_object", Value: e.ObjectID},
+	}
+}
+
+// MismatchingStateError is returned when attempting to update a reference where the expected object ID does not match
+// the actual object ID that the reference currently points to.
+type MismatchingStateError struct {
+	// ReferenceName is the name of the reference that was being updated.
+	ReferenceName string
+	// ExpectedObjectID is the expected object ID as specified by the caller.
+	ExpectedObjectID string
+	// ActualObjectID is the actual object ID that the reference was pointing to.
+	ActualObjectID string
+}
+
+func (e MismatchingStateError) Error() string {
+	return "reference does not point to expected object"
+}
+
+// ErrorMetadata implements the `structerr.ErrorMetadater` interface and provides error metadata about the expected and
+// actual object ID of the failed reference update.
+func (e MismatchingStateError) ErrorMetadata() []structerr.MetadataItem {
+	return []structerr.MetadataItem{
+		{Key: "reference", Value: e.ReferenceName},
+		{Key: "expected_object_id", Value: e.ExpectedObjectID},
+		{Key: "actual_object_id", Value: e.ActualObjectID},
+	}
 }
 
 // state represents a possible state the updater can be in.
@@ -333,6 +410,7 @@ var (
 	inTransactionConflictRegex   = regexp.MustCompile(`^fatal: .*: cannot lock ref '.*': cannot process '(.*)' and '(.*)' at the same time\n$`)
 	nonExistentObjectRegex       = regexp.MustCompile(`^fatal: .*: cannot update ref '.*': trying to write ref '(.*)' with nonexistent object (.*)\n$`)
 	nonCommitObjectRegex         = regexp.MustCompile(`^fatal: .*: cannot update ref '.*': trying to write non-commit object (.*) to branch '(.*)'\n`)
+	mismatchingStateRegex        = regexp.MustCompile(`^fatal: .*: cannot lock ref '(.*)': is at (.*) but expected (.*)\n$`)
 )
 
 func (u *Updater) setState(state string) error {
@@ -422,5 +500,14 @@ func (u *Updater) handleIOError(fallbackErr error) error {
 		}
 	}
 
+	matches = mismatchingStateRegex.FindSubmatch(stderr)
+	if len(matches) > 2 {
+		return MismatchingStateError{
+			ReferenceName:    string(matches[1]),
+			ExpectedObjectID: string(matches[3]),
+			ActualObjectID:   string(matches[2]),
+		}
+	}
+
 	return structerr.New("%w", fallbackErr).WithMetadata("stderr", string(stderr))
 }
diff --git a/internal/git/updateref/updateref_test.go b/internal/git/updateref/updateref_test.go
index 77ef4bc8f..0a409acc2 100644
--- a/internal/git/updateref/updateref_test.go
+++ b/internal/git/updateref/updateref_test.go
@@ -328,9 +328,11 @@ func TestUpdater_update(t *testing.T) {
 	require.NoError(t, updater.Start())
 	require.NoError(t, updater.Update("refs/heads/main", newCommitID, otherCommitID))
 
-	require.Equal(t, structerr.New("%w", fmt.Errorf("state update to %q failed: %w", "commit", io.EOF)).WithMetadata(
-		"stderr", fmt.Sprintf("fatal: commit: cannot lock ref 'refs/heads/main': is at %s but expected %s\n", oldCommitID, otherCommitID),
-	), updater.Commit())
+	require.Equal(t, MismatchingStateError{
+		ReferenceName:    "refs/heads/main",
+		ExpectedObjectID: otherCommitID.String(),
+		ActualObjectID:   oldCommitID.String(),
+	}, updater.Commit())
 	require.Equal(t, invalidStateTransitionError{expected: stateIdle, actual: stateClosed}, updater.Start())
 
 	require.Equal(t, gittest.ResolveRevision(t, cfg, repoPath, "refs/heads/main"), oldCommitID)
diff --git a/internal/gitaly/service/operations/apply_patch_test.go b/internal/gitaly/service/operations/apply_patch_test.go
index 68f1265dc..b8dffa014 100644
--- a/internal/gitaly/service/operations/apply_patch_test.go
+++ b/internal/gitaly/service/operations/apply_patch_test.go
@@ -439,10 +439,11 @@ To restore the original branch and stop patching, run "git am --abort".
 				futureCommit := gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents(git.ObjectID(currentCommit)), gittest.WithBranch(git.DefaultBranch))
 				return expected{
 					oldOID: currentCommit,
-					err: testhelper.WithInterceptedMetadata(
-						structerr.NewInternal(`update reference: Could not update %s. Please refresh and try again.`, git.DefaultRef),
-						"stderr",
-						fmt.Sprintf("fatal: prepare: cannot lock ref 'refs/heads/main': is at %s but expected %s\n", futureCommit, currentCommit),
+					err: testhelper.WithInterceptedMetadataItems(
+						structerr.NewInternal("update reference: reference update: reference does not point to expected object"),
+						structerr.MetadataItem{Key: "actual_object_id", Value: futureCommit},
+						structerr.MetadataItem{Key: "expected_object_id", Value: currentCommit},
+						structerr.MetadataItem{Key: "reference", Value: "refs/heads/main"},
 					),
 				}
 			},
diff --git a/internal/gitaly/service/operations/branches_test.go b/internal/gitaly/service/operations/branches_test.go
index 4501e356a..970db09b5 100644
--- a/internal/gitaly/service/operations/branches_test.go
+++ b/internal/gitaly/service/operations/branches_test.go
@@ -6,6 +6,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"io"
 	"strings"
 	"testing"
 
@@ -390,7 +391,7 @@ func TestUserCreateBranch_Failure(t *testing.T) {
 			startPoint: "master",
 			user:       gittest.TestUser,
 			err: testhelper.WithInterceptedMetadata(
-				structerr.NewFailedPrecondition("Could not update refs/heads/master. Please refresh and try again."),
+				structerr.NewFailedPrecondition("reference update: state update to %q failed: %w", "prepare", io.EOF),
 				"stderr",
 				"fatal: prepare: cannot lock ref 'refs/heads/master': reference already exists\n",
 			),
@@ -401,7 +402,11 @@ func TestUserCreateBranch_Failure(t *testing.T) {
 			branchName: "improve",
 			startPoint: "master",
 			user:       gittest.TestUser,
-			err:        status.Errorf(codes.FailedPrecondition, "Could not update refs/heads/improve. Please refresh and try again."),
+			err: testhelper.WithInterceptedMetadataItems(
+				structerr.NewFailedPrecondition("reference update: file directory conflict"),
+				structerr.MetadataItem{Key: "conflicting_reference", Value: "refs/heads/improve"},
+				structerr.MetadataItem{Key: "existing_reference", Value: "refs/heads/improve/awesome"},
+			),
 		},
 	}
 
@@ -606,10 +611,18 @@ func TestUserDeleteBranch(t *testing.T) {
 						ExpectedOldOid: firstCommit.String(),
 					},
 					repoPath: repoPath,
-					expectedErr: structerr.NewFailedPrecondition("reference update failed: Could not update refs/heads/%s. Please refresh and try again.", branchName).
+					expectedErr: structerr.NewFailedPrecondition("reference update failed: reference update: reference does not point to expected object").
+						WithDetail(&testproto.ErrorMetadata{
+							Key:   []byte("actual_object_id"),
+							Value: []byte(secondCommit),
+						}).
 						WithDetail(&testproto.ErrorMetadata{
-							Key:   []byte("stderr"),
-							Value: []byte(fmt.Sprintf("fatal: prepare: cannot lock ref 'refs/heads/%s': is at %s but expected %s\n", branchName, secondCommit, firstCommit)),
+							Key:   []byte("expected_object_id"),
+							Value: []byte(firstCommit),
+						}).
+						WithDetail(&testproto.ErrorMetadata{
+							Key:   []byte("reference"),
+							Value: []byte("refs/heads/" + branchName),
 						}).
 						WithDetail(&gitalypb.UserDeleteBranchError{
 							Error: &gitalypb.UserDeleteBranchError_ReferenceUpdate{
@@ -747,8 +760,12 @@ func TestUserDeleteBranch_concurrentUpdate(t *testing.T) {
 		BranchName: []byte("concurrent-update"),
 		User:       gittest.TestUser,
 	})
-	testhelper.RequireGrpcError(t, structerr.NewFailedPrecondition("reference update failed: Could not update refs/heads/concurrent-update. Please refresh and try again.").WithDetail(
-		&gitalypb.UserDeleteBranchError{
+	testhelper.RequireGrpcError(t, structerr.NewFailedPrecondition("reference update failed: reference update: reference is already locked").
+		WithDetail(&testproto.ErrorMetadata{
+			Key:   []byte("reference"),
+			Value: []byte("refs/heads/concurrent-update"),
+		}).
+		WithDetail(&gitalypb.UserDeleteBranchError{
 			Error: &gitalypb.UserDeleteBranchError_ReferenceUpdate{
 				ReferenceUpdate: &gitalypb.ReferenceUpdateError{
 					OldOid:        commitID.String(),
@@ -756,8 +773,7 @@ func TestUserDeleteBranch_concurrentUpdate(t *testing.T) {
 					ReferenceName: []byte("refs/heads/concurrent-update"),
 				},
 			},
-		},
-	), err)
+		}), err)
 	require.Nil(t, response)
 }
 
diff --git a/internal/gitaly/service/operations/cherry_pick_test.go b/internal/gitaly/service/operations/cherry_pick_test.go
index 0f26d527a..267e56b78 100644
--- a/internal/gitaly/service/operations/cherry_pick_test.go
+++ b/internal/gitaly/service/operations/cherry_pick_test.go
@@ -269,10 +269,11 @@ func TestUserCherryPick(t *testing.T) {
 						ExpectedOldOid: data.masterCommit,
 					},
 					&gitalypb.UserCherryPickResponse{},
-					testhelper.WithInterceptedMetadata(
-						structerr.NewInternal("update reference with hooks: Could not update refs/heads/master. Please refresh and try again."),
-						"stderr",
-						fmt.Sprintf("fatal: prepare: cannot lock ref 'refs/heads/master': is at %s but expected %s\n", commit, data.masterCommit),
+					testhelper.WithInterceptedMetadataItems(
+						structerr.NewInternal("update reference with hooks: reference update: reference does not point to expected object"),
+						structerr.MetadataItem{Key: "actual_object_id", Value: commit},
+						structerr.MetadataItem{Key: "expected_object_id", Value: data.masterCommit},
+						structerr.MetadataItem{Key: "reference", Value: "refs/heads/master"},
 					)
 			},
 		},
diff --git a/internal/gitaly/service/operations/commit_files_test.go b/internal/gitaly/service/operations/commit_files_test.go
index bee801b5b..92bef38bb 100644
--- a/internal/gitaly/service/operations/commit_files_test.go
+++ b/internal/gitaly/service/operations/commit_files_test.go
@@ -1164,10 +1164,11 @@ func testSuccessfulUserCommitFilesRequest(t *testing.T, ctx context.Context) {
 			repoPath:       repoPath,
 			branchName:     "few-commits",
 			expectedOldOID: text.ChompBytes(gittest.Exec(t, cfg, "-C", repoPath, "rev-parse", "refs/heads/few-commits~1")),
-			expectedError: testhelper.WithInterceptedMetadata(
-				structerr.NewFailedPrecondition("Could not update refs/heads/few-commits. Please refresh and try again."),
-				"stderr",
-				"fatal: prepare: cannot lock ref 'refs/heads/few-commits': is at 0031876facac3f2b2702a0e53a26e89939a42209 but expected bf6e164cac2dc32b1f391ca4290badcbe4ffc5fb\n",
+			expectedError: testhelper.WithInterceptedMetadataItems(
+				structerr.NewFailedPrecondition("reference update: reference does not point to expected object"),
+				structerr.MetadataItem{Key: "actual_object_id", Value: "0031876facac3f2b2702a0e53a26e89939a42209"},
+				structerr.MetadataItem{Key: "expected_object_id", Value: "bf6e164cac2dc32b1f391ca4290badcbe4ffc5fb"},
+				structerr.MetadataItem{Key: "reference", Value: "refs/heads/few-commits"},
 			),
 		},
 		{
diff --git a/internal/gitaly/service/operations/merge_test.go b/internal/gitaly/service/operations/merge_test.go
index 948df2aac..43b7e11d5 100644
--- a/internal/gitaly/service/operations/merge_test.go
+++ b/internal/gitaly/service/operations/merge_test.go
@@ -191,10 +191,18 @@ func testUserMergeBranch(t *testing.T, ctx context.Context) {
 					secondRequest:          &gitalypb.UserMergeBranchRequest{Apply: true},
 					secondExpectedResponse: &gitalypb.OperationBranchUpdate{},
 					secondExpectedErr: func(response *gitalypb.UserMergeBranchResponse) error {
-						return structerr.NewFailedPrecondition("Could not update refs/heads/master. Please refresh and try again.").
+						return structerr.NewFailedPrecondition("reference update: reference does not point to expected object").
 							WithDetail(&testproto.ErrorMetadata{
-								Key:   []byte("stderr"),
-								Value: []byte(fmt.Sprintf("fatal: prepare: cannot lock ref 'refs/heads/master': is at %s but expected %s\n", secondCommit, data.masterCommit)),
+								Key:   []byte("actual_object_id"),
+								Value: []byte(secondCommit),
+							}).
+							WithDetail(&testproto.ErrorMetadata{
+								Key:   []byte("expected_object_id"),
+								Value: []byte(data.masterCommit),
+							}).
+							WithDetail(&testproto.ErrorMetadata{
+								Key:   []byte("reference"),
+								Value: []byte("refs/heads/" + data.branch),
 							}).
 							WithDetail(&gitalypb.UserMergeBranchError{
 								Error: &gitalypb.UserMergeBranchError_ReferenceUpdate{
@@ -719,10 +727,18 @@ func testUserMergeBranchConcurrentUpdate(t *testing.T, ctx context.Context) {
 
 	secondResponse, err := mergeBidi.Recv()
 	testhelper.RequireGrpcError(t,
-		structerr.NewFailedPrecondition("Could not update refs/heads/gitaly-merge-test-branch. Please refresh and try again.").
+		structerr.NewFailedPrecondition("reference update: reference does not point to expected object").
+			WithDetail(&testproto.ErrorMetadata{
+				Key:   []byte("actual_object_id"),
+				Value: []byte(concurrentCommitID),
+			}).
+			WithDetail(&testproto.ErrorMetadata{
+				Key:   []byte("expected_object_id"),
+				Value: []byte("281d3a76f31c812dbf48abce82ccf6860adedd81"),
+			}).
 			WithDetail(&testproto.ErrorMetadata{
-				Key:   []byte("stderr"),
-				Value: []byte("fatal: prepare: cannot lock ref 'refs/heads/gitaly-merge-test-branch': is at 549090fbeacc6607bc70648d3ba554c355e670c5 but expected 281d3a76f31c812dbf48abce82ccf6860adedd81\n"),
+				Key:   []byte("reference"),
+				Value: []byte("refs/heads/" + mergeBranchName),
 			}).
 			WithDetail(&gitalypb.UserMergeBranchError{
 				Error: &gitalypb.UserMergeBranchError_ReferenceUpdate{
diff --git a/internal/gitaly/service/operations/revert_test.go b/internal/gitaly/service/operations/revert_test.go
index 71fa4411e..010e5f183 100644
--- a/internal/gitaly/service/operations/revert_test.go
+++ b/internal/gitaly/service/operations/revert_test.go
@@ -367,10 +367,11 @@ func TestUserRevert(t *testing.T) {
 						Message:        []byte("Reverting " + secondCommitID),
 						ExpectedOldOid: firstCommitID.String(),
 					},
-					expectedError: testhelper.WithInterceptedMetadata(
-						structerr.NewInternal("update reference with hooks: Could not update refs/heads/%s. Please refresh and try again.", branchName),
-						"stderr",
-						fmt.Sprintf("fatal: prepare: cannot lock ref 'refs/heads/%s': is at %s but expected %s\n", branchName, secondCommitID, firstCommitID),
+					expectedError: testhelper.WithInterceptedMetadataItems(
+						structerr.NewInternal("update reference with hooks: reference update: reference does not point to expected object"),
+						structerr.MetadataItem{Key: "actual_object_id", Value: secondCommitID},
+						structerr.MetadataItem{Key: "expected_object_id", Value: firstCommitID},
+						structerr.MetadataItem{Key: "reference", Value: "refs/heads/" + branchName},
 					),
 				}
 			},
diff --git a/internal/gitaly/service/operations/submodules.go b/internal/gitaly/service/operations/submodules.go
index 2de4167d6..036129fea 100644
--- a/internal/gitaly/service/operations/submodules.go
+++ b/internal/gitaly/service/operations/submodules.go
@@ -253,7 +253,10 @@ func (s *Server) userUpdateSubmodule(ctx context.Context, req *gitalypb.UserUpda
 		var updateRefError updateref.Error
 		if errors.As(err, &updateRefError) {
 			return &gitalypb.UserUpdateSubmoduleResponse{
-				CommitError: err.Error(),
+				// TODO: this needs to be converted to a structured error, and once done we should stop
+				// returning this Ruby-esque error message in favor of the actual error that was
+				// returned by `updateReferenceWithHooks()`.
+				CommitError: fmt.Sprintf("Could not update %s. Please refresh and try again.", updateRefError.Reference),
 			}, nil
 		}
 
diff --git a/internal/gitaly/service/operations/tags_test.go b/internal/gitaly/service/operations/tags_test.go
index bccb397fe..ba8a9f756 100644
--- a/internal/gitaly/service/operations/tags_test.go
+++ b/internal/gitaly/service/operations/tags_test.go
@@ -267,10 +267,11 @@ func TestUserDeleteTag(t *testing.T) {
 						User:           gittest.TestUser,
 						ExpectedOldOid: firstCommit.String(),
 					},
-					expectedError: testhelper.WithInterceptedMetadata(
-						structerr.NewFailedPrecondition("Could not update refs/tags/%s. Please refresh and try again.", tagName),
-						"stderr",
-						fmt.Sprintf("fatal: prepare: cannot lock ref 'refs/tags/%s': is at %s but expected %s\n", tagName, secondCommit, firstCommit),
+					expectedError: testhelper.WithInterceptedMetadataItems(
+						structerr.NewFailedPrecondition("reference update: reference does not point to expected object"),
+						structerr.MetadataItem{Key: "actual_object_id", Value: secondCommit},
+						structerr.MetadataItem{Key: "expected_object_id", Value: firstCommit},
+						structerr.MetadataItem{Key: "reference", Value: "refs/tags/" + tagName},
 					),
 					expectedTags: []string{"ganymede"},
 				}
diff --git a/internal/structerr/error.go b/internal/structerr/error.go
index 184b0da90..048258b6b 100644
--- a/internal/structerr/error.go
+++ b/internal/structerr/error.go
@@ -269,24 +269,9 @@ func (e Error) errorChain() []Error {
 	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.
+// Metadata returns the Error's metadata. Please refer to `ExtractMetadata()` for the exact semantics of this function.
 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
+	return ExtractMetadata(e)
 }
 
 // MetadataItems returns a copy of all metadata items added to this error. This function has the
@@ -356,3 +341,40 @@ func (e Error) WithGRPCCode(code codes.Code) Error {
 	e.code = code
 	return e
 }
+
+// ErrorMetadater is an interface that can be implemented by error types in order to provide custom metadata items
+// without itself being a `structerr.Error`.
+type ErrorMetadater interface {
+	// ErrorMetadata returns the list of metadata items attached to this error.
+	ErrorMetadata() []MetadataItem
+}
+
+// ExtractMetadata extracts metadata from the given error if any of the errors in its chain contain any. Errors may
+// contain in case they are either a `structerr.Error` or in case they implement the `ErrorMetadater` interface. 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 ExtractMetadata(err error) map[string]any {
+	metadata := map[string]any{}
+
+	for ; err != nil; err = errors.Unwrap(err) {
+		var metadataItems []MetadataItem
+		if structerr, ok := err.(Error); ok {
+			metadataItems = structerr.metadata
+		} else if errorMetadater, ok := err.(ErrorMetadater); ok {
+			metadataItems = errorMetadater.ErrorMetadata()
+		} else {
+			continue
+		}
+
+		for _, m := range metadataItems {
+			if _, exists := metadata[m.Key]; !exists {
+				metadata[m.Key] = m.Value
+			}
+		}
+	}
+
+	return metadata
+}
diff --git a/internal/structerr/error_test.go b/internal/structerr/error_test.go
index c1a65b689..a998e5329 100644
--- a/internal/structerr/error_test.go
+++ b/internal/structerr/error_test.go
@@ -325,6 +325,18 @@ func TestError_Is(t *testing.T) {
 	}
 }
 
+type customWithMetadataError struct{}
+
+func (customWithMetadataError) Error() string {
+	return "custom error"
+}
+
+func (customWithMetadataError) ErrorMetadata() []MetadataItem {
+	return []MetadataItem{
+		{Key: "custom", Value: "error"},
+	}
+}
+
 func TestError_Metadata(t *testing.T) {
 	t.Parallel()
 
@@ -338,6 +350,7 @@ func TestError_Metadata(t *testing.T) {
 			expectedItemsByKey[item.Key] = item.Value
 		}
 		require.Equal(t, expectedItemsByKey, err.Metadata())
+		require.Equal(t, expectedItemsByKey, ExtractMetadata(err))
 	}
 
 	t.Run("without metadata", func(t *testing.T) {
@@ -478,6 +491,18 @@ func TestError_Metadata(t *testing.T) {
 			{Key: "b", Value: "b"},
 		})
 	})
+
+	t.Run("custom type with metadata", func(t *testing.T) {
+		err := New("top-level: %w", customWithMetadataError{})
+
+		require.Equal(t, Error{
+			err:  fmt.Errorf("top-level: %w", customWithMetadataError{}),
+			code: codes.Unknown,
+		}, err)
+		requireItems(t, err, []MetadataItem{
+			{Key: "custom", Value: "error"},
+		})
+	})
 }
 
 func TestError_Details(t *testing.T) {
diff --git a/internal/structerr/grpc_server.go b/internal/structerr/grpc_server.go
index 367f149e7..d2f565849 100644
--- a/internal/structerr/grpc_server.go
+++ b/internal/structerr/grpc_server.go
@@ -2,7 +2,6 @@ package structerr
 
 import (
 	"context"
-	"errors"
 
 	"github.com/sirupsen/logrus"
 )
@@ -10,13 +9,7 @@ import (
 // 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
-		}
-
+	if metadata := ExtractMetadata(err); len(metadata) > 0 {
 		return logrus.Fields{
 			"error_metadata": metadata,
 		}
diff --git a/internal/testhelper/testserver/structerr_interceptors.go b/internal/testhelper/testserver/structerr_interceptors.go
index 26b53d63c..6c4ffb4b1 100644
--- a/internal/testhelper/testserver/structerr_interceptors.go
+++ b/internal/testhelper/testserver/structerr_interceptors.go
@@ -2,7 +2,6 @@ package testserver
 
 import (
 	"context"
-	"errors"
 	"sort"
 
 	"gitlab.com/gitlab-org/gitaly/v16/internal/structerr"
@@ -26,13 +25,7 @@ func StructErrStreamInterceptor(srv interface{}, stream grpc.ServerStream, info
 }
 
 func interceptedError(err error) error {
-	var structErr structerr.Error
-	if errors.As(err, &structErr) {
-		metadata := structErr.Metadata()
-		if len(metadata) == 0 {
-			return err
-		}
-
+	if metadata := structerr.ExtractMetadata(err); len(metadata) > 0 {
 		keys := make([]string, 0, len(metadata))
 		for key := range metadata {
 			keys = append(keys, key)