Skip to content

Commit

Permalink
Inspect exception cause by default & don't exclude ActiveJob::Deseria…
Browse files Browse the repository at this point in the history
…lizationError (#1180)

* Turn on inspect_exception_causes_for_exclusion by default

With this config turned on, we can avoid matching the surface exceptions
in integrations, which could cause issues like #1071.

Solves #642.

* Remove ActiveJob::DeserializationError from ignored list

Since the previous commit solves #642, this commit can remove
ActiveJob::DeserializationError from the ignored exceptions list.

Solves #1071.

* Update async document

* Update changelog
  • Loading branch information
st0012 committed Jan 17, 2021
1 parent 551410b commit 76415b4
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 2 deletions.
5 changes: 5 additions & 0 deletions sentry-rails/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# Changelog

## Unreleased (4.2.0)

- Inspect exception cause by default & don't exclude ActiveJob::DeserializationError [#1180](https://github.com/getsentry/sentry-ruby/pull/1180)
- Fixes [#1071](https://github.com/getsentry/sentry-ruby/issues/1071)

## 4.1.4

- Don't include headers & request info in tracing span or breadcrumb [#1199](https://github.com/getsentry/sentry-ruby/pull/1199)
Expand Down
1 change: 0 additions & 1 deletion sentry-rails/lib/sentry/rails/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ module Rails
'ActionController::UnknownFormat',
'ActionController::UnknownHttpMethod',
'ActionDispatch::Http::Parameters::ParseError',
'ActiveJob::DeserializationError', # Can cause infinite loops
'ActiveRecord::RecordNotFound'
].freeze
class Configuration
Expand Down
47 changes: 47 additions & 0 deletions sentry-rails/spec/sentry/rails/activejob_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,53 @@ def rescue_callback(error); end
expect(Sentry.get_current_scope.extra).to eq({})
end

context "when DeserializationError happens in user's jobs" do
before do
make_basic_app
end

class DeserializationErrorJob < ActiveJob::Base
def perform
1/0
rescue
raise ActiveJob::DeserializationError
end
end

it "reports the root cause to Sentry" do
expect do
DeserializationErrorJob.perform_now
end.to raise_error(ActiveJob::DeserializationError, /divided by 0/)

expect(transport.events.size).to eq(1)

event = transport.events.last.to_json_compatible
expect(event.dig("exception", "values", 0, "type")).to eq("ZeroDivisionError")
end

context "and in SentryJob too" do
before do
Sentry.configuration.async = lambda do |event|
SentryJob.perform_now(event)
end
end

class SentryJob < ActiveJob::Base
def perform(event)
Post.find(0)
rescue
raise ActiveJob::DeserializationError
end
end

it "doesn't cause infinite loop" do
expect do
DeserializationErrorJob.perform_now
end.to raise_error(ActiveJob::DeserializationError, /divided by 0/)
end
end
end

context 'using rescue_from' do
it 'does not trigger Sentry' do
job = RescuedActiveJob.new
Expand Down
2 changes: 2 additions & 0 deletions sentry-ruby/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
## Unreleased (4.2.0)

- Add ThreadsInterface [#1178](https://github.com/getsentry/sentry-ruby/pull/1178)
- Inspect exception cause by default & don't exclude ActiveJob::DeserializationError [#1180](https://github.com/getsentry/sentry-ruby/pull/1180)
- Fixes [#1071](https://github.com/getsentry/sentry-ruby/issues/1071)

## 4.1.4

Expand Down
1 change: 1 addition & 0 deletions sentry-ruby/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ config.async = lambda { |event, hint| SentryJob.perform_later(event, hint) }

class SentryJob < ActiveJob::Base
queue_as :default
discard_on ActiveJob::DeserializationError # this will prevent infinite loop when there's an issue deserializing SentryJob

def perform(event, hint)
Sentry.send_event(event, hint)
Expand Down
2 changes: 1 addition & 1 deletion sentry-ruby/lib/sentry/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ def initialize
self.enabled_environments = []
self.exclude_loggers = []
self.excluded_exceptions = IGNORE_DEFAULT.dup
self.inspect_exception_causes_for_exclusion = false
self.inspect_exception_causes_for_exclusion = true
self.linecache = ::Sentry::LineCache.new
self.logger = ::Sentry::Logger.new(STDOUT)
self.project_root = detect_project_root
Expand Down
4 changes: 4 additions & 0 deletions sentry-ruby/spec/sentry/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,10 @@ class MyTestException < RuntimeError; end
context 'when the raised exception has a cause that is in excluded_exceptions' do
let(:incoming_exception) { build_exception_with_cause(MyTestException.new) }
context 'when inspect_exception_causes_for_exclusion is false' do
before do
subject.inspect_exception_causes_for_exclusion = false
end

it 'returns true' do
expect(subject.exception_class_allowed?(incoming_exception)).to eq true
end
Expand Down

0 comments on commit 76415b4

Please sign in to comment.