Merge branch 'sh-improve-merge-service-logging' into 'master'

Improve error logging of MergeService

Relates to #23505

See merge request !6975

2 files changed, 18 insertions, 6 deletions

diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb
index b037780c431..ab9056a3250 100644
--- a/app/services/merge_requests/merge_service.rb
+++ b/app/services/merge_requests/merge_service.rb
@@ -11,14 +11,14 @@ module MergeRequests
     def execute(merge_request)
       @merge_request = merge_request
 
-      return error('Merge request is not mergeable') unless @merge_request.mergeable?
+      return log_merge_error('Merge request is not mergeable', true) unless @merge_request.mergeable?
 
       merge_request.in_locked_state do
         if commit
           after_merge
           success
         else
-          error('Can not merge changes')
+          log_merge_error('Can not merge changes', true)
         end
       end
     end
@@ -46,8 +46,8 @@ module MergeRequests
       merge_request.update(merge_error: e.message)
       false
     rescue StandardError => e
-      merge_request.update(merge_error: "Something went wrong during merge")
-      Rails.logger.error(e.message)
+      merge_request.update(merge_error: "Something went wrong during merge: #{e.message}")
+      log_merge_error(e.message)
       false
     ensure
       merge_request.update(in_progress_merge_commit_sha: nil)
@@ -65,5 +65,17 @@ module MergeRequests
     def branch_deletion_user
       @merge_request.force_remove_source_branch? ? @merge_request.author : current_user
     end
+
+    def log_merge_error(message, http_error = false)
+      Rails.logger.error("MergeService ERROR: #{merge_request_info} - #{message}")
+
+      error(message) if http_error
+    end
+
+    def merge_request_info
+      project = merge_request.project
+
+      "#{project.to_reference}#{merge_request.to_reference}"
+    end
   end
 end
diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb
index ee53e110aee..9163c0c104e 100644
--- a/spec/services/merge_requests/merge_service_spec.rb
+++ b/spec/services/merge_requests/merge_service_spec.rb
@@ -120,13 +120,13 @@ describe MergeRequests::MergeService, services: true do
       let(:service) { MergeRequests::MergeService.new(project, user, commit_message: 'Awesome message') }
 
       it 'saves error if there is an exception' do
-        allow(service).to receive(:repository).and_raise("error")
+        allow(service).to receive(:repository).and_raise("error message")
 
         allow(service).to receive(:execute_hooks)
 
         service.execute(merge_request)
 
-        expect(merge_request.merge_error).to eq("Something went wrong during merge")
+        expect(merge_request.merge_error).to eq("Something went wrong during merge: error message")
       end
 
       it 'saves error if there is an PreReceiveError exception' do