Commit 5b21ff0d authored by Patrick Bajao's avatar Patrick Bajao Committed by Alex Kalderimis

Show more appropriate error message when merging fails

Show a generic error when there are exceptions but still log
the error.

If no exception occurred but the commit wasn't created or other
exceptions was raised, show a generic failure message.
parent 2045bd62
...@@ -8,6 +8,8 @@ module MergeRequests ...@@ -8,6 +8,8 @@ module MergeRequests
# Executed when you do merge via GitLab UI # Executed when you do merge via GitLab UI
# #
class MergeService < MergeRequests::MergeBaseService class MergeService < MergeRequests::MergeBaseService
GENERIC_ERROR_MESSAGE = 'An error occurred while merging'
delegate :merge_jid, :state, to: :@merge_request delegate :merge_jid, :state, to: :@merge_request
def execute(merge_request, options = {}) def execute(merge_request, options = {})
...@@ -79,7 +81,7 @@ module MergeRequests ...@@ -79,7 +81,7 @@ module MergeRequests
if commit_id if commit_id
log_info("Git merge finished on JID #{merge_jid} commit #{commit_id}") log_info("Git merge finished on JID #{merge_jid} commit #{commit_id}")
else else
raise_error('Conflicts detected during merge') raise_error(GENERIC_ERROR_MESSAGE)
end end
merge_request.update!(merge_commit_sha: commit_id) merge_request.update!(merge_commit_sha: commit_id)
...@@ -96,7 +98,7 @@ module MergeRequests ...@@ -96,7 +98,7 @@ module MergeRequests
"Something went wrong during merge pre-receive hook. #{e.message}".strip "Something went wrong during merge pre-receive hook. #{e.message}".strip
rescue => e rescue => e
handle_merge_error(log_message: e.message) handle_merge_error(log_message: e.message)
raise_error('Something went wrong during merge') raise_error(GENERIC_ERROR_MESSAGE)
end end
def after_merge def after_merge
......
---
title: Show more appropriate error message when merging fails
merge_request: 52671
author:
type: fixed
...@@ -388,7 +388,7 @@ RSpec.describe 'Merge request > User sees merge widget', :js do ...@@ -388,7 +388,7 @@ RSpec.describe 'Merge request > User sees merge widget', :js do
click_button 'Merge' click_button 'Merge'
page.within('.mr-widget-body') do page.within('.mr-widget-body') do
expect(page).to have_content('Conflicts detected during merge') expect(page).to have_content('An error occurred while merging')
end end
end end
end end
......
...@@ -310,12 +310,12 @@ RSpec.describe MergeRequests::MergeService do ...@@ -310,12 +310,12 @@ RSpec.describe MergeRequests::MergeService do
it 'logs and saves error if there is an exception' do it 'logs and saves error if there is an exception' do
error_message = 'error message' error_message = 'error message'
allow(service).to receive(:repository).and_raise('error message') 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 include('Something went wrong during merge') expect(merge_request.merge_error).to eq(described_class::GENERIC_ERROR_MESSAGE)
expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message)) expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message))
end end
...@@ -343,9 +343,7 @@ RSpec.describe MergeRequests::MergeService do ...@@ -343,9 +343,7 @@ RSpec.describe MergeRequests::MergeService do
expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message)) expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message))
end end
it 'logs and saves error if there is a merge conflict' do it 'logs and saves error if commit is not created' do
error_message = 'Conflicts detected during merge'
allow_any_instance_of(Repository).to receive(:merge).and_return(false) allow_any_instance_of(Repository).to receive(:merge).and_return(false)
allow(service).to receive(:execute_hooks) allow(service).to receive(:execute_hooks)
...@@ -353,8 +351,8 @@ RSpec.describe MergeRequests::MergeService do ...@@ -353,8 +351,8 @@ RSpec.describe MergeRequests::MergeService do
expect(merge_request).to be_open expect(merge_request).to be_open
expect(merge_request.merge_commit_sha).to be_nil expect(merge_request.merge_commit_sha).to be_nil
expect(merge_request.merge_error).to include(error_message) expect(merge_request.merge_error).to include(described_class::GENERIC_ERROR_MESSAGE)
expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message)) expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(described_class::GENERIC_ERROR_MESSAGE))
end end
context 'when squashing is required' do context 'when squashing is required' 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