Commit c7bcaadf authored by Patrick Steinhardt's avatar Patrick Steinhardt

gitaly_client: Handle detailed errors for UserSquash

Gitaly is in the process of migrating UserSquash to use detailed errors
like we already do for UserMergeBranch. 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 the OperationService for this change by handling those new error
details. Theoretically, this allows us to implement more fine-grained
handling of these errors now: `ResolveRevisionErrors` contain the
revision that Gitaly tried to resolve but couldn't, 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 `GitError`. 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 UserSquash.
parent d85e262e
......@@ -299,6 +299,26 @@ module Gitlab
end
response.squash_sha
rescue GRPC::BadStatus => e
detailed_error = decode_detailed_error(e)
case detailed_error&.error
when :resolve_revision, :rebase_conflict
# Theoretically, we could now raise specific errors based on the type
# of the detailed error. Most importantly, we get error details when
# Gitaly was not able to resolve the `start_sha` or `end_sha` via a
# ResolveRevisionError, and we get information about which files are
# conflicting via a MergeConflictError.
#
# We don't do this now though such that we can maintain backwards
# compatibility with the minimum required set of changes during the
# transitory period where we're migrating UserSquash to use
# structured errors. We thus continue to just return a GitError, like
# we previously did.
raise Gitlab::Git::Repository::GitError, e.details
else
raise
end
end
def user_update_submodule(user:, submodule:, commit_sha:, branch:, message:)
......
......@@ -436,6 +436,58 @@ RSpec.describe Gitlab::GitalyClient::OperationService do
Gitlab::Git::Repository::GitError, "something failed")
end
end
shared_examples '#user_squash with an error' do
it 'raises a GitError exception' do
expect_any_instance_of(Gitaly::OperationService::Stub)
.to receive(:user_squash).with(request, kind_of(Hash))
.and_raise(raised_error)
expect { subject }.to raise_error(expected_error)
end
end
context 'when ResolveRevisionError is raised' do
let(:raised_error) do
new_detailed_error(
GRPC::Core::StatusCodes::INVALID_ARGUMENT,
'something failed',
Gitaly::UserSquashError.new(
resolve_revision: Gitaly::ResolveRevisionError.new(
revision: start_sha
)))
end
let(:expected_error) { Gitlab::Git::Repository::GitError }
it_behaves_like '#user_squash 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 '#user_squash 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 '#user_squash with an error'
end
end
describe '#user_commit_files' do
......
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