Add support for PreReceiveError in UserMergeBranch

Closes #978

3 files changed, 75 insertions, 8 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index bb8ea020c..cd69e41a8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 UNRELEASED
 
+- Add support for PreReceiveError in UserMergeBranch
+  https://gitlab.com/gitlab-org/gitaly/merge_requests/573
 - Add support for Refs field in DeleteRefs RPC
   https://gitlab.com/gitlab-org/gitaly/merge_requests/565
 - Wait between ruby worker removal from pool and graceful shutdown
diff --git a/internal/service/operations/merge_test.go b/internal/service/operations/merge_test.go
index 2f666e1ea..38ceb024b 100644
--- a/internal/service/operations/merge_test.go
+++ b/internal/service/operations/merge_test.go
@@ -224,6 +224,67 @@ func TestFailedMergeConcurrentUpdate(t *testing.T) {
 	require.Equal(t, commit.Id, concurrentCommitID, "RPC should not have trampled concurrent update")
 }
 
+func TestFailedMergeDueToHooks(t *testing.T) {
+	testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	server, serverSocketPath := runOperationServiceServer(t)
+	defer server.Stop()
+
+	client, conn := newOperationClient(t, serverSocketPath)
+	defer conn.Close()
+
+	prepareMergeBranch(t, testRepoPath)
+
+	hookContent := []byte("#!/bin/sh\necho 'failure'\nexit 1")
+
+	for _, hookName := range gitlabPreHooks {
+		t.Run(hookName, func(t *testing.T) {
+			require.NoError(t, os.MkdirAll(path.Join(testRepoPath, "hooks"), 0755))
+			hookPath := path.Join(testRepoPath, "hooks", hookName)
+			ioutil.WriteFile(hookPath, hookContent, 0755)
+			defer os.Remove(hookPath)
+
+			ctx, cancel := testhelper.Context()
+			defer cancel()
+
+			mergeBidi, err := client.UserMergeBranch(ctx)
+			require.NoError(t, err)
+
+			mergeCommitMessage := "Merged by Gitaly"
+			firstRequest := &pb.UserMergeBranchRequest{
+				Repository: testRepo,
+				User:       mergeUser,
+				CommitId:   commitToMerge,
+				Branch:     []byte(mergeBranchName),
+				Message:    []byte(mergeCommitMessage),
+			}
+
+			require.NoError(t, mergeBidi.Send(firstRequest), "send first request")
+
+			_, err = mergeBidi.Recv()
+			require.NoError(t, err, "receive first response")
+
+			require.NoError(t, mergeBidi.Send(&pb.UserMergeBranchRequest{Apply: true}), "apply merge")
+			require.NoError(t, mergeBidi.CloseSend(), "close send")
+
+			secondResponse, err := mergeBidi.Recv()
+			require.NoError(t, err, "receive second response")
+			require.Contains(t, secondResponse.PreReceiveError, "failure")
+
+			err = consumeEOF(func() error {
+				_, err = mergeBidi.Recv()
+				return err
+			})
+			require.NoError(t, err, "consume EOF")
+
+			currentBranchHead := testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "rev-parse", mergeBranchName)
+			require.Equal(t, mergeBranchHeadBefore, strings.TrimSpace(string(currentBranchHead)), "branch head updated")
+		})
+	}
+
+}
+
 func TestSuccessfulUserFFBranchRequest(t *testing.T) {
 	ctx, cancel := testhelper.Context()
 	defer cancel()
diff --git a/ruby/lib/gitaly_server/operations_service.rb b/ruby/lib/gitaly_server/operations_service.rb
index f20428f1d..3626e6c10 100644
--- a/ruby/lib/gitaly_server/operations_service.rb
+++ b/ruby/lib/gitaly_server/operations_service.rb
@@ -113,16 +113,20 @@ module GitalyServer
           target_branch = first_request.branch.dup
           message = first_request.message.dup
 
-          result = repository.merge(user, source_sha, target_branch, message) do |commit_id|
-            y << Gitaly::UserMergeBranchResponse.new(commit_id: commit_id)
-
-            second_request = session.next
-            unless second_request.apply
-              raise GRPC::FailedPrecondition.new('merge aborted by client')
+          begin
+            result = repository.merge(user, source_sha, target_branch, message) do |commit_id|
+              y << Gitaly::UserMergeBranchResponse.new(commit_id: commit_id)
+
+              second_request = session.next
+              unless second_request.apply
+                raise GRPC::FailedPrecondition.new('merge aborted by client')
+              end
             end
-          end
 
-          y << Gitaly::UserMergeBranchResponse.new(branch_update: branch_update_result(result))
+            y << Gitaly::UserMergeBranchResponse.new(branch_update: branch_update_result(result))
+          rescue Gitlab::Git::HooksService::PreReceiveError => e
+            y << Gitaly::UserMergeBranchResponse.new(pre_receive_error: e.message)
+          end
         end
       end
     end