Merge pull request rails#52684 from Shopify/error-subscriber-backtrace

ActiveSupport::ErrorReporter#report assigns a backtrace to unraised exceptions

Author: byroot

activesupport/CHANGELOG.md

@@ -1,3 +1,10 @@
+*   `ActiveSupport::ErrorReporter#report` now assigns a backtrace to unraised exceptions.
+
+    Previously reporting an un-raised exception would result in an error report without
+    a backtrace. Now it automatically generates one.
+
+    *Jean Boussier*
+
 *   Add `escape_html_entities` option to `ActiveSupport::JSON.encode`.
 
     This allows for overriding the global configuration found at

activesupport/lib/active_support/error_reporter.rb

@@ -144,9 +144,9 @@ def record(*error_classes, severity: :error, context: {}, source: DEFAULT_SOURCE
     #
     def unexpected(error, severity: :warning, context: {}, source: DEFAULT_SOURCE)
       error = RuntimeError.new(error) if error.is_a?(String)
-      error.set_backtrace(caller(1)) if error.backtrace.nil?
 
       if @debug_mode
+        ensure_backtrace(error)
         raise UnexpectedError, "#{error.class.name}: #{error.message}", error.backtrace, cause: error
       else
         report(error, handled: true, severity: severity, context: context, source: source)
@@ -209,6 +209,7 @@ def set_context(...)
     #
     def report(error, handled: true, severity: handled ? :warning : :error, context: {}, source: DEFAULT_SOURCE)
       return if error.instance_variable_defined?(:@__rails_error_reported)
+      ensure_backtrace(error)
 
       unless SEVERITIES.include?(severity)
         raise ArgumentError, "severity must be one of #{SEVERITIES.map(&:inspect).join(", ")}, got: #{severity.inspect}"
@@ -237,5 +238,28 @@ def report(error, handled: true, severity: handled ? :warning : :error, context:
 
       nil
     end
+
+    private
+      def ensure_backtrace(error)
+        return if error.frozen? # re-raising won't add a backtrace
+        return unless error.backtrace.nil?
+
+        begin
+          # We could use Exception#set_backtrace, but until Ruby 3.4
+          # it only support setting `Exception#backtrace` and not
+          # `Exception#backtrace_locations`. So raising the exception
+          # is a good way to build a real backtrace.
+          raise error
+        rescue error.class => error
+        end
+
+        count = 0
+        while error.backtrace_locations.first&.path == __FILE__
+          count += 1
+          error.backtrace_locations.shift
+        end
+
+        error.backtrace.shift(count)
+      end
   end
 end

activesupport/test/error_reporter_test.rb

@@ -161,6 +161,16 @@ class ErrorReporterTest < ActiveSupport::TestCase
     assert_equal [[error, false, :error, "application", {}]], @subscriber.events
   end
 
+  test "#report assigns a backtrace if it's missing" do
+    error = RuntimeError.new("Oops")
+    assert_nil error.backtrace
+    assert_nil error.backtrace_locations
+
+    assert_nil @reporter.report(error)
+    assert_not_predicate error.backtrace, :empty?
+    assert_not_predicate error.backtrace_locations, :empty?
+  end
+
   test "#record passes through the return value" do
     result = @reporter.record do
       2 + 2
@@ -173,6 +183,7 @@ class ErrorReporterTest < ActiveSupport::TestCase
     assert_nil @reporter.unexpected(error)
     assert_equal [[error, true, :warning, "application", {}]], @subscriber.events
     assert_not_predicate error.backtrace, :empty?
+    assert_not_predicate error.backtrace_locations, :empty?
   end
 
   test "#unexpected accepts an error message" do