operations: Return error from UserSquash on conflict

We do not return an error in case resolving rebasing commits in
UserSquash call fails. This makes us blind for the error conditions in
this RPC, which is in turn creating problems in Praefect setups where we
have to rely on errors in order to decide whether we have to create
replication jobs or not.

Return structured errors in case the error condition triggers such that
callers can tell what kind of error they have been hitting.

Changelog: changed

4 files changed, 70 insertions, 6 deletions

diff --git a/cmd/gitaly-git2go/rebase.go b/cmd/gitaly-git2go/rebase.go
index 8d6d44010..0ca365ff4 100644
--- a/cmd/gitaly-git2go/rebase.go
+++ b/cmd/gitaly-git2go/rebase.go
@@ -149,6 +149,22 @@ func (cmd *rebaseSubcommand) rebase(ctx context.Context, request *git2go.RebaseC
 		}
 
 		if err := rebase.Commit(op.Id, nil, &committer, commit.Message()); err != nil {
+			if git.IsErrorCode(err, git.ErrorCodeUnmerged) {
+				index, err := rebase.InmemoryIndex()
+				if err != nil {
+					return "", fmt.Errorf("getting conflicting index: %w", err)
+				}
+
+				conflictingFiles, err := getConflictingFiles(index)
+				if err != nil {
+					return "", fmt.Errorf("getting conflicting files: %w", err)
+				}
+
+				return "", fmt.Errorf("commit %q: %w", op.Id.String(), git2go.ConflictingFilesError{
+					ConflictingFiles: conflictingFiles,
+				})
+			}
+
 			// If the commit has already been applied on the target branch then we can
 			// skip it if we were told to.
 			if request.SkipEmptyCommits && git.IsErrorCode(err, git.ErrorCodeApplied) {
diff --git a/cmd/gitaly-git2go/rebase_test.go b/cmd/gitaly-git2go/rebase_test.go
index 8aa7e013f..5c6e84299 100644
--- a/cmd/gitaly-git2go/rebase_test.go
+++ b/cmd/gitaly-git2go/rebase_test.go
@@ -156,7 +156,7 @@ func TestRebase_rebase(t *testing.T) {
 		{
 			desc:        "Rebase with conflict",
 			branch:      "rebase-encoding-failure-trigger",
-			expectedErr: "rebase: commit \"eb8f5fb9523b868cef583e09d4bf70b99d2dd404\": conflicts have not been resolved",
+			expectedErr: "rebase: commit \"eb8f5fb9523b868cef583e09d4bf70b99d2dd404\": there are conflicting files",
 		},
 		{
 			desc:        "Orphaned branch",
diff --git a/internal/gitaly/service/operations/squash.go b/internal/gitaly/service/operations/squash.go
index 270fdd5d3..b4bc3f644 100644
--- a/internal/gitaly/service/operations/squash.go
+++ b/internal/gitaly/service/operations/squash.go
@@ -183,6 +183,35 @@ func (s *Server) userSquash(ctx context.Context, req *gitalypb.UserSquashRequest
 		SkipEmptyCommits: true,
 	})
 	if err != nil {
+		if featureflag.UserSquashImprovedErrorHandling.IsEnabled(ctx) {
+			var conflictErr git2go.ConflictingFilesError
+
+			if errors.As(err, &conflictErr) {
+				conflictingFiles := make([][]byte, 0, len(conflictErr.ConflictingFiles))
+				for _, conflictingFile := range conflictErr.ConflictingFiles {
+					conflictingFiles = append(conflictingFiles, []byte(conflictingFile))
+				}
+
+				detailedErr, err := helper.ErrWithDetails(
+					helper.ErrInternalf("rebasing commits: %w", err),
+					&gitalypb.UserSquashError{
+						Error: &gitalypb.UserSquashError_RebaseConflict{
+							RebaseConflict: &gitalypb.MergeConflictError{
+								ConflictingFiles: conflictingFiles,
+							},
+						},
+					},
+				)
+				if err != nil {
+					return "", helper.ErrInternalf("error details: %w", err)
+				}
+
+				return "", detailedErr
+			}
+
+			return "", helper.ErrInternalf("rebasing commits: %w", err)
+		}
+
 		return "", fmt.Errorf("rebasing end onto start commit: %w", gitError{
 			Err:    err,
 			ErrMsg: err.Error(),
diff --git a/internal/gitaly/service/operations/squash_test.go b/internal/gitaly/service/operations/squash_test.go
index fae48985a..668503535 100644
--- a/internal/gitaly/service/operations/squash_test.go
+++ b/internal/gitaly/service/operations/squash_test.go
@@ -486,8 +486,11 @@ func TestUserSquash_validation(t *testing.T) {
 }
 
 func TestUserSquash_conflicts(t *testing.T) {
+	testhelper.NewFeatureSets(featureflag.UserSquashImprovedErrorHandling).Run(t, testUserSquashConflicts)
+}
+
+func testUserSquashConflicts(t *testing.T, ctx context.Context) {
 	t.Parallel()
-	ctx := testhelper.Context(t)
 
 	ctx, cfg, repo, repoPath, client := setupOperationsService(t, ctx)
 
@@ -514,11 +517,27 @@ func TestUserSquash_conflicts(t *testing.T) {
 		StartSha:      theirs.String(),
 		EndSha:        ours.String(),
 	})
-	require.NoError(t, err)
 
-	testhelper.ProtoEqual(t, &gitalypb.UserSquashResponse{
-		GitError: fmt.Sprintf("rebase: commit %q: conflicts have not been resolved", ours),
-	}, response)
+	if featureflag.UserSquashImprovedErrorHandling.IsEnabled(ctx) {
+		testhelper.RequireGrpcError(t, errWithDetails(t,
+			helper.ErrInternalf("rebasing commits: rebase: commit %q: there are conflicting files", ours),
+			&gitalypb.UserSquashError{
+				Error: &gitalypb.UserSquashError_RebaseConflict{
+					RebaseConflict: &gitalypb.MergeConflictError{
+						ConflictingFiles: [][]byte{
+							[]byte("b"),
+						},
+					},
+				},
+			},
+		), err)
+		require.Nil(t, response)
+	} else {
+		require.NoError(t, err)
+		testhelper.ProtoEqual(t, &gitalypb.UserSquashResponse{
+			GitError: fmt.Sprintf("rebase: commit %q: there are conflicting files", ours),
+		}, response)
+	}
 }
 
 func TestUserSquash_ancestry(t *testing.T) {