From 157f958b5d304eb0ab16d0a31fc5768881b86d30 Mon Sep 17 00:00:00 2001
From: Stan Lo <stan001212@gmail.com>
Date: Fri, 23 Jul 2021 12:21:38 +0800
Subject: [PATCH] Fix sentry-rails' backtrace_cleanup_callback injection
 (#1510)

* 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 https://github.com/getsentry/sentry-ruby/issues/1076#issuecomment-884754612.

* Update changelog
---
 CHANGELOG.md                                   |  1 +
 sentry-rails/lib/sentry/rails/railtie.rb       |  2 +-
 .../sentry/rails/controller_methods_spec.rb    |  2 +-
 sentry-rails/spec/sentry/rails_spec.rb         | 18 +++++++-----------
 sentry-rails/spec/spec_helper.rb               | 10 ----------
 5 files changed, 10 insertions(+), 23 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6164242c8..d7ea98a2a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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
 
diff --git a/sentry-rails/lib/sentry/rails/railtie.rb b/sentry-rails/lib/sentry/rails/railtie.rb
index 4fc9afb16..9e20890d4 100644
--- a/sentry-rails/lib/sentry/rails/railtie.rb
+++ b/sentry-rails/lib/sentry/rails/railtie.rb
@@ -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
diff --git a/sentry-rails/spec/sentry/rails/controller_methods_spec.rb b/sentry-rails/spec/sentry/rails/controller_methods_spec.rb
index ba7f646e6..58fb02d51 100644
--- a/sentry-rails/spec/sentry/rails/controller_methods_spec.rb
+++ b/sentry-rails/spec/sentry/rails/controller_methods_spec.rb
@@ -9,7 +9,7 @@ def request
   end
 
   before do
-    perform_basic_setup
+    make_basic_app
   end
 
   let(:options) do
diff --git a/sentry-rails/spec/sentry/rails_spec.rb b/sentry-rails/spec/sentry/rails_spec.rb
index a9ca201ef..326d2673e 100644
--- a/sentry-rails/spec/sentry/rails_spec.rb
+++ b/sentry-rails/spec/sentry/rails_spec.rb
@@ -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
diff --git a/sentry-rails/spec/spec_helper.rb b/sentry-rails/spec/spec_helper.rb
index e196b0281..b3acf7e3b 100644
--- a/sentry-rails/spec/spec_helper.rb
+++ b/sentry-rails/spec/spec_helper.rb
@@ -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