Merge branch 'sh-improve-pre-receive-error-ff-merge-message' into 'master'

Improve merge error when pre-receive hooks fail in fast-forward merge

See merge request gitlab-org/gitlab!44843

changelogs/unreleased/sh-improve-pre-receive-error-ff-merge-message.yml

+---
+title: Improve merge error when pre-receive hooks fail in fast-forward merge
+merge_request: 44843
+author:
+type: fixed

lib/gitlab/git/pre_receive_error.rb

@@ -18,13 +18,15 @@ module Gitlab
 
       attr_reader :raw_message
 
-      def initialize(message = '', user_message = '')
+      def initialize(message = '', fallback_message: '')
         @raw_message = message
 
-        if user_message.present?
-          super(sanitize(user_message))
+        sanitized_msg = sanitize(message)
+
+        if sanitized_msg.present?
+          super(sanitized_msg)
         else
-          super(sanitize(message))
+          super(fallback_message)
         end
       end
 

lib/gitlab/gitaly_client/operation_service.rb

@@ -179,7 +179,7 @@ module Gitlab
         )
 
         if response.pre_receive_error.present?
-          raise Gitlab::Git::PreReceiveError.new(response.pre_receive_error, "GL-HOOK-ERR: pre-receive hook failed.")
+          raise Gitlab::Git::PreReceiveError.new(response.pre_receive_error, fallback_message: "pre-receive hook failed.")
         end
 
         Gitlab::Git::OperationService::BranchUpdate.from_gitaly(response.branch_update)

spec/lib/gitlab/git/pre_receive_error_spec.rb

@@ -21,13 +21,21 @@ RSpec.describe Gitlab::Git::PreReceiveError do
         expect(ex.raw_message).to eq(raw_message)
       end
 
-      it 'sanitizes the user message' do
-        raw_message = 'Raw message'
-        ex = described_class.new(raw_message, "#{prefix} User message")
+      it 'prefers the original message over the fallback' do
+        raw_message = "#{prefix} Hello,\nworld!"
+        ex = described_class.new(raw_message, fallback_message: "User message")
 
+        expect(ex.message).to eq('Hello,')
         expect(ex.raw_message).to eq(raw_message)
-        expect(ex.message).to eq('User message')
       end
     end
+
+    it 'uses the fallback message' do
+      raw_message = 'Hello\n'
+      ex = described_class.new(raw_message, fallback_message: "User message")
+
+      expect(ex.raw_message).to eq(raw_message)
+      expect(ex.message).to eq('User message')
+    end
   end
 end

spec/services/merge_requests/ff_merge_service_spec.rb

@@ -114,7 +114,7 @@ RSpec.describe MergeRequests::FfMergeService do
         error_message = 'error message'
         raw_message = 'The truth is out there'
 
-        pre_receive_error = Gitlab::Git::PreReceiveError.new(raw_message, "GitLab: #{error_message}")
+        pre_receive_error = Gitlab::Git::PreReceiveError.new(raw_message, fallback_message: error_message)
         allow(service).to receive(:repository).and_raise(pre_receive_error)
         allow(service).to receive(:execute_hooks)
         expect(Gitlab::ErrorTracking).to receive(:track_exception).with(