Commit 72af0e73 authored by Robert Speicher's avatar Robert Speicher

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

Improve error logging of MergeService

Relates to #23505

See merge request !6975
parents d22d8e8f 75b7ba3f
...@@ -11,14 +11,14 @@ module MergeRequests ...@@ -11,14 +11,14 @@ module MergeRequests
def execute(merge_request) def execute(merge_request)
@merge_request = 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 merge_request.in_locked_state do
if commit if commit
after_merge after_merge
success success
else else
error('Can not merge changes') log_merge_error('Can not merge changes', true)
end end
end end
end end
...@@ -46,8 +46,8 @@ module MergeRequests ...@@ -46,8 +46,8 @@ module MergeRequests
merge_request.update(merge_error: e.message) merge_request.update(merge_error: e.message)
false false
rescue StandardError => e rescue StandardError => e
merge_request.update(merge_error: "Something went wrong during merge") merge_request.update(merge_error: "Something went wrong during merge: #{e.message}")
Rails.logger.error(e.message) log_merge_error(e.message)
false false
ensure ensure
merge_request.update(in_progress_merge_commit_sha: nil) merge_request.update(in_progress_merge_commit_sha: nil)
...@@ -65,5 +65,17 @@ module MergeRequests ...@@ -65,5 +65,17 @@ module MergeRequests
def branch_deletion_user def branch_deletion_user
@merge_request.force_remove_source_branch? ? @merge_request.author : current_user @merge_request.force_remove_source_branch? ? @merge_request.author : current_user
end 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
end end
...@@ -120,13 +120,13 @@ describe MergeRequests::MergeService, services: true do ...@@ -120,13 +120,13 @@ describe MergeRequests::MergeService, services: true do
let(:service) { MergeRequests::MergeService.new(project, user, commit_message: 'Awesome message') } let(:service) { MergeRequests::MergeService.new(project, user, commit_message: 'Awesome message') }
it 'saves error if there is an exception' do 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) allow(service).to receive(:execute_hooks)
service.execute(merge_request) 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 end
it 'saves error if there is an PreReceiveError exception' do it 'saves error if there is an PreReceiveError exception' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment