diff --git a/CHANGELOG b/CHANGELOG index 3b89fa05801..c05d81f56ba 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -33,6 +33,7 @@ v 8.10.0 (unreleased) - Add basic system information like memory and disk usage to the admin panel - Don't garbage collect commits that have related DB records like comments - More descriptive message for git hooks and file locks + - Handle custom Git hook result in GitLab UI v 8.9.5 (unreleased) - Improve the request / withdraw access button. !4860 diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index 9aaf5a5e561..3bec66cea88 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -36,10 +36,13 @@ module MergeRequests commit_id = repository.merge(current_user, merge_request.source_sha, merge_request.target_branch, options) merge_request.update(merge_commit_sha: commit_id) + rescue GitHooksService::PreReceiveError => e + 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) - return false + false end def after_merge diff --git a/lib/gitlab/git/hook.rb b/lib/gitlab/git/hook.rb index 5415f4844d3..420c6883c45 100644 --- a/lib/gitlab/git/hook.rb +++ b/lib/gitlab/git/hook.rb @@ -41,7 +41,7 @@ module Gitlab chdir: repo_path } - Open3.popen3(vars, path, options) do |stdin, _, stderr, wait_thr| + Open3.popen3(vars, path, options) do |stdin, stdout, stderr, wait_thr| exit_status = true stdin.sync = true @@ -60,7 +60,7 @@ module Gitlab unless wait_thr.value == 0 exit_status = false - exit_message = stderr.gets + exit_message = retrieve_error_message(stderr, stdout) end end @@ -76,6 +76,11 @@ module Gitlab [status, nil] end + + def retrieve_error_message(stderr, stdout) + err_message = stderr.gets + err_message.blank? ? stdout.gets : err_message + end end end end diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index 1b0396eb686..2f72cd60071 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -65,6 +65,16 @@ describe MergeRequests::MergeService, services: true do expect(merge_request.merge_error).to eq("Something went wrong during merge") end + + it 'saves error if there is an PreReceiveError exception' do + allow(service).to receive(:repository).and_raise(GitHooksService::PreReceiveError, "error") + + allow(service).to receive(:execute_hooks) + + service.execute(merge_request) + + expect(merge_request.merge_error).to eq("error") + end end end end