Merge branch 'sh-track-ff-merge-pre-receive-errors' into 'master'

Log raw pre-receive message in fast-forward merge

See merge request gitlab-org/gitlab!38354

app/services/merge_requests/ff_merge_service.rb

@@ -22,6 +22,7 @@ module MergeRequests
 
       ff_merge
     rescue Gitlab::Git::PreReceiveError => e
+      Gitlab::ErrorTracking.track_exception(e, pre_receive_message: e.raw_message, merge_request_id: merge_request&.id)
       raise MergeError, e.message
     rescue StandardError => e
       raise MergeError, "Something went wrong during merge: #{e.message}"

changelogs/unreleased/sh-track-ff-merge-pre-receive-errors.yml

+---
+title: Log raw pre-receive message in fast-forward merge
+merge_request: 38354
+author:
+type: other

lib/gitlab/git/pre_receive_error.rb

@@ -16,9 +16,17 @@ module Gitlab
 
       SAFE_MESSAGE_REGEX = /^(#{SAFE_MESSAGE_PREFIXES.join('|')})\s*(?<safe_message>.+)/.freeze
 
-      def initialize(message = '')
+      attr_reader :raw_message
+
+      def initialize(message = '', user_message = '')
+        @raw_message = message
+
+        if user_message.present?
+          super(sanitize(user_message))
+        else
           super(sanitize(message))
         end
+      end
 
       private
 

lib/gitlab/gitaly_client/operation_service.rb

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

spec/lib/gitlab/git/pre_receive_error_spec.rb

@@ -6,15 +6,27 @@ RSpec.describe Gitlab::Git::PreReceiveError do
   Gitlab::Git::PreReceiveError::SAFE_MESSAGE_PREFIXES.each do |prefix|
     context "error messages prefixed with #{prefix}" do
       it 'accepts only errors lines with the prefix' do
-        ex = described_class.new("#{prefix} Hello,\nworld!")
+        raw_message = "#{prefix} Hello,\nworld!"
+        ex = described_class.new(raw_message)
 
         expect(ex.message).to eq('Hello,')
+        expect(ex.raw_message).to eq(raw_message)
       end
 
       it 'makes its message HTML-friendly' do
-        ex = described_class.new("#{prefix} Hello,\n#{prefix} world!\n")
+        raw_message = "#{prefix} Hello,\n#{prefix} world!\n"
+        ex = described_class.new(raw_message)
 
         expect(ex.message).to eq('Hello,<br>world!')
+        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")
+
+        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

@@ -113,9 +113,16 @@ RSpec.describe MergeRequests::FfMergeService do
 
       it 'logs and saves error if there is an PreReceiveError exception' do
         error_message = 'error message'
+        raw_message = 'The truth is out there'
 
-        allow(service).to receive(:repository).and_raise(Gitlab::Git::PreReceiveError, "GitLab: #{error_message}")
+        pre_receive_error = Gitlab::Git::PreReceiveError.new(raw_message, "GitLab: #{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(
+          pre_receive_error,
+          pre_receive_message: raw_message,
+          merge_request_id: merge_request.id
+        )
 
         service.execute(merge_request)