Merge pull request #1990 from Skumring/fix-sentry-capture-error-after-sentry-ruby-6-upgrade

Restore Sentry event delivery after sentry-ruby 6 upgrade

Author: dmitry-sinina

Gemfile

@@ -116,7 +116,7 @@ group :development, :test do
   gem 'bullet'
   gem 'byebug'
 
-  gem 'brakeman'
+  gem 'brakeman', require: false
   gem 'bundler-audit', require: false
   gem 'factory_bot_rails'
   gem 'parallel_tests'

config/initializers/sentry.rb

@@ -4,6 +4,6 @@
 # but only run the full configuration when sentry is enabled and not in a Rake task.
 require 'sentry-ruby'
 
-if CaptureError.enabled? && !defined?(::Rake)
+if CaptureError.enabled? && !CaptureError.rake_task_invocation?
   CaptureError.configure!
 end

lib/capture_error.rb

@@ -14,6 +14,19 @@ def enabled?
       YetiConfig.sentry.enabled
     end
 
+    # Checks whether the current process is executing a Rake task.
+    #
+    # `defined?(::Rake)` is unreliable here: `Rake` is an ordinary Ruby constant
+    # and gets loaded into long-running processes (puma, delayed_job, schedulers)
+    # transitively via `Bundler.require`. `Rake.application.top_level_tasks` is
+    # only populated when Rake is actually executing tasks.
+    # @return [TrueClass,FalseClass]
+    def rake_task_invocation?
+      return false unless defined?(::Rake)
+
+      ::Rake.application.top_level_tasks.any?
+    end
+
     # Configures Sentry gem.
     # Use only once in initializer.
     def configure!
@@ -32,14 +45,7 @@ def configure!
         sentry_config.inspect_exception_causes_for_exclusion = false
 
         # use Rails' parameter filter to sanitize the event
-        filters = [
-          ActiveSupport::ParameterFilter.new(Rails.application.config.filter_parameters),
-          ActiveSupport::ParameterFilter.new(['Authorization'])
-        ]
-        sentry_config.before_send = lambda { |event, _hint = nil|
-          filters.each { |filter| event = filter.filter(event.to_hash) }
-          event
-        }
+        sentry_config.before_send = build_before_send
 
         # Usage of an async option is discouraged by sentry-ruby developers.
         # By default sentry will create its own background workers to send sentry events.
@@ -170,6 +176,40 @@ def with_clean_context(context)
 
     private
 
+    # Builds the `before_send` lambda that sanitizes events using Rails'
+    # configured `filter_parameters` plus an explicit `Authorization` filter.
+    #
+    # Sentry-ruby 6 requires `before_send` to return a `Sentry::ErrorEvent`
+    # (events that come back as `Hash` are dropped by `Sentry::Client#send_event`
+    # with `Discarded event because before_send didn't return a Sentry::ErrorEvent`).
+    # We therefore mutate the event in place via its public setters and return
+    # the event itself.
+    # @return [Proc]
+    def build_before_send
+      filters = [
+        ActiveSupport::ParameterFilter.new(Rails.application.config.filter_parameters),
+        ActiveSupport::ParameterFilter.new(['Authorization'])
+      ]
+      lambda do |event, _hint = nil|
+        filters.each { |filter| filter_event!(event, filter) }
+        event
+      end
+    end
+
+    def filter_event!(event, filter)
+      event.user     = filter.filter(event.user)     if event.user.is_a?(Hash)
+      event.extra    = filter.filter(event.extra)    if event.extra.is_a?(Hash)
+      event.tags     = filter.filter(event.tags)     if event.tags.is_a?(Hash)
+      event.contexts = filter.filter(event.contexts) if event.contexts.is_a?(Hash)
+
+      return unless event.request
+
+      request = event.request
+      request.data    = filter.filter(request.data)    if request.data.is_a?(Hash)
+      request.headers = filter.filter(request.headers) if request.headers.is_a?(Hash)
+      request.env     = filter.filter(request.env)     if request.env.is_a?(Hash)
+    end
+
     def with_capture_context(context, exception: nil)
       Sentry.with_scope do |scope|
         if scope

