Skip to content

Commit

Permalink
ActiveSupport::ErrorReporter#report assigns a backtrace to unraised e…
Browse files Browse the repository at this point in the history
…xceptions

Previously reporting an un-raised exception would result in an error report without
a backtrace. Now it automatically generates one.
  • Loading branch information
byroot committed Aug 23, 2024
1 parent ca05b17 commit c834de6
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 1 deletion.
7 changes: 7 additions & 0 deletions activesupport/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
26 changes: 25 additions & 1 deletion activesupport/lib/active_support/error_reporter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ 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?
ensure_backtrace(error)

if @debug_mode
raise UnexpectedError, "#{error.class.name}: #{error.message}", error.backtrace, cause: error
Expand Down Expand Up @@ -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}"
Expand Down Expand Up @@ -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
11 changes: 11 additions & 0 deletions activesupport/test/error_reporter_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit c834de6

Please sign in to comment.