Commit efb13037 authored by John Cai's avatar John Cai

gitaly: Adapt to structured errors returned by UserRebaseConfirmable

Gitaly is in the process of migrating UserSquash to use detailed errors
like we already do for UserMergeBranch and UserSquash. This change will
break the current calling convention, where Gitaly will return some
errors by returning successfully, but with an `git_error` field set in
the response. Instead, Gitaly will return a proper error for all
conditions but give clients the ability to distinguish error cases via
error details.

Prepare UserRebaseConfirmable for this change by handling those new error
details. Theoretically, this allows us to implement more fine-grained
handling of these errors now: `AccessCheck` contain an error from
Gitaly calling the Gitlab API, and `MergeConflictErrors` contain
information about the files that have been conflicting. To ease the
transition though we're treating those errors exactly the same as we did
previously, where we raised a `PreReceiveError` when there was a error
during pre_receive and `GitError` otherwise. We can still iterate on this
at a later point.

The end result should be that there is no real user-visible change in
how we handle UserRebaseConfirmable.
parent 5aebfbe1
......@@ -479,7 +479,7 @@ gem 'ssh_data', '~> 1.2'
gem 'spamcheck', '~> 0.1.0'
# Gitaly GRPC protocol definitions
gem 'gitaly', '~> 14.9.0.pre.rc3'
gem 'gitaly', '~> 14.9.0.pre.rc4'
# KAS GRPC protocol definitions
gem 'kas-grpc', '~> 0.0.2'
......
......@@ -455,7 +455,7 @@ GEM
rails (>= 3.2.0)
git (1.7.0)
rchardet (~> 1.8)
gitaly (14.9.0.pre.rc3)
gitaly (14.9.0.pre.rc4)
grpc (~> 1.0)
github-markup (1.7.0)
gitlab (4.16.1)
......@@ -1486,7 +1486,7 @@ DEPENDENCIES
gettext (~> 3.3)
gettext_i18n_rails (~> 1.8.0)
gettext_i18n_rails_js (~> 1.3)
gitaly (~> 14.9.0.pre.rc3)
gitaly (~> 14.9.0.pre.rc4)
github-markup (~> 1.7.0)
gitlab-chronic (~> 0.10.5)
gitlab-dangerfiles (~> 2.10.2)
......
......@@ -263,6 +263,19 @@ module Gitlab
perform_next_gitaly_rebase_request(response_enum)
rebase_sha
rescue GRPC::BadStatus => e
detailed_error = decode_detailed_error(e)
case detailed_error&.error
when :access_check
access_check_error = detailed_error.access_check
# These messages were returned from internal/allowed API calls
raise Gitlab::Git::PreReceiveError.new(fallback_message: access_check_error.error_message)
when :rebase_conflict
raise Gitlab::Git::Repository::GitError, e.details
else
raise e
end
ensure
request_enum.close
end
......
......@@ -386,6 +386,73 @@ RSpec.describe Gitlab::GitalyClient::OperationService do
it_behaves_like 'cherry pick and revert errors'
end
describe '#rebase' do
let(:response) { Gitaly::UserRebaseConfirmableResponse.new }
subject do
client.rebase(
user,
'',
branch: 'master',
branch_sha: 'b83d6e391c22777fca1ed3012fce84f633d7fed0',
remote_repository: repository,
remote_branch: 'master'
)
end
shared_examples '#rebase with an error' do
it 'raises a GitError exception' do
expect_any_instance_of(Gitaly::OperationService::Stub)
.to receive(:user_rebase_confirmable)
.and_raise(raised_error)
expect { subject }.to raise_error(expected_error)
end
end
context 'when AccessError is raised' do
let(:raised_error) do
new_detailed_error(
GRPC::Core::StatusCodes::INTERNAL,
'something failed',
Gitaly::UserRebaseConfirmableError.new(
access_check: Gitaly::AccessCheckError.new(
error_message: 'something went wrong'
)))
end
let(:expected_error) { Gitlab::Git::PreReceiveError }
it_behaves_like '#rebase with an error'
end
context 'when RebaseConflictError is raised' do
let(:raised_error) do
new_detailed_error(
GRPC::Core::StatusCodes::INTERNAL,
'something failed',
Gitaly::UserSquashError.new(
rebase_conflict: Gitaly::MergeConflictError.new(
conflicting_files: ['conflicting-file']
)))
end
let(:expected_error) { Gitlab::Git::Repository::GitError }
it_behaves_like '#rebase with an error'
end
context 'when non-detailed gRPC error is raised' do
let(:raised_error) do
GRPC::Internal.new('non-detailed error')
end
let(:expected_error) { GRPC::Internal }
it_behaves_like '#rebase with an error'
end
end
describe '#user_squash' do
let(:start_sha) { 'b83d6e391c22777fca1ed3012fce84f633d7fed0' }
let(:end_sha) { '54cec5282aa9f21856362fe321c800c236a61615' }
......
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