From 672499801ed04c5009a81fd883e37bf82a55e5f1 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Tue, 30 Nov 2021 00:48:45 -0800
Subject: [PATCH] Show rebase pre-receive error to user

If a project has a push rule configured, a rebase may fail quietly with
the message, "Rebase failed. Please rebase locally. Please try again."
The user has no idea why this rebase failed.

Since pre-receive messages are sanitized to show only text that starts
with "GitLab:", we can safely display these to the user in
`RebaseService`, just as we do in `MergeService`.

Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/213608
---
 app/services/merge_requests/rebase_service.rb | 18 +++++++++++++---
 .../merge_requests/rebase_service_spec.rb     | 21 +++++++++++++++++++
 2 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/app/services/merge_requests/rebase_service.rb b/app/services/merge_requests/rebase_service.rb
index 9423194c01dcd8..d1f45b4b49cffc 100644
--- a/app/services/merge_requests/rebase_service.rb
+++ b/app/services/merge_requests/rebase_service.rb
@@ -4,7 +4,7 @@ module MergeRequests
   class RebaseService < MergeRequests::BaseService
     REBASE_ERROR = 'Rebase failed. Please rebase locally'
 
-    attr_reader :merge_request
+    attr_reader :merge_request, :rebase_error
 
     def execute(merge_request, skip_ci: false)
       @merge_request = merge_request
@@ -13,7 +13,7 @@ def execute(merge_request, skip_ci: false)
       if rebase
         success
       else
-        error(REBASE_ERROR)
+        error(rebase_error)
       end
     end
 
@@ -22,11 +22,23 @@ def rebase
 
       true
     rescue StandardError => e
-      log_error(exception: e, message: REBASE_ERROR, save_message_on_model: true)
+      set_rebase_error(e)
+      log_error(exception: e, message: rebase_error, save_message_on_model: true)
 
       false
     ensure
       merge_request.update_column(:rebase_jid, nil)
     end
+
+    private
+
+    def set_rebase_error(exception)
+      @rebase_error =
+        if exception.is_a?(Gitlab::Git::PreReceiveError)
+          "Something went wrong during the rebase pre-receive hook: #{exception.message}."
+        else
+          REBASE_ERROR
+        end
+    end
   end
 end
diff --git a/spec/services/merge_requests/rebase_service_spec.rb b/spec/services/merge_requests/rebase_service_spec.rb
index ca561376581573..e671bbf2cd6783 100644
--- a/spec/services/merge_requests/rebase_service_spec.rb
+++ b/spec/services/merge_requests/rebase_service_spec.rb
@@ -80,6 +80,27 @@
       end
     end
 
+    context 'with a pre-receive failure' do
+      let(:pre_receive_error) { "Commit message does not follow the pattern 'ACME'" }
+      let(:merge_error) { "Something went wrong during the rebase pre-receive hook: #{pre_receive_error}." }
+
+      before do
+        allow(repository).to receive(:gitaly_operation_client).and_raise(Gitlab::Git::PreReceiveError, "GitLab: #{pre_receive_error}")
+      end
+
+      it 'saves a specific message' do
+        subject.execute(merge_request)
+
+        expect(merge_request.reload.merge_error).to eq merge_error
+      end
+
+      it 'returns an error' do
+        expect(service.execute(merge_request)).to match(
+          status: :error,
+          message: merge_error)
+      end
+    end
+
     context 'with git command failure' do
       before do
         allow(repository).to receive(:gitaly_operation_client).and_raise(Gitlab::Git::Repository::GitError, 'Something went wrong')
-- 
GitLab