Commit dad06025 authored by Stan Hu's avatar Stan Hu

Log raw pre-receive message in fast-forward merge

When a pre-receive error happens in the fast-forward merge, the original
error is discarded, and only text prefaced with `GL-HOOK-ERR` is passed
through. However, this makes it difficult to know what went wrong.

To fix this, we:

1. Store the raw message to `Gitlab::Git::PreReceiveError`
2. Log an exception with the raw message

Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/227876
parent 0d013cc9
...@@ -22,6 +22,7 @@ module MergeRequests ...@@ -22,6 +22,7 @@ module MergeRequests
ff_merge ff_merge
rescue Gitlab::Git::PreReceiveError => e rescue Gitlab::Git::PreReceiveError => e
Gitlab::ErrorTracking.track_exception(e, pre_receive_message: e.raw_message, merge_request_id: merge_request&.id)
raise MergeError, e.message raise MergeError, e.message
rescue StandardError => e rescue StandardError => e
raise MergeError, "Something went wrong during merge: #{e.message}" raise MergeError, "Something went wrong during merge: #{e.message}"
......
---
title: Log raw pre-receive message in fast-forward merge
merge_request: 38354
author:
type: other
...@@ -16,8 +16,16 @@ module Gitlab ...@@ -16,8 +16,16 @@ module Gitlab
SAFE_MESSAGE_REGEX = /^(#{SAFE_MESSAGE_PREFIXES.join('|')})\s*(?<safe_message>.+)/.freeze SAFE_MESSAGE_REGEX = /^(#{SAFE_MESSAGE_PREFIXES.join('|')})\s*(?<safe_message>.+)/.freeze
def initialize(message = '') attr_reader :raw_message
super(sanitize(message))
def initialize(message = '', user_message = '')
@raw_message = message
if user_message.present?
super(sanitize(user_message))
else
super(sanitize(message))
end
end end
private private
......
...@@ -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("GL-HOOK-ERR: pre-receive hook failed.") raise Gitlab::Git::PreReceiveError.new(response.pre_receive_error, "GL-HOOK-ERR: pre-receive hook failed.")
end end
Gitlab::Git::OperationService::BranchUpdate.from_gitaly(response.branch_update) Gitlab::Git::OperationService::BranchUpdate.from_gitaly(response.branch_update)
......
...@@ -6,15 +6,27 @@ RSpec.describe Gitlab::Git::PreReceiveError do ...@@ -6,15 +6,27 @@ RSpec.describe Gitlab::Git::PreReceiveError do
Gitlab::Git::PreReceiveError::SAFE_MESSAGE_PREFIXES.each do |prefix| Gitlab::Git::PreReceiveError::SAFE_MESSAGE_PREFIXES.each do |prefix|
context "error messages prefixed with #{prefix}" do context "error messages prefixed with #{prefix}" do
it 'accepts only errors lines with the prefix' do it 'accepts only errors lines with the prefix' do
ex = described_class.new("#{prefix} Hello,\nworld!") raw_message = "#{prefix} Hello,\nworld!"
ex = described_class.new(raw_message)
expect(ex.message).to eq('Hello,') expect(ex.message).to eq('Hello,')
expect(ex.raw_message).to eq(raw_message)
end end
it 'makes its message HTML-friendly' do it 'makes its message HTML-friendly' do
ex = described_class.new("#{prefix} Hello,\n#{prefix} world!\n") raw_message = "#{prefix} Hello,\n#{prefix} world!\n"
ex = described_class.new(raw_message)
expect(ex.message).to eq('Hello,<br>world!') expect(ex.message).to eq('Hello,<br>world!')
expect(ex.raw_message).to eq(raw_message)
end
it 'sanitizes the user message' do
raw_message = 'Raw message'
ex = described_class.new(raw_message, "#{prefix} User message")
expect(ex.raw_message).to eq(raw_message)
expect(ex.message).to eq('User message')
end end
end end
end end
......
...@@ -113,9 +113,16 @@ RSpec.describe MergeRequests::FfMergeService do ...@@ -113,9 +113,16 @@ RSpec.describe MergeRequests::FfMergeService do
it 'logs and saves error if there is an PreReceiveError exception' do it 'logs and saves error if there is an PreReceiveError exception' do
error_message = 'error message' error_message = 'error message'
raw_message = 'The truth is out there'
allow(service).to receive(:repository).and_raise(Gitlab::Git::PreReceiveError, "GitLab: #{error_message}") pre_receive_error = Gitlab::Git::PreReceiveError.new(raw_message, "GitLab: #{error_message}")
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(
pre_receive_error,
pre_receive_message: raw_message,
merge_request_id: merge_request.id
)
service.execute(merge_request) service.execute(merge_request)
......
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