Skip to content
Snippets Groups Projects
Commit 84a1be28 authored by John Cai's avatar John Cai
Browse files

Gitaly ref service: teach delete_refs to parse structured errors

Moving decode_detailed_error

decode_detailed_error is useful for more than just operation_service.
Move this method one level up to GitalyClient so other services can take
advantage of it.

Move new_detailed_error to spec helper

new_detailed_error is a useful helper for specs other than just
operations_service_spec. Move this into its own spec helper.

Add new git errors

Add two new git error types. We will utilize these in a future commit
that parses a gRPC detailed error and throws an appropriate error on the
Rails side.

Teach delete_refs to parse gRPC detailed errors

Give delete_refs the ability to catch a gRPC error and parse the
detailed error if there is one.
parent c2aa008d
No related merge requests found
......@@ -18,6 +18,8 @@ module Git
UnknownRef = Class.new(BaseError)
CommandTimedOut = Class.new(CommandError)
InvalidPageToken = Class.new(BaseError)
InvalidRefFormatError = Class.new(BaseError)
ReferencesLockedError = Class.new(BaseError)
class << self
include Gitlab::EncodingHelper
......
......@@ -483,6 +483,22 @@ def self.max_stacks
stack_counter.select { |_, v| v == max }.keys
end
def self.decode_detailed_error(err)
# details could have more than one in theory, but we only have one to worry about for now.
detailed_error = err.to_rpc_status&.details&.first
return unless detailed_error.present?
prefix = %r{type\.googleapis\.com\/gitaly\.(?<error_type>.+)}
error_type = prefix.match(detailed_error.type_url)[:error_type]
Gitaly.const_get(error_type, false).decode(detailed_error.value)
rescue NameError, NoMethodError
# Error Class might not be known to ruby yet
nil
end
private_class_method :max_stacks
end
end
......@@ -102,7 +102,7 @@ def user_delete_branch(branch_name, user)
raise Gitlab::Git::PreReceiveError, pre_receive_error
end
rescue GRPC::BadStatus => e
detailed_error = decode_detailed_error(e)
detailed_error = GitalyClient.decode_detailed_error(e)
case detailed_error&.error
when :custom_hook
......@@ -166,7 +166,7 @@ def user_merge_branch(user, source_sha, target_branch, message)
Gitlab::Git::OperationService::BranchUpdate.from_gitaly(branch_update)
rescue GRPC::BadStatus => e
detailed_error = decode_detailed_error(e)
detailed_error = GitalyClient.decode_detailed_error(e)
case detailed_error&.error
when :access_check
......@@ -277,7 +277,7 @@ def rebase(user, rebase_id, branch:, branch_sha:, remote_repository:, remote_bra
rebase_sha
rescue GRPC::BadStatus => e
detailed_error = decode_detailed_error(e)
detailed_error = GitalyClient.decode_detailed_error(e)
case detailed_error&.error
when :access_check
......@@ -314,7 +314,7 @@ def user_squash(user, start_sha, end_sha, author, message, time = Time.now.utc)
response.squash_sha
rescue GRPC::BadStatus => e
detailed_error = decode_detailed_error(e)
detailed_error = GitalyClient.decode_detailed_error(e)
case detailed_error&.error
when :resolve_revision, :rebase_conflict
......@@ -474,7 +474,7 @@ def call_cherry_pick_or_revert(rpc, user:, commit:, branch_name:, message:, star
handle_cherry_pick_or_revert_response(response)
rescue GRPC::BadStatus => e
detailed_error = decode_detailed_error(e)
detailed_error = GitalyClient.decode_detailed_error(e)
case detailed_error&.error
when :access_check
......@@ -538,21 +538,6 @@ def user_commit_files_action_header(action)
raise ArgumentError, "Unknown action '#{action[:action]}'"
end
def decode_detailed_error(err)
# details could have more than one in theory, but we only have one to worry about for now.
detailed_error = err.to_rpc_status&.details&.first
return unless detailed_error.present?
prefix = %r{type\.googleapis\.com\/gitaly\.(?<error_type>.+)}
error_type = prefix.match(detailed_error.type_url)[:error_type]
Gitaly.const_get(error_type, false).decode(detailed_error.value)
rescue NameError, NoMethodError
# Error Class might not be known to ruby yet
nil
end
def custom_hook_error_message(custom_hook_error)
# Custom hooks may return messages via either stdout or stderr which have a specific prefix. If
# that prefix is present we'll want to print the hook's output, otherwise we'll want to print the
......
......@@ -132,6 +132,17 @@ def delete_refs(refs: [], except_with_prefixes: [])
response = GitalyClient.call(@repository.storage, :ref_service, :delete_refs, request, timeout: GitalyClient.medium_timeout)
raise Gitlab::Git::Repository::GitError, response.git_error if response.git_error.present?
rescue GRPC::BadStatus => e
detailed_error = GitalyClient.decode_detailed_error(e)
case detailed_error&.error
when :invalid_format
raise Gitlab::Git::InvalidRefFormatError, "references have an invalid format: #{detailed_error.invalid_format.refs.join(",")}"
when :references_locked
raise Gitlab::Git::ReferencesLockedError
else
raise e
end
end
# Limit: 0 implies no limit, thus all tag names will be returned
......
......@@ -2,9 +2,6 @@
require 'spec_helper'
require 'google/rpc/status_pb'
require 'google/protobuf/well_known_types'
RSpec.describe Gitlab::GitalyClient::OperationService do
let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project, :repository) }
......@@ -816,14 +813,4 @@
end
end
end
def new_detailed_error(error_code, error_message, details)
status_error = Google::Rpc::Status.new(
code: error_code,
message: error_message,
details: [Google::Protobuf::Any.pack(details)]
)
GRPC::BadStatus.new(error_code, error_message, { "grpc-status-details-bin" => Google::Rpc::Status.encode(status_error) })
end
end
......@@ -258,13 +258,54 @@
describe '#delete_refs' do
let(:prefixes) { %w(refs/heads refs/keep-around) }
subject(:delete_refs) { client.delete_refs(except_with_prefixes: prefixes) }
it 'sends a delete_refs message' do
expect_any_instance_of(Gitaly::RefService::Stub)
.to receive(:delete_refs)
.with(gitaly_request_with_params(except_with_prefix: prefixes), kind_of(Hash))
.and_return(double('delete_refs_response', git_error: ""))
client.delete_refs(except_with_prefixes: prefixes)
delete_refs
end
context 'with a references locked error' do
let(:references_locked_error) do
new_detailed_error(
GRPC::Core::StatusCodes::FAILED_PRECONDITION,
"error message",
Gitaly::DeleteRefsError.new(references_locked: Gitaly::ReferencesLockedError.new))
end
it 'raises ReferencesLockedError' do
expect_any_instance_of(Gitaly::RefService::Stub).to receive(:delete_refs)
.with(gitaly_request_with_params(except_with_prefix: prefixes), kind_of(Hash))
.and_raise(references_locked_error)
expect { delete_refs }.to raise_error(Gitlab::Git::ReferencesLockedError)
end
end
context 'with a invalid format error' do
let(:invalid_refs) {['\invali.\d/1', '\.invali/d/2']}
let(:invalid_reference_format_error) do
new_detailed_error(
GRPC::Core::StatusCodes::INVALID_ARGUMENT,
"error message",
Gitaly::DeleteRefsError.new(invalid_format: Gitaly::InvalidRefFormatError.new(refs: invalid_refs)))
end
it 'raises InvalidRefFormatError' do
expect_any_instance_of(Gitaly::RefService::Stub)
.to receive(:delete_refs)
.with(gitaly_request_with_params(except_with_prefix: prefixes), kind_of(Hash))
.and_raise(invalid_reference_format_error)
expect { delete_refs }.to raise_error do |error|
expect(error).to be_a(Gitlab::Git::InvalidRefFormatError)
expect(error.message).to eq("references have an invalid format: #{invalid_refs.join(",")}")
end
end
end
end
......
......@@ -537,4 +537,44 @@ def call_gitaly(count = 1)
end
end
end
describe '.decode_detailed_error' do
let(:detailed_error) do
new_detailed_error(GRPC::Core::StatusCodes::INVALID_ARGUMENT,
"error message",
Gitaly::InvalidRefFormatError.new)
end
let(:error_without_details) do
error_code = GRPC::Core::StatusCodes::INVALID_ARGUMENT
error_message = "error message"
status_error = Google::Rpc::Status.new(
code: error_code,
message: error_message,
details: nil
)
GRPC::BadStatus.new(
error_code,
error_message,
{ "grpc-status-details-bin" => Google::Rpc::Status.encode(status_error) })
end
context 'decodes a structured error' do
using RSpec::Parameterized::TableSyntax
where(:error, :result) do
detailed_error | Gitaly::InvalidRefFormatError.new
error_without_details | nil
StandardError.new | nil
end
with_them do
it 'returns correct detailed error' do
expect(described_class.decode_detailed_error(error)).to eq(result)
end
end
end
end
end
......@@ -204,6 +204,7 @@
config.include SnowplowHelpers
config.include RenderedHelpers
config.include RSpec::Benchmark::Matchers, type: :benchmark
config.include DetailedErrorHelpers
include StubFeatureFlags
include StubSnowplow
......
# frozen_string_literal: true
require 'google/rpc/status_pb'
require 'google/protobuf/well_known_types'
module DetailedErrorHelpers
def new_detailed_error(error_code, error_message, details)
status_error = Google::Rpc::Status.new(
code: error_code,
message: error_message,
details: [Google::Protobuf::Any.pack(details)]
)
GRPC::BadStatus.new(
error_code,
error_message,
{ "grpc-status-details-bin" => Google::Rpc::Status.encode(status_error) })
end
end
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