diff --git a/Gemfile b/Gemfile index 2fbe351ed23414da9091b1a75771148e09f2cf57..6017dcd909302f859bd291f6908f722c0d1e5be2 100644 --- a/Gemfile +++ b/Gemfile @@ -483,7 +483,7 @@ gem 'ssh_data', '~> 1.3' gem 'spamcheck', '~> 1.0.0' # Gitaly GRPC protocol definitions -gem 'gitaly', '~> 15.3.0-rc4' +gem 'gitaly', '~> 15.4.0-rc2' # KAS GRPC protocol definitions gem 'kas-grpc', '~> 0.0.2' diff --git a/Gemfile.lock b/Gemfile.lock index f8e5f86ea0cd23fe234c6a0ad0d0c20b1627b809..7b78495dea28f7c4e8ed41f2b506df48e464953c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -531,7 +531,7 @@ GEM rails (>= 3.2.0) git (1.11.0) rchardet (~> 1.8) - gitaly (15.3.0.pre.rc4) + gitaly (15.4.0.pre.rc2) grpc (~> 1.0) github-markup (1.7.0) gitlab (4.16.1) @@ -1586,7 +1586,7 @@ DEPENDENCIES gettext (~> 3.3) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.3) - gitaly (~> 15.3.0.pre.rc4) + gitaly (~> 15.4.0.pre.rc2) github-markup (~> 1.7.0) gitlab-chronic (~> 0.10.5) gitlab-dangerfiles (~> 3.5.1) diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb index e9dbfe9ba1753ffd630f90df4b0d6b9ee003587a..7835fb32f59aac53353f873e6facd59e377b404a 100644 --- a/lib/gitlab/gitaly_client/operation_service.rb +++ b/lib/gitlab/gitaly_client/operation_service.rb @@ -85,8 +85,20 @@ def user_create_branch(branch_name, user, start_point) target_commit = Gitlab::Git::Commit.decorate(@repository, branch.target_commit) Gitlab::Git::Branch.new(@repository, branch.name, target_commit.id, target_commit) - rescue GRPC::FailedPrecondition => ex - raise Gitlab::Git::Repository::InvalidRef, ex + rescue GRPC::BadStatus => e + detailed_error = GitalyClient.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 + if e.code == GRPC::Core::StatusCodes::FAILED_PRECONDITION + raise Gitlab::Git::Repository::InvalidRef, e + end + + raise + end end def user_update_branch(branch_name, user, newrev, oldrev) diff --git a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb index 5d854f0c9d1cf254315dff0c7d226c7a491f376c..7e8aaa3cdf4426a7804eede8d17c7d954bc6a2de 100644 --- a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb @@ -56,6 +56,85 @@ Gitlab::Git::PreReceiveError, "something failed") end end + + context 'with structured errors' do + context 'with CustomHookError' 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::UserCreateBranchError.new( + custom_hook: Gitaly::CustomHookError.new( + stdout: stdout, + stderr: stderr, + hook_type: Gitaly::CustomHookError::HookType::HOOK_TYPE_PRERECEIVE + ))) + end + + shared_examples 'failed branch creation' do + it 'raised a PreRecieveError' do + expect_any_instance_of(Gitaly::OperationService::Stub) + .to receive(:user_create_branch) + .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 without prefix' 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 'failed branch creation' + end + + context 'when details contain stderr with prefix' do + let(:stderr) { "GL-HOOK-ERR: something" } + let(:stdout) { "GL-HOOK-ERR: stdout is overridden by stderr" } + let(:expected_message) { "something" } + let(:expected_raw_message) { stderr } + + it_behaves_like 'failed branch creation' + end + + context 'when details contain stdout without prefix' do + let(:stderr) { " \n" } + let(:stdout) { "something" } + let(:expected_message) { error_message } + let(:expected_raw_message) { stdout } + + it_behaves_like 'failed branch creation' + end + + context 'when details contain stdout with prefix' do + let(:stderr) { " \n" } + let(:stdout) { "GL-HOOK-ERR: something" } + let(:expected_message) { "something" } + let(:expected_raw_message) { stdout } + + it_behaves_like 'failed branch creation' + end + + context 'when details contain no stderr or stdout' do + let(:stderr) { " \n" } + let(:stdout) { "\n \n" } + let(:expected_message) { error_message } + let(:expected_raw_message) { "\n \n" } + + it_behaves_like 'failed branch creation' + end + end + end end describe '#user_update_branch' do