Unsilence BacktraceCleaner with BACKTRACE without removing silencers

rails/rails@957a3e5 allowed the BACKTRACE env to unsilence backtraces in normals runs not just tests. However, the implementation achieves this by removing the silences in an initializer. This means that the state of backtrace silencer becomes cached by spring and the developer needs to reset spring every time they want to silence or unsilence the backtrace. It also means custom silencers added in an initializer need to be added after the :configure_backtrace_cleaner initializer or else they won't be un-silenced properly.

Instead, let's teach Rails::BacktraceCleaner to unsilence the backtrace if the BACKTRACE variable is set. ActiveSupport::BacktraceCleaner#clean already allows this by passing nil as the second argument, which just filters but does not silence the backtrace. So, in Rails::BacktraceCleaner#clean we can call super with nil in the second argument if the BACKTRACE variable is present. That way we do not need to remove the silencers in an initializer, and we don't have to reset spring to silence the backtrace.

Author: andrewn617

railties/lib/rails/application/bootstrap.rb

@@ -75,10 +75,6 @@ module Bootstrap
         end
       end
 
-      initializer :configure_backtrace_cleaner, group: :all do
-        Rails.backtrace_cleaner.remove_silencers! if ENV["BACKTRACE"]
-      end
-
       # Initialize cache early in the stack so railties can make use of it.
       initializer :initialize_cache, group: :all do
         cache_format_version = config.active_support.delete(:cache_format_version)

railties/lib/rails/backtrace_cleaner.rb

@@ -25,5 +25,18 @@ def initialize
       end
       add_silencer { |line| !APP_DIRS_PATTERN.match?(line) }
     end
+
+    def clean(backtrace, kind = :silent)
+      kind = nil if ENV["BACKTRACE"]
+
+      super(backtrace, kind)
+    end
+    alias_method :filter, :clean
+
+    def clean_frame(frame, kind = :silent)
+      kind = nil if ENV["BACKTRACE"]
+
+      super(frame, kind)
+    end
   end
 end

railties/test/application/backtrace_cleaner_test.rb

@@ -1,68 +1,55 @@
 # frozen_string_literal: true
 
-require "isolation/abstract_unit"
-require "rack/test"
 require "env_helpers"
 
 module ApplicationTests
   class BacktraceCleanerTest < ActiveSupport::TestCase
-    include ActiveSupport::Testing::Isolation
-    include Rack::Test::Methods
     include EnvHelpers
 
-    def teardown
-      teardown_app
+    setup do
+      @cleaner = Rails::BacktraceCleaner.new
     end
 
-    test "backtrace is cleaned" do
-      setup_app
+    test "#clean silences Rails code from backtrace" do
+      backtrace = [
+        "app/controllers/foo_controller.rb:4:in 'index'",
+        "rails/railties/lib/rails/engine.rb:536:in `call"
+      ]
 
-      app("development")
-      get "/"
-      if RUBY_VERSION >= "3.4"
-        assert_includes last_response.body, "app/app/controllers/foo_controller.rb:4:in 'FooController#index'"
-      else
-        assert_includes last_response.body, "app/app/controllers/foo_controller.rb:4:in `index'"
-      end
-      assert_not_includes last_response.body, "rails/railties/test/env_helpers.rb"
+      cleaned = @cleaner.clean(backtrace)
+
+      assert_equal ["app/controllers/foo_controller.rb:4:in 'index'"], cleaned
     end
 
-    test "backtrace is not cleaned" do
+    test "#clean does not silence when BACKTRACE is set" do
       switch_env("BACKTRACE", "1") do
-        setup_app
+        backtrace = [
+          "app/app/controllers/foo_controller.rb:4:in 'index'",
+          "/rails/railties/lib/rails/engine.rb:536:in `call"
+        ]
+
+        cleaned = @cleaner.clean(backtrace)
 
-        app("development")
-        get "/"
-        if RUBY_VERSION >= "3.4"
-          assert_includes last_response.body, "app/app/controllers/foo_controller.rb:4:in 'FooController#index'"
-        else
-          assert_includes last_response.body, "app/app/controllers/foo_controller.rb:4:in `index'"
-        end
-        assert_includes last_response.body, "rails/railties/test/env_helpers.rb"
+        assert_equal backtrace, cleaned
       end
     end
 
-    private
-      def setup_app
-        build_app
+    test "#clean_frame silences Rails code" do
+      frame = "rails/railties/lib/rails/engine.rb:536:in `call"
 
-        controller :foo, <<-RUBY
-          class FooController < ApplicationController
-            def index
-              begin
-                raise "ERROR"
-              rescue StandardError => e
-                render plain: e.backtrace.join("\n")
-              end
-            end
-          end
-        RUBY
+      cleaned = @cleaner.clean_frame(frame)
+
+      assert_equal nil, cleaned
+    end
 
-        app_file "config/routes.rb", <<-RUBY
-          Rails.application.routes.draw do
-            root to: "foo#index"
-          end
-        RUBY
+    test "#clean_frame does not silence when BACKTRACE is set" do
+      switch_env("BACKTRACE", "1") do
+        frame = "rails/railties/lib/rails/engine.rb:536:in `call"
+
+        cleaned = @cleaner.clean_frame(frame)
+
+        assert_equal frame, cleaned
       end
+    end
   end
 end