Commit 7083ab22 authored by Stan Hu's avatar Stan Hu Committed by Igor Drozdov

Improve merge error when pre-receive hooks fail in fast-forward merge

When fast-forward merge is enabled, merges might fail due to a failed
pre-receive check (e.g. due to file locks, commit message push rules,
etc.) but the UI would only display:

```
Merge failed: pre-receive hook failed. Please try again.
```

While the raw message was logged in Sentry and in `exceptions_json.log`,
this kept users in the dark.

Gitaly prefaces the error response status from the `/internal/allowed`
API call with `GitLab:`, so we can use this to display the error to the
user. We now only fallback to a default if we do not have anything to
show. Now, users see a more informative message, such as:

```
Merge failed: Commit message does not follow the pattern
'MERGEME'. Please try again.
```

Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/246816
parent 30053faa
---
title: Improve merge error when pre-receive hooks fail in fast-forward merge
merge_request: 44843
author:
type: fixed
...@@ -18,13 +18,15 @@ module Gitlab ...@@ -18,13 +18,15 @@ module Gitlab
attr_reader :raw_message attr_reader :raw_message
def initialize(message = '', user_message = '') def initialize(message = '', fallback_message: '')
@raw_message = message @raw_message = message
if user_message.present? sanitized_msg = sanitize(message)
super(sanitize(user_message))
if sanitized_msg.present?
super(sanitized_msg)
else else
super(sanitize(message)) super(fallback_message)
end end
end end
......
...@@ -179,7 +179,7 @@ module Gitlab ...@@ -179,7 +179,7 @@ module Gitlab
) )
if response.pre_receive_error.present? if response.pre_receive_error.present?
raise Gitlab::Git::PreReceiveError.new(response.pre_receive_error, "GL-HOOK-ERR: pre-receive hook failed.") raise Gitlab::Git::PreReceiveError.new(response.pre_receive_error, fallback_message: "pre-receive hook failed.")
end end
Gitlab::Git::OperationService::BranchUpdate.from_gitaly(response.branch_update) Gitlab::Git::OperationService::BranchUpdate.from_gitaly(response.branch_update)
......
...@@ -21,13 +21,21 @@ RSpec.describe Gitlab::Git::PreReceiveError do ...@@ -21,13 +21,21 @@ RSpec.describe Gitlab::Git::PreReceiveError do
expect(ex.raw_message).to eq(raw_message) expect(ex.raw_message).to eq(raw_message)
end end
it 'sanitizes the user message' do it 'prefers the original message over the fallback' do
raw_message = 'Raw message' raw_message = "#{prefix} Hello,\nworld!"
ex = described_class.new(raw_message, "#{prefix} User message") ex = described_class.new(raw_message, fallback_message: "User message")
expect(ex.message).to eq('Hello,')
expect(ex.raw_message).to eq(raw_message) expect(ex.raw_message).to eq(raw_message)
expect(ex.message).to eq('User message')
end end
end end
it 'uses the fallback message' do
raw_message = 'Hello\n'
ex = described_class.new(raw_message, fallback_message: "User message")
expect(ex.raw_message).to eq(raw_message)
expect(ex.message).to eq('User message')
end
end end
end end
...@@ -114,7 +114,7 @@ RSpec.describe MergeRequests::FfMergeService do ...@@ -114,7 +114,7 @@ RSpec.describe MergeRequests::FfMergeService do
error_message = 'error message' error_message = 'error message'
raw_message = 'The truth is out there' raw_message = 'The truth is out there'
pre_receive_error = Gitlab::Git::PreReceiveError.new(raw_message, "GitLab: #{error_message}") pre_receive_error = Gitlab::Git::PreReceiveError.new(raw_message, fallback_message: error_message)
allow(service).to receive(:repository).and_raise(pre_receive_error) allow(service).to receive(:repository).and_raise(pre_receive_error)
allow(service).to receive(:execute_hooks) allow(service).to receive(:execute_hooks)
expect(Gitlab::ErrorTracking).to receive(:track_exception).with( expect(Gitlab::ErrorTracking).to receive(:track_exception).with(
......
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