diff --git a/Gemfile b/Gemfile
index d472cc83ec1fb2a338b466709f8d5613e8bac921..5d2e5697e3855de9f0ca2ec9bba2558ee762c557 100644
--- a/Gemfile
+++ b/Gemfile
@@ -485,7 +485,7 @@ gem 'ssh_data', '~> 1.2'
gem 'spamcheck', '~> 0.1.0'
# Gitaly GRPC protocol definitions
-gem 'gitaly', '~> 15.0.0-rc3'
+gem 'gitaly', '~> 15.1.0-rc1'
# KAS GRPC protocol definitions
gem 'kas-grpc', '~> 0.0.2'
diff --git a/Gemfile.lock b/Gemfile.lock
index 53e8c321d22a415a037cf130a15e00fe8f9b9104..fccc223c99c25a26cea5aee1dcfd83a6d696387d 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -466,7 +466,7 @@ GEM
rails (>= 3.2.0)
git (1.7.0)
rchardet (~> 1.8)
- gitaly (15.0.0.pre.rc3)
+ gitaly (15.1.0.pre.rc1)
grpc (~> 1.0)
github-markup (1.7.0)
gitlab (4.16.1)
@@ -1529,7 +1529,7 @@ DEPENDENCIES
gettext (~> 3.3)
gettext_i18n_rails (~> 1.8.0)
gettext_i18n_rails_js (~> 1.3)
- gitaly (~> 15.0.0.pre.rc3)
+ gitaly (~> 15.1.0.pre.rc1)
github-markup (~> 1.7.0)
gitlab-chronic (~> 0.10.5)
gitlab-dangerfiles (~> 3.0)
diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb
index 5a1699541d02ef62dcd4a3a93171d52d55b87e16..46a9329d1485b7e106dfa44196af1ad874c3f1ae 100644
--- a/lib/gitlab/gitaly_client/operation_service.rb
+++ b/lib/gitlab/gitaly_client/operation_service.rb
@@ -101,6 +101,16 @@ def user_delete_branch(branch_name, user)
if pre_receive_error = response.pre_receive_error.presence
raise Gitlab::Git::PreReceiveError, pre_receive_error
end
+ rescue GRPC::BadStatus => e
+ detailed_error = decode_detailed_error(e)
+
+ case detailed_error&.error
+ when :custom_hook
+ raise Gitlab::Git::PreReceiveError.new(custom_hook_error_message(detailed_error.custom_hook),
+ fallback_message: e.details)
+ else
+ raise
+ end
end
def user_merge_to_ref(user, source_sha:, branch:, target_ref:, message:, first_parent_ref:, allow_conflicts: false)
@@ -164,14 +174,8 @@ def user_merge_branch(user, source_sha, target_branch, message)
# These messages were returned from internal/allowed API calls
raise Gitlab::Git::PreReceiveError.new(fallback_message: access_check_error.error_message)
when :custom_hook
- # 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
- # Gitaly error as a fallback.
- custom_hook_error = detailed_error.custom_hook
- custom_hook_output = custom_hook_error.stderr.presence || custom_hook_error.stdout
- error_message = EncodingHelper.encode!(custom_hook_output)
-
- raise Gitlab::Git::PreReceiveError.new(error_message, fallback_message: e.details)
+ raise Gitlab::Git::PreReceiveError.new(custom_hook_error_message(detailed_error.custom_hook),
+ fallback_message: e.details)
when :reference_update
# We simply ignore any reference update errors which are typically an
# indicator of multiple RPC calls trying to update the same reference
@@ -550,6 +554,14 @@ def decode_detailed_error(err)
# 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
+ # Gitaly error as a fallback.
+ custom_hook_output = custom_hook_error.stderr.presence || custom_hook_error.stdout
+ EncodingHelper.encode!(custom_hook_output)
+ end
end
end
end
diff --git a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb
index 9b6efa30ff7aa80ec0cd08ae53be64c26c0adb2f..4b88472db60e755d201049ee6c0da8a9ec8e8b5b 100644
--- a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb
+++ b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb
@@ -170,6 +170,65 @@
Gitlab::Git::PreReceiveError, "something failed")
end
end
+
+ context 'with a custom hook error' do
+ let(:stdout) { nil }
+ let(:stderr) { nil }
+ let(:error_message) { "error_message" }
+ let(:custom_hook_error) do
+ new_detailed_error(
+ GRPC::Core::StatusCodes::PERMISSION_DENIED,
+ error_message,
+ Gitaly::UserDeleteBranchError.new(
+ custom_hook: Gitaly::CustomHookError.new(
+ stdout: stdout,
+ stderr: stderr,
+ hook_type: Gitaly::CustomHookError::HookType::HOOK_TYPE_PRERECEIVE
+ )))
+ end
+
+ shared_examples 'a failed branch deletion' do
+ it 'raises a PreRecieveError' do
+ expect_any_instance_of(Gitaly::OperationService::Stub)
+ .to receive(:user_delete_branch).with(request, kind_of(Hash))
+ .and_raise(custom_hook_error)
+
+ expect { subject }.to raise_error do |error|
+ expect(error).to be_a(Gitlab::Git::PreReceiveError)
+ expect(error.message).to eq(expected_message)
+ expect(error.raw_message).to eq(expected_raw_message)
+ end
+ end
+ end
+
+ context 'when details contain stderr' do
+ let(:stderr) { "something" }
+ let(:stdout) { "GL-HOOK-ERR: stdout is overridden by stderr" }
+ let(:expected_message) { error_message }
+ let(:expected_raw_message) { stderr }
+
+ it_behaves_like 'a failed branch deletion'
+ end
+
+ context 'when details contain stdout' do
+ let(:stderr) { " \n" }
+ let(:stdout) { "something" }
+ let(:expected_message) { error_message }
+ let(:expected_raw_message) { stdout }
+
+ it_behaves_like 'a failed branch deletion'
+ end
+ end
+
+ context 'with a non-detailed error' do
+ it 'raises a GRPC error' do
+ expect_any_instance_of(Gitaly::OperationService::Stub)
+ .to receive(:user_delete_branch).with(request, kind_of(Hash))
+ .and_raise(GRPC::Internal.new('non-detailed error'))
+
+ expect { subject }.to raise_error(GRPC::Internal)
+ end
+ end
end
describe '#user_merge_branch' do