diff options
author | Jan Provaznik <jprovaznik@gitlab.com> | 2018-04-29 17:30:49 +0200 |
---|---|---|
committer | Jan Provaznik <jprovaznik@gitlab.com> | 2018-05-02 09:22:20 +0200 |
commit | d973872072097152bf16ebb808a2c65912b9f3d0 (patch) | |
tree | e736f65fe88a11c7f77138ba78cb4f841e51b77e /app/services | |
parent | ee189fd511e1a2c06f05e0d40e1d0b8875151391 (diff) | |
download | gitlab-ce-d973872072097152bf16ebb808a2c65912b9f3d0.tar.gz |
Save and expose only generic merge error
When an error occurs during merge, the error message is exposed to user
and it is also saved in DB. This error message may be user unfriendly
(as in !41820) and it could also expose a detailed backend information.
Instead of displaying the specific error message, only sanitized generic
message is displayed. This is potentially controversial change because
disadvantage is that user doesn't get specific reason of failure.
Additional changes:
* repository.merge including exceptions is is extracted into a
separate method to make things clearer
* update! is used instead of update so we don't silently ignore
an error
Related to !41857
Diffstat (limited to 'app/services')
-rw-r--r-- | app/services/merge_requests/merge_service.rb | 29 |
1 files changed, 19 insertions, 10 deletions
diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index cedfcb50e09..2209a60a840 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -50,21 +50,30 @@ module MergeRequests end def commit - message = params[:commit_message] || merge_request.merge_commit_message - log_info("Git merge started on JID #{merge_jid}") - commit_id = repository.merge(current_user, source, merge_request, message) - log_info("Git merge finished on JID #{merge_jid} commit #{commit_id}") + commit_id = try_merge + + if commit_id + log_info("Git merge finished on JID #{merge_jid} commit #{commit_id}") + else + raise MergeError, 'Conflicts detected during merge' + end - raise MergeError, 'Conflicts detected during merge' unless commit_id + merge_request.update!(merge_commit_sha: commit_id) + end + + def try_merge + message = params[:commit_message] || merge_request.merge_commit_message - merge_request.update(merge_commit_sha: commit_id) + repository.merge(current_user, source, merge_request, message) rescue Gitlab::Git::HooksService::PreReceiveError => e - raise MergeError, e.message - rescue StandardError => e - raise MergeError, "Something went wrong during merge: #{e.message}" + handle_merge_error(log_message: e.message) + raise MergeError, 'Something went wrong during merge pre-receive hook' + rescue => e + handle_merge_error(log_message: e.message) + raise MergeError, 'Something went wrong during merge' ensure - merge_request.update(in_progress_merge_commit_sha: nil) + merge_request.update!(in_progress_merge_commit_sha: nil) end def after_merge |