Skip to content

Clean up gRPC debug_error_string messages in Sentry

Stan Hu requested to merge sh-sanitize-grpc-debug-error-string into master

The upgrade of gRPC brought along a debug_error_string that makes it harder for Sentry to group similar exceptions together. There doesn't appear to be a way to turn this off, so we clean this up in a Sentry field processsor. Now exceptions will have a field grpc_debug_error_string field in the extra section.

Relates to #238465

Alternate ideas

We might want to use a gRPC interceptor and just sanitize the debug_error_string out completely. That might be easier than doing this, which doesn't help with leaking information in the UI.

Testing notes

  1. I had to disable the better_errors gem to make Sentry report exceptions that occurred in the Web interfae:
diff --git a/Gemfile b/Gemfile
index b45a2c67257..2e4e585aa1e 100644
--- a/Gemfile
+++ b/Gemfile
@@ -342,7 +342,7 @@ group :development do
   gem 'letter_opener_web', '~> 1.3.4'
 
   # Better errors handler
-  gem 'better_errors', '~> 2.7.1'
+  # gem 'better_errors', '~> 2.7.1'
   gem 'binding_of_caller', '~> 0.8.0'
 
   # thin instead webrick
diff --git a/config/environments/development.rb b/config/environments/development.rb
index 9d4fc6ba5e9..92eb40bdd69 100644
--- a/config/environments/development.rb
+++ b/config/environments/development.rb
@@ -50,7 +50,7 @@ Rails.application.configure do
   config.assets.quiet = true
 
   # BetterErrors live shell (REPL) on every stack frame
-  BetterErrors::Middleware.allow_ip!("127.0.0.1/0")
+  # BetterErrors::Middleware.allow_ip!("127.0.0.1/0")
 
   # Reassign some performance related settings when we profile the app
   if Gitlab::Utils.to_boolean(ENV['RAILS_PROFILE'].to_s)
  1. To get real debug_error_string messsages, I hacked Gitaly to sleep for a long time:
diff --git a/internal/service/commit/find_commit.go b/internal/service/commit/find_commit.go
index ca2cf7ca..2a6e5687 100644
--- a/internal/service/commit/find_commit.go
+++ b/internal/service/commit/find_commit.go
@@ -3,6 +3,8 @@ package commit
 import (
 	"context"
 
+	"time"
+
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/git/log"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
@@ -22,5 +24,7 @@ func (s *server) FindCommit(ctx context.Context, in *gitalypb.FindCommitRequest)
 		return &gitalypb.FindCommitResponse{}, nil
 	}
 
+	time.Sleep(60 * time.Second)
+
 	return &gitalypb.FindCommitResponse{Commit: commit}, err
 }

Then I hacked Rails to try to look up a commit:

diff --git a/app/controllers/admin/dashboard_controller.rb b/app/controllers/admin/dashboard_controller.rb
index b7b535e70df..0755214fdc9 100644
--- a/app/controllers/admin/dashboard_controller.rb
+++ b/app/controllers/admin/dashboard_controller.rb
@@ -9,6 +9,9 @@ class Admin::DashboardController < Admin::ApplicationController
   # rubocop: disable CodeReuse/ActiveRecord
   def index
     @counts = Gitlab::Database::Count.approximate_counts(COUNTED_ITEMS)
+    Project.first.commit
+    raise GRPC::BadStatus.new_status_exception(4, debug_error_string = 'hello again')

     @projects = Project.order_id_desc.without_deleted.with_route.limit(10)
     @users = User.order_id_desc.limit(10)
     @groups = Group.order_id_desc.with_route.limit(10)

diff --git a/lib/gitlab/git/wraps_gitaly_errors.rb b/lib/gitlab/git/wraps_gitaly_errors.rb
index 9963bcfbf1c..76bc30479b3 100644
--- a/lib/gitlab/git/wraps_gitaly_errors.rb
+++ b/lib/gitlab/git/wraps_gitaly_errors.rb
@@ -5,12 +5,12 @@ module Gitlab
     module WrapsGitalyErrors
       def wrapped_gitaly_errors(&block)
         yield block
-      rescue GRPC::NotFound => e
-        raise Gitlab::Git::Repository::NoRepository.new(e)
-      rescue GRPC::InvalidArgument => e
-        raise ArgumentError.new(e)
-      rescue GRPC::BadStatus => e
-        raise Gitlab::Git::CommandError.new(e)
+        #      rescue GRPC::NotFound => e
+        #        raise Gitlab::Git::Repository::NoRepository.new(e)
+        #      rescue GRPC::InvalidArgument => e
+        #        raise ArgumentError.new(e)
+        #      rescue GRPC::BadStatus => e
+        #        raise Gitlab::Git::CommandError.new(e)
       end
     end
   end
diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb
index 464d2519b27..5c1f4488c0d 100644
--- a/lib/gitlab/gitaly_client/commit_service.rb
+++ b/lib/gitlab/gitaly_client/commit_service.rb
@@ -452,7 +452,7 @@ module Gitlab
           revision: encode_binary(revision)
         )
 
-        response = GitalyClient.call(@repository.storage, :commit_service, :find_commit, request, timeout: GitalyClient.medium_timeout)
+        response = GitalyClient.call(@repository.storage, :commit_service, :find_commit, request, timeout: GitalyClient.fast_timeout)
 
         response.commit
       end
Edited by Stan Hu

Merge request reports