Commit d85e262e authored by Patrick Steinhardt's avatar Patrick Steinhardt

gitaly_client: Move detailed error handling into UserMergeBranch

Gitaly has started to return detailed errors in the context of
UserMergeBranch, which was the first proof of concept to see whether
this new model makes sense for error handling. With the introduction of
structured error handling we also had to convert Rails to handle these
new types of errors, which led to the introduction of the new helper
function `#decode_detailed_error`. This function both is responsible for
decoding the error, and translating it into Rails-specific errors.

While it makes sense to have it decode errors, the concrete translation
into Rails-specific errors is something that is specific for every RPC:
UserMergeBranch can return one set of error details, while other RPCs
may return a different set of error details. Mixing this all into a
single function that is supposed to be shared between RPCs thus does not
make much sense.

Refactor the code to move the RPC-specific error translation into the
RPC's own error handling. This prepares for the introduction of error
detail handling in UserSquash.
parent 253e23d7
...@@ -164,16 +164,20 @@ module Gitlab ...@@ -164,16 +164,20 @@ module Gitlab
Gitlab::Git::OperationService::BranchUpdate.from_gitaly(branch_update) Gitlab::Git::OperationService::BranchUpdate.from_gitaly(branch_update)
rescue GRPC::BadStatus => e rescue GRPC::BadStatus => e
decoded_error = decode_detailed_error(e) detailed_error = decode_detailed_error(e)
raise unless decoded_error.present? case detailed_error&.error
when :access_check
# We simply ignore any reference update errors which are typically an access_check_error = detailed_error.access_check
# indicator of multiple RPC calls trying to update the same reference # These messages were returned from internal/allowed API calls
# at the same point in time. raise Gitlab::Git::PreReceiveError.new(fallback_message: access_check_error.error_message)
return if decoded_error.is_a?(Gitlab::Git::ReferenceUpdateError) when :reference_update
# We simply ignore any reference update errors which are typically an
raise decoded_error # indicator of multiple RPC calls trying to update the same reference
# at the same point in time.
else
raise
end
ensure ensure
request_enum.close request_enum.close
end end
...@@ -492,23 +496,7 @@ module Gitlab ...@@ -492,23 +496,7 @@ module Gitlab
prefix = %r{type\.googleapis\.com\/gitaly\.(?<error_type>.+)} prefix = %r{type\.googleapis\.com\/gitaly\.(?<error_type>.+)}
error_type = prefix.match(detailed_error.type_url)[:error_type] error_type = prefix.match(detailed_error.type_url)[:error_type]
detailed_error = Gitaly.const_get(error_type, false).decode(detailed_error.value) Gitaly.const_get(error_type, false).decode(detailed_error.value)
case detailed_error.error
when :access_check
access_check_error = detailed_error.access_check
# These messages were returned from internal/allowed API calls
Gitlab::Git::PreReceiveError.new(fallback_message: access_check_error.error_message)
when :reference_update
reference_update_error = detailed_error.reference_update
Gitlab::Git::ReferenceUpdateError.new(err.details,
reference_update_error.reference_name,
reference_update_error.old_oid,
reference_update_error.new_oid)
else
# We're handling access_check only for now, but we'll add more detailed error types
nil
end
rescue NameError, NoMethodError rescue NameError, NoMethodError
# Error Class might not be known to ruby yet # Error Class might not be known to ruby yet
nil nil
......
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