Skip to content

Commit

Permalink
Fix sentry-rails' backtrace_cleanup_callback injection (#1510)
Browse files Browse the repository at this point in the history
* Remove perform_basic_setup test helper to avoid misuse

* Don't override backtrace_cleanup_callback set by the user

`sentry-rails` should check the presence `backtrace_cleanup_callback`
before assigning the default one.

The problem was spotted in #1076 (comment).

* Update changelog
  • Loading branch information
st0012 committed Jul 23, 2021
1 parent bae8c5b commit 157f958
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 23 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
- Fixes [#1502](https://github.com/getsentry/sentry-ruby/issues/1502)
- Declare `delayed_job` and `sidekiq` as integration gem's dependency [#1506](https://github.com/getsentry/sentry-ruby/pull/1506)
- `DSN#server` shouldn't include path [#1505](https://github.com/getsentry/sentry-ruby/pull/1505)
- Fix `sentry-rails`' `backtrace_cleanup_callback` injection [#1510](https://github.com/getsentry/sentry-ruby/pull/1510)

## 4.6.1

Expand Down
2 changes: 1 addition & 1 deletion sentry-rails/lib/sentry/rails/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ def inject_breadcrumbs_logger
def setup_backtrace_cleanup_callback
backtrace_cleaner = Sentry::Rails::BacktraceCleaner.new

Sentry.configuration.backtrace_cleanup_callback = lambda do |backtrace|
Sentry.configuration.backtrace_cleanup_callback ||= lambda do |backtrace|
backtrace_cleaner.clean(backtrace)
end
end
Expand Down
2 changes: 1 addition & 1 deletion sentry-rails/spec/sentry/rails/controller_methods_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def request
end

before do
perform_basic_setup
make_basic_app
end

let(:options) do
Expand Down
18 changes: 7 additions & 11 deletions sentry-rails/spec/sentry/rails_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -160,19 +160,15 @@
end

it "doesn't filters exception backtrace if backtrace_cleanup_callback is overridden" do
original_cleanup_callback = Sentry.configuration.backtrace_cleanup_callback

begin
Sentry.configuration.backtrace_cleanup_callback = nil
make_basic_app do |config|
config.backtrace_cleanup_callback = lambda { |backtrace| backtrace }
end

get "/view_exception"
get "/view_exception"

traces = event.dig("exception", "values", 0, "stacktrace", "frames")
expect(traces.dig(-1, "filename")).to eq("inline template")
expect(traces.dig(-1, "function")).not_to be_nil
ensure
Sentry.configuration.backtrace_cleanup_callback = original_cleanup_callback
end
traces = event.dig("exception", "values", 0, "stacktrace", "frames")
expect(traces.dig(-1, "filename")).to eq("inline template")
expect(traces.dig(-1, "function")).not_to be_nil
end

context "with config.exceptions_app = self.routes" do
Expand Down
10 changes: 0 additions & 10 deletions sentry-rails/spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,3 @@ def reload_send_event_job
expect(defined?(Sentry::SendEventJob)).to eq(nil)
load File.join(Dir.pwd, "app", "jobs", "sentry", "send_event_job.rb")
end

def perform_basic_setup
Sentry.init do |config|
config.dsn = DUMMY_DSN
config.logger = ::Logger.new(nil)
config.transport.transport_class = Sentry::DummyTransport
# for sending events synchronously
config.background_worker_threads = 0
end
end

0 comments on commit 157f958

Please sign in to comment.