Strip gRPC debug_error_string from Gitaly exceptions

This reverts https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40938
because it wasn't actually working when gRPC exceptions were wrapped in
`Gitlab::Git::CommandError` via `wrapped_gitaly_errors`.

A much simpler approach is to strip `debug_error_string` when a
`Gitlab::Git::BaseError` is created. With this change, we have
effectively the same exception strings as we did before the upgrade of
gRPC.

Note that it appears Sentry retains a backtrace of the original gRPC
exception that contains the `debug_error_string`, so if we really want
to look at it the information is still there.

This should solve https://gitlab.com/gitlab-org/gitlab/-/issues/238465.

lib/gitlab/error_tracking.rb

@@ -26,8 +26,6 @@ module Gitlab
           # Sanitize fields based on those sanitized from Rails.
           config.sanitize_fields = Rails.application.config.filter_parameters.map(&:to_s)
           config.processors << ::Gitlab::ErrorTracking::Processor::SidekiqProcessor
-          config.processors << ::Gitlab::ErrorTracking::Processor::GrpcErrorProcessor
-
           # Sanitize authentication headers
           config.sanitize_http_headers = %w[Authorization Private-Token]
           config.tags = extra_tags_from_env.merge(program: Gitlab.process_name)

lib/gitlab/error_tracking/processor/grpc_error_processor.rb

-# frozen_string_literal: true
-
-module Gitlab
-  module ErrorTracking
-    module Processor
-      class GrpcErrorProcessor < ::Raven::Processor
-        DEBUG_ERROR_STRING_REGEX = RE2('(.*) debug_error_string:(.*)')
-
-        def process(value)
-          return value unless grpc_exception?(value)
-
-          process_message(value)
-          process_exception_values(value)
-          process_custom_fingerprint(value)
-
-          value
-        end
-
-        def grpc_exception?(value)
-          value[:exception] && value[:message].start_with?('GRPC::')
-        end
-
-        def process_message(value)
-          message, debug_str = split_debug_error_string(value[:message])
-
-          return unless message
-
-          value[:message] = message
-          extra = value[:extra] || {}
-          extra[:grpc_debug_error_string] = debug_str if debug_str
-        end
-
-        def process_exception_values(value)
-          exceptions = value.dig(:exception, :values)
-
-          return unless exceptions.is_a?(Array)
-
-          exceptions.each do |entry|
-            message, _ = split_debug_error_string(entry[:value])
-            entry[:value] = message if message
-          end
-        end
-
-        def process_custom_fingerprint(value)
-          fingerprint = value[:fingerprint]
-
-          return value unless custom_grpc_fingerprint?(fingerprint)
-
-          message, _ = split_debug_error_string(fingerprint[1])
-          fingerprint[1] = message if message
-        end
-
-        private
-
-        def custom_grpc_fingerprint?(fingerprint)
-          fingerprint.is_a?(Array) && fingerprint.length == 2 && fingerprint[0].start_with?('GRPC::')
-        end
-
-        def split_debug_error_string(message)
-          return unless message
-
-          match = DEBUG_ERROR_STRING_REGEX.match(message)
-
-          return unless match
-
-          [match[1], match[2]]
-        end
-      end
-    end
-  end
-end

lib/gitlab/git.rb

@@ -13,7 +13,6 @@ module Gitlab
     TAG_REF_PREFIX = "refs/tags/"
     BRANCH_REF_PREFIX = "refs/heads/"
 
-    BaseError = Class.new(StandardError)
     CommandError = Class.new(BaseError)
     CommitError = Class.new(BaseError)
     OSError = Class.new(BaseError)

lib/gitlab/git/base_error.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Git
+    class BaseError < StandardError
+      DEBUG_ERROR_STRING_REGEX = /(.*?) debug_error_string:.*$/m.freeze
+
+      def initialize(msg = nil)
+        if msg
+          raw_message = msg.to_s
+          match = DEBUG_ERROR_STRING_REGEX.match(raw_message)
+          raw_message = match[1] if match
+
+          super(raw_message)
+        else
+          super
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/error_tracking/processor/grpc_error_processor_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Gitlab::ErrorTracking::Processor::GrpcErrorProcessor do
-  describe '#process' do
-    subject { described_class.new }
-
-    context 'when there is no GRPC exception' do
-      let(:data) { { fingerprint: ['ArgumentError', 'Missing arguments'] } }
-
-      it 'leaves data unchanged' do
-        expect(subject.process(data)).to eq(data)
-      end
-    end
-
-    context 'when there is a GPRC exception with a debug string' do
-      let(:data) do
-        {
-          exception: {
-            values: [
-              {
-                value: "GRPC::DeadlineExceeded: 4:DeadlineExceeded. debug_error_string:{\"hello\":1}"
-              }
-            ]
-          },
-          extra: {
-            caller: 'test'
-          },
-          message: "GRPC::DeadlineExceeded: 4:DeadlineExceeded. debug_error_string:{\"hello\":1}",
-          fingerprint: [
-            "GRPC::DeadlineExceeded",
-            "4:Deadline Exceeded. debug_error_string:{\"created\":\"@1598938192.005782000\",\"description\":\"Error received from peer unix:/home/git/gitalypraefect.socket\",\"file\":\"src/core/lib/surface/call.cc\",\"file_line\":1055,\"grpc_message\":\"Deadline Exceeded\",\"grpc_status\":4}"
-          ]
-        }
-      end
-
-      let(:expected) do
-        {
-          message: "GRPC::DeadlineExceeded: 4:DeadlineExceeded.",
-          fingerprint: [
-            "GRPC::DeadlineExceeded",
-            "4:Deadline Exceeded."
-          ],
-          exception: {
-            values: [
-              {
-                value: "GRPC::DeadlineExceeded: 4:DeadlineExceeded."
-              }
-            ]
-          },
-          extra: {
-            caller: 'test',
-            grpc_debug_error_string: "{\"hello\":1}"
-         }
-        }
-      end
-
-      it 'removes the debug error string and stores it as an extra field' do
-        expect(subject.process(data)).to eq(expected)
-      end
-
-      context 'with no custom fingerprint' do
-        before do
-          data.delete(:fingerprint)
-          expected.delete(:fingerprint)
-        end
-
-        it 'removes the debug error string and stores it as an extra field' do
-          expect(subject.process(data)).to eq(expected)
-        end
-      end
-    end
-  end
-end

spec/lib/gitlab/git/base_error_spec.rb

+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require 'rspec-parameterized'
+
+RSpec.describe Gitlab::Git::BaseError do
+  using RSpec::Parameterized::TableSyntax
+
+  subject { described_class.new(message).to_s }
+
+  where(:message, :result) do
+    "GRPC::DeadlineExceeded: 4:DeadlineExceeded. debug_error_string:{\"hello\":1}" | "GRPC::DeadlineExceeded: 4:DeadlineExceeded."
+    "GRPC::DeadlineExceeded: 4:DeadlineExceeded." | "GRPC::DeadlineExceeded: 4:DeadlineExceeded."
+    "GRPC::DeadlineExceeded: 4:DeadlineExceeded. debug_error_string:{\"created\":\"@1598978902.544524530\",\"description\":\"Error received from peer ipv4: debug_error_string:test\"}" | "GRPC::DeadlineExceeded: 4:DeadlineExceeded."
+    "9:Multiple lines\nTest line. debug_error_string:{\"created\":\"@1599074877.106467000\"}" | "9:Multiple lines\nTest line."
+    "other message" | "other message"
+    nil | "Gitlab::Git::BaseError"
+  end
+
+  with_them do
+    it { is_expected.to eq(result) }
+  end
+end