Commit 727a0716 authored by Will Chandler's avatar Will Chandler

Expand branch creation error message

Currently when `repository#add_branch` returns `false` in
`Branches::CreateService` we return error message that the source
reference was invalid. This is the most likely cause of this error, but
not the only one.

Another cause is creating a branch with a subcomponent that matches the
full name of an existing branch, e.g. `feature/widget` and `feature`. In
this scenario it is the new branch name that is the problem, not the
source ref, but our error message gives no clues to this.

This is a particular problem for pull mirroring, where the SHA1 of the
source branch is used rather than a human-readable name. This requires
the user to copy down the SHA and go to the source repo to attempt to
understand which branch is causing the failure.

Ideally we would have separate errors for invalid source refs and
invalid branch names, but `repository#add_branch` hides the information
we need to distinguish between them.

This commit expands the error message to mention which branch we failed
to create, making it easier for users to find out they were creating
invalid branch names.
parent f0e2eef5
...@@ -18,7 +18,7 @@ module Branches ...@@ -18,7 +18,7 @@ module Branches
if new_branch if new_branch
success(new_branch) success(new_branch)
else else
error("Invalid reference name: #{ref}") error("Failed to create branch '#{branch_name}': invalid reference name '#{ref}'")
end end
rescue Gitlab::Git::PreReceiveError => e rescue Gitlab::Git::PreReceiveError => e
Gitlab::ErrorTracking.track_exception(e, pre_receive_message: e.raw_message, branch_name: branch_name, ref: ref) Gitlab::ErrorTracking.track_exception(e, pre_receive_message: e.raw_message, branch_name: branch_name, ref: ref)
......
...@@ -719,10 +719,11 @@ RSpec.describe API::Branches do ...@@ -719,10 +719,11 @@ RSpec.describe API::Branches do
end end
it 'returns 400 if ref name is invalid' do it 'returns 400 if ref name is invalid' do
error_message = 'Failed to create branch \'new_design3\': invalid reference name \'foo\''
post api(route, user), params: { branch: 'new_design3', ref: 'foo' } post api(route, user), params: { branch: 'new_design3', ref: 'foo' }
expect(response).to have_gitlab_http_status(:bad_request) expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response['message']).to eq('Invalid reference name: foo') expect(json_response['message']).to eq(error_message)
end end
end end
......
...@@ -35,11 +35,12 @@ RSpec.describe 'Creation of a new branch' do ...@@ -35,11 +35,12 @@ RSpec.describe 'Creation of a new branch' do
end end
context 'when ref is not correct' do context 'when ref is not correct' do
err_msg = 'Failed to create branch \'another_branch\': invalid reference name \'unknown\''
let(:new_branch) { 'another_branch' } let(:new_branch) { 'another_branch' }
let(:ref) { 'unknown' } let(:ref) { 'unknown' }
it_behaves_like 'a mutation that returns errors in the response', it_behaves_like 'a mutation that returns errors in the response',
errors: ['Invalid reference name: unknown'] errors: [err_msg]
end end
end end
end end
...@@ -38,11 +38,11 @@ RSpec.describe Branches::CreateService do ...@@ -38,11 +38,11 @@ RSpec.describe Branches::CreateService do
end end
it 'returns an error with a reference name' do it 'returns an error with a reference name' do
error_message = 'Failed to create branch \'new-feature\': invalid reference name unknown' err_msg = 'Failed to create branch \'new-feature\': invalid reference name \'unknown\''
result = service.execute('new-feature', 'unknown') result = service.execute('new-feature', 'unknown')
expect(result[:status]).to eq(:error) expect(result[:status]).to eq(:error)
expect(result[:message]).to eq('Invalid reference name: unknown') expect(result[:message]).to eq(err_msg)
end end
end end
......
...@@ -87,7 +87,7 @@ RSpec.describe Commits::CommitPatchService do ...@@ -87,7 +87,7 @@ RSpec.describe Commits::CommitPatchService do
context 'when specifying a non existent start branch' do context 'when specifying a non existent start branch' do
let(:start_branch) { 'does-not-exist' } let(:start_branch) { 'does-not-exist' }
it_behaves_like 'an error response', 'Invalid reference name' it_behaves_like 'an error response', 'Failed to create branch'
end end
end end
end end
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