From f0574450519054cf64088d118fb5dcabf8d04a08 Mon Sep 17 00:00:00 2001 From: James Liu Date: Wed, 1 Oct 2025 11:18:44 +1000 Subject: [PATCH] log: Capture stderr of git-diff in CommitDiff When git-diff(1) fails on the cmd.Wait() call within the eachDiff iterator inside the CommiDiff RPC handler, we return a generic error message in the RPC response. The actual error message produced by the child process is avilable in stderr. Capture stderr and return it to the client as gRPC metadata. This will also ensure it's logged for further analysis. --- internal/gitaly/service/diff/commit_diff_test.go | 7 ++++++- internal/gitaly/service/diff/utils.go | 8 ++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/internal/gitaly/service/diff/commit_diff_test.go b/internal/gitaly/service/diff/commit_diff_test.go index fc7cd89f7bd..40b21f7ea3a 100644 --- a/internal/gitaly/service/diff/commit_diff_test.go +++ b/internal/gitaly/service/diff/commit_diff_test.go @@ -2,6 +2,7 @@ package diff import ( "errors" + "fmt" "io" "strings" "testing" @@ -1134,7 +1135,11 @@ func TestCommitDiff_nonexistentCommit(t *testing.T) { }) require.NoError(t, err) - testhelper.RequireGrpcError(t, structerr.NewFailedPrecondition("eachDiff: exit status 128"), drainCommitDiffResponse(stream)) + testhelper.RequireGrpcError(t, testhelper.WithInterceptedMetadata( + structerr.NewFailedPrecondition("eachDiff: exit status 128"), + "stderr", fmt.Sprintf("fatal: bad object %s\n", nonExistentCommitID.String()), + ), + drainCommitDiffResponse(stream)) } func getDiffsFromCommitDiffClient(t *testing.T, client gitalypb.DiffService_CommitDiffClient) []*diff.Diff { diff --git a/internal/gitaly/service/diff/utils.go b/internal/gitaly/service/diff/utils.go index 5b53cd63e6a..81558182ded 100644 --- a/internal/gitaly/service/diff/utils.go +++ b/internal/gitaly/service/diff/utils.go @@ -1,6 +1,7 @@ package diff import ( + "bytes" "context" "errors" @@ -36,7 +37,8 @@ func validateRequest(ctx context.Context, locator storage.Locator, in requestWit func (s *server) eachDiff(ctx context.Context, repo *localrepo.Repo, objectHash git.ObjectHash, subCmd gitcmd.Command, limits diff.Limits, callback func(*diff.Diff) error) error { diffConfig := gitcmd.ConfigPair{Key: "diff.noprefix", Value: "false"} - cmd, err := repo.Exec(ctx, subCmd, gitcmd.WithConfig(diffConfig), gitcmd.WithSetupStdout()) + var cmdStderr bytes.Buffer + cmd, err := repo.Exec(ctx, subCmd, gitcmd.WithConfig(diffConfig), gitcmd.WithSetupStdout(), gitcmd.WithStderr(&cmdStderr)) if err != nil { return structerr.NewInternal("cmd: %w", err) } @@ -54,7 +56,9 @@ func (s *server) eachDiff(ctx context.Context, repo *localrepo.Repo, objectHash } if err := cmd.Wait(); err != nil { - return structerr.NewFailedPrecondition("%w", err) + return structerr.NewFailedPrecondition("%w", err).WithMetadataItems( + structerr.MetadataItem{Key: "stderr", Value: cmdStderr.String()}, + ) } return nil -- GitLab