Add structured error parsing for user_cherry_pick

Gitaly's UserCherryPick RPC will begin to return structured errors. The
reason is that currently we return an error message embedded in the
response and hence it looks like the RPC succeeded. This not only hides
errors in the metrics and logs, Praefect will also expect transaction
voting to happen correctly for successful requests. However, an error
happened which sometimes causes voting to not happen at all. In this
situation, Praefect will schedule unneeded replication jobs.

This change prepares for the Gitaly change by putting in place code that
parses the structured error and raises the same errors as it did when
parsing the response for the analagous errors.

Changelog: changed

Gemfile

@@ -483,7 +483,7 @@ gem 'ssh_data', '~> 1.2'
 gem 'spamcheck', '~> 0.1.0'
 
 # Gitaly GRPC protocol definitions
-gem 'gitaly', '~> 14.10.0-rc1'
+gem 'gitaly', '~> 15.0.0-rc2'
 
 # KAS GRPC protocol definitions
 gem 'kas-grpc', '~> 0.0.2'

Gemfile.lock

@@ -465,7 +465,7 @@ GEM
       rails (>= 3.2.0)
     git (1.7.0)
       rchardet (~> 1.8)
-    gitaly (14.10.0.pre.rc1)
+    gitaly (15.0.0.pre.rc2)
       grpc (~> 1.0)
     github-markup (1.7.0)
     gitlab (4.16.1)
@@ -1504,7 +1504,7 @@ DEPENDENCIES
   gettext (~> 3.3)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.3)
-  gitaly (~> 14.10.0.pre.rc1)
+  gitaly (~> 15.0.0.pre.rc2)
   github-markup (~> 1.7.0)
   gitlab-chronic (~> 0.10.5)
   gitlab-dangerfiles (~> 3.0)

lib/gitlab/gitaly_client/operation_service.rb

@@ -464,6 +464,21 @@ 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)
+
+        case detailed_error&.error
+        when :access_check
+          access_check_error = detailed_error.access_check
+          # These messages were returned from internal/allowed API calls
+          raise Gitlab::Git::PreReceiveError.new(fallback_message: access_check_error.error_message)
+        when :cherry_pick_conflict
+          raise Gitlab::Git::Repository::CreateTreeError, "index conflict"
+        when :target_branch_diverged
+          raise Gitlab::Git::CommitError, "branch diverged"
+        else
+          raise e
+        end
       end
 
       def handle_cherry_pick_or_revert_response(response)

spec/lib/gitlab/gitaly_client/operation_service_spec.rb

@@ -340,6 +340,16 @@
     end
   end
 
+  shared_examples '#user_cherry_pick with a gRPC error' do
+    it 'raises a GitError exception' do
+      expect_any_instance_of(Gitaly::OperationService::Stub)
+      .to receive(:user_cherry_pick)
+      .and_raise(raised_error)
+
+      expect { subject }.to raise_error(expected_error)
+    end
+  end
+
   describe '#user_cherry_pick' do
     let(:response_class) { Gitaly::UserCherryPickResponse }
 
@@ -354,13 +364,71 @@
       )
     end
 
-    before do
-      expect_any_instance_of(Gitaly::OperationService::Stub)
-        .to receive(:user_cherry_pick).with(kind_of(Gitaly::UserCherryPickRequest), kind_of(Hash))
-        .and_return(response)
+    context 'when errors are not raised but returned in the response' do
+      before do
+        expect_any_instance_of(Gitaly::OperationService::Stub)
+          .to receive(:user_cherry_pick).with(kind_of(Gitaly::UserCherryPickRequest), kind_of(Hash))
+          .and_return(response)
+      end
+
+      it_behaves_like 'cherry pick and revert errors'
     end
 
-    it_behaves_like 'cherry pick and revert errors'
+    context 'when AccessError is raised' do
+      let(:raised_error) do
+        new_detailed_error(
+          GRPC::Core::StatusCodes::INTERNAL,
+          'something failed',
+          Gitaly::UserCherryPickError.new(
+            access_check: Gitaly::AccessCheckError.new(
+              error_message: 'something went wrong'
+            )))
+      end
+
+      let(:expected_error) { Gitlab::Git::PreReceiveError }
+
+      it_behaves_like '#user_cherry_pick with a gRPC error'
+    end
+
+    context 'when NotAncestorError is raised' do
+      let(:raised_error) do
+        new_detailed_error(
+          GRPC::Core::StatusCodes::FAILED_PRECONDITION,
+          'Branch diverged',
+          Gitaly::UserCherryPickError.new(
+            target_branch_diverged: Gitaly::NotAncestorError.new
+          )
+        )
+      end
+
+      let(:expected_error) { Gitlab::Git::CommitError }
+
+      it_behaves_like '#user_cherry_pick with a gRPC error'
+    end
+
+    context 'when NotAncestorError is raised' do
+      let(:raised_error) do
+        new_detailed_error(
+          GRPC::Core::StatusCodes::FAILED_PRECONDITION,
+          'Conflict',
+          Gitaly::UserCherryPickError.new(
+            cherry_pick_conflict: Gitaly::MergeConflictError.new
+          )
+        )
+      end
+
+      let(:expected_error) { Gitlab::Git::Repository::CreateTreeError }
+
+      it_behaves_like '#user_cherry_pick with a gRPC error'
+    end
+
+    context 'when a non-detailed gRPC error is raised' do
+      let(:raised_error) { GRPC::Internal.new('non-detailed error') }
+        
+      let(:expected_error) { GRPC::Internal }
+
+      it_behaves_like '#user_cherry_pick with a gRPC error'
+    end
   end
 
   describe '#user_revert' do