spec/lib/capture_error_spec.rb

@@ -1,6 +1,99 @@
 # frozen_string_literal: true
 
 RSpec.describe CaptureError do
+  describe '.rake_task_invocation?' do
+    subject { described_class.rake_task_invocation? }
+
+    context 'when Rake is not loaded' do
+      before { hide_const('Rake') }
+
+      it { is_expected.to be false }
+    end
+
+    context 'when Rake is loaded but no top-level tasks (puma, scheduler, console)' do
+      before do
+        allow(Rake.application).to receive(:top_level_tasks).and_return([])
+      end
+
+      it { is_expected.to be false }
+    end
+
+    context 'when Rake is executing a top-level task' do
+      before do
+        allow(Rake.application).to receive(:top_level_tasks).and_return(['db:migrate'])
+      end
+
+      it { is_expected.to be true }
+    end
+  end
+
+  describe '#build_before_send (private)' do
+    subject(:before_send) { described_class.send(:build_before_send) }
+
+    let(:configuration) do
+      config = Sentry::Configuration.new
+      config.dsn = 'http://public@example.invalid/1'
+      config
+    end
+    let(:event) { Sentry::ErrorEvent.new(configuration: configuration) }
+
+    before do
+      allow(Rails.application.config).to receive(:filter_parameters).and_return([:password])
+    end
+
+    it 'should return the same Sentry::ErrorEvent object (not a Hash)' do
+      result = before_send.call(event)
+      expect(result).to be(event)
+      expect(result).to be_a(Sentry::ErrorEvent)
+    end
+
+    it 'should mask filter_parameters keys in event.extra and keep other keys' do
+      event.extra = { password: 'secret', visible: 'ok' }
+      result = before_send.call(event)
+      expect(result.extra).to eq(password: '[FILTERED]', visible: 'ok')
+    end
+
+    it 'should mask Authorization in event.tags' do
+      event.tags = { 'Authorization' => 'Bearer abc', 'probe' => 'on' }
+      result = before_send.call(event)
+      expect(result.tags).to eq('Authorization' => '[FILTERED]', 'probe' => 'on')
+    end
+
+    it 'should mask filter_parameters keys in event.user and event.contexts' do
+      event.user = { id: 1, password: 'pw' }
+      event.contexts = { extra_ctx: { password: 'pw2', note: 'visible' } }
+      result = before_send.call(event)
+      expect(result.user).to eq(id: 1, password: '[FILTERED]')
+      expect(result.contexts).to eq(extra_ctx: { password: '[FILTERED]', note: 'visible' })
+    end
+
+    it 'should mask filter_parameters keys and Authorization in event.request fields' do
+      request = Sentry::RequestInterface.allocate
+      request.data = { password: 'pw', visible: 'ok' }
+      request.headers = { 'Authorization' => 'Bearer xyz', 'X-Trace' => 't1' }
+      request.env = { 'HTTP_AUTHORIZATION' => 'Bearer xyz', 'REMOTE_ADDR' => '127.0.0.1' }
+      event.instance_variable_set(:@request, request)
+
+      result = before_send.call(event)
+      expect(result.request.data).to eq(password: '[FILTERED]', visible: 'ok')
+      expect(result.request.headers).to eq('Authorization' => '[FILTERED]', 'X-Trace' => 't1')
+      expect(result.request.env).to eq('HTTP_AUTHORIZATION' => '[FILTERED]', 'REMOTE_ADDR' => '127.0.0.1')
+    end
+
+    it 'should not raise when event.request is nil' do
+      event.extra = { password: 'pw' }
+      expect { before_send.call(event) }.not_to raise_error
+    end
+
+    it 'should not raise when event fields are nil' do
+      event.extra = nil
+      event.tags = nil
+      event.user = nil
+      event.contexts = nil
+      expect { before_send.call(event) }.not_to raise_error
+    end
+  end
+
   describe '.with_clean_context' do
     subject do
       described_class.with_clean_context(context) { block_result }