Store only generic message if rebase fails

Instead of storing detailed rebase error, only a generic message is
stored with MR. The reason is that this message is exposed and displayed
to end user and there is no reason to expose detailed backend
information. Error message is still logged so detailed information
can be found in logfile by admin if needed.

Related #41820
This commit is contained in:
Jan Provaznik 2018-01-09 16:49:39 +01:00
parent 678a00d60a
commit a560f785f7
3 changed files with 17 additions and 10 deletions

View File

@ -1,12 +1,14 @@
module MergeRequests
class RebaseService < MergeRequests::WorkingCopyBaseService
REBASE_ERROR = 'Rebase failed. Please rebase locally'.freeze
def execute(merge_request)
@merge_request = merge_request
if rebase
success
else
error('Failed to rebase. Should be done manually')
error(REBASE_ERROR)
end
end
@ -22,8 +24,8 @@ module MergeRequests
true
rescue => e
log_error('Failed to rebase branch:')
log_error(e.message, save_message_on_model: true)
log_error(REBASE_ERROR, save_message_on_model: true)
log_error(e.message)
false
end
end

View File

@ -0,0 +1,5 @@
---
title: Display user friendly error message if rebase fails.
merge_request:
author:
type: fixed

View File

@ -32,7 +32,7 @@ describe MergeRequests::RebaseService do
it 'returns an error' do
expect(service.execute(merge_request)).to match(status: :error,
message: 'Failed to rebase. Should be done manually')
message: described_class::REBASE_ERROR)
end
end
@ -41,15 +41,15 @@ describe MergeRequests::RebaseService do
allow(repository).to receive(:run_git!).and_raise('Something went wrong')
end
it 'saves the error message' do
it 'saves a generic error message' do
subject.execute(merge_request)
expect(merge_request.reload.merge_error).to eq 'Something went wrong'
expect(merge_request.reload.merge_error).to eq described_class::REBASE_ERROR
end
it 'returns an error' do
expect(service.execute(merge_request)).to match(status: :error,
message: 'Failed to rebase. Should be done manually')
message: described_class::REBASE_ERROR)
end
end
@ -58,15 +58,15 @@ describe MergeRequests::RebaseService do
allow(repository).to receive(:run_git!).and_raise(Gitlab::Git::Repository::GitError, 'Something went wrong')
end
it 'saves the error message' do
it 'saves a generic error message' do
subject.execute(merge_request)
expect(merge_request.reload.merge_error).to eq 'Something went wrong'
expect(merge_request.reload.merge_error).to eq described_class::REBASE_ERROR
end
it 'returns an error' do
expect(service.execute(merge_request)).to match(status: :error,
message: 'Failed to rebase. Should be done manually')
message: described_class::REBASE_ERROR)
end
end