getsentry / sentry-ruby

Sentry SDK for Ruby
https://sentry.io/for/ruby
MIT License
927 stars 493 forks source link

Test helper `extract_sentry_exceptions` does not work with parameters filtering #2274

Closed ixti closed 5 months ago

ixti commented 5 months ago

Issue Description

When using parameter filter in before_send, the result is Hash, but extract_sentry_exceptions expects the instance of Event.

Reproduction Steps

Configure Sentry

Sentry.init do |config|
  # ...

  parameter_filter   = ActiveSupport::ParameterFilter.new(Rails.application.config.filter_parameters)
  config.before_send = lambda do |event, _hint|
    parameter_filter.filter(event.to_hash)
  end

  # ...
end

Sample app code:

class Reproduction
  def call
    raise "Boom!"
  rescue => e
    Sentry.capture_exception(e)
  end
end

Sample test code:

RSpec.describe Reproduction do
  describe "#example" do
    it "reports to Sentry" do
      expect { Reproduction.new.call }.not_to raise_error

      sentry_event = sentry_events.first
      event_cause  = extract_sentry_exceptions(sentry_event).first

      expect(event_cause.type).to eq("RuntimeError")
    end
  end
end

Expected Behavior

Specs should pass.

Actual Behavior

Spec fails, due to:

 NoMethodError:
   undefined method `exception' for an instance of Hash
 # /home/ixti/.gem/ruby/3.3.0/gems/sentry-ruby-5.16.1/lib/sentry/test_helper.rb:81:in `extract_sentry_exceptions'

Ruby Version

3.3.0

SDK Version

5.16.1

Integration and Its Version

rails, sidekiq (SDK matching versions)

Sentry Config

No response

ixti commented 5 months ago

Without using said helper, I can achieve desired goals:

      event_cause = sentry_events.first.to_hash[:exception]&.values&.dig(0, 0, :type)

      expect(event_cause).to eq("RuntimeError")
sl0thentr0py commented 5 months ago

hey @ixti , you should return an Event in your before_send, the hash support is only for the previous async mode and will be removed in the next major.

I know using the parameter filter is convenient this way but we should find another solution for this, you're not supposed to process it like that.

There is an EventScrubber I need to implement for this use case but I haven't gotten around to it.

ixti commented 5 months ago

@sl0thentr0py Oh, got it. Glad I caught it before then :D Wasn't aware of this.