Error reporting API: Add a source attribute

Ref: https://github.com/rails/rails/pull/43625#issuecomment-1109595539

Some users may not be interested by some internal errors.
By providing a `source` attribute we allow to easilly filter
these errors out.
This commit is contained in:
Jean Boussier 2022-05-02 12:02:39 +02:00
parent df1e1bc35c
commit c9a2bc284c
7 changed files with 41 additions and 19 deletions

View File

@ -1,3 +1,10 @@
* `ActiveSupport::ErrorReporter` now accepts and forward a `source:` parameter.
This allow libraries to signal the origin of the errors, and reporters
to easily ignore some sources.
*Jean Boussier*
* Fix and add protections for XSS in `ActionView::Helpers` and `ERB::Util`.
Add the method `ERB::Util.xml_name_escape` to escape dangerous characters

View File

@ -300,8 +300,12 @@ module ActiveSupport
def rescue_error_with(fallback)
yield
rescue Dalli::DalliError => error
ActiveSupport.error_reporter&.report(error, handled: true, severity: :warning)
logger.error("DalliError (#{error}): #{error.message}") if logger
ActiveSupport.error_reporter&.report(
error,
severity: :warning,
source: "mem_cache_store.active_support",
)
fallback
end
end

View File

@ -60,6 +60,11 @@ module ActiveSupport
if logger
logger.error { "RedisCacheStore: #{method} failed, returned #{returning.inspect}: #{exception.class}: #{exception.message}" }
end
ActiveSupport.error_reporter&.report(
exception,
severity: :warning,
source: "redis_cache_store.active_support",
)
end
# The maximum number of entries to receive per SCAN call.
@ -460,7 +465,6 @@ module ActiveSupport
def failsafe(method, returning: nil)
yield
rescue ::Redis::BaseError => error
ActiveSupport.error_reporter&.report(error, handled: true, severity: :warning)
@error_handler&.call(method: method, exception: error, returning: returning)
returning
end

View File

@ -40,6 +40,7 @@ module ActiveSupport
# end
class ErrorReporter
SEVERITIES = %i(error warning info)
DEFAULT_SOURCE = "application"
attr_accessor :logger
@ -54,17 +55,17 @@ module ActiveSupport
# 1 + '1'
# end
#
def handle(error_class = StandardError, severity: :warning, context: {}, fallback: nil)
def handle(error_class = StandardError, severity: :warning, context: {}, fallback: nil, source: DEFAULT_SOURCE)
yield
rescue error_class => error
report(error, handled: true, severity: severity, context: context)
report(error, handled: true, severity: severity, context: context, source: source)
fallback.call if fallback
end
def record(error_class = StandardError, severity: :error, context: {})
def record(error_class = StandardError, severity: :error, context: {}, source: DEFAULT_SOURCE)
yield
rescue error_class => error
report(error, handled: false, severity: severity, context: context)
report(error, handled: false, severity: severity, context: context, source: source)
raise
end
@ -107,7 +108,7 @@ module ActiveSupport
# When the block based +handle+ and +record+ methods are not suitable, you can directly use +report+
#
# Rails.error.report(error)
def report(error, handled: true, severity: handled ? :warning : :error, context: {})
def report(error, handled: true, severity: handled ? :warning : :error, context: {}, source: DEFAULT_SOURCE)
unless SEVERITIES.include?(severity)
raise ArgumentError, "severity must be one of #{SEVERITIES.map(&:inspect).join(", ")}, got: #{severity.inspect}"
end
@ -116,7 +117,7 @@ module ActiveSupport
disabled_subscribers = ActiveSupport::IsolatedExecutionState[self]
@subscribers.each do |subscriber|
unless disabled_subscribers&.any? { |s| s === subscriber }
subscriber.report(error, handled: handled, severity: severity, context: full_context)
subscriber.report(error, handled: handled, severity: severity, context: full_context, source: source)
end
rescue => subscriber_error
if logger

View File

@ -91,7 +91,7 @@ module ActiveSupport
begin
yield
rescue => error
error_reporter.report(error, handled: false)
error_reporter.report(error, handled: false, source: "unhandled_error.active_support")
raise
ensure
instance.complete!

View File

@ -15,8 +15,8 @@ class ErrorReporterTest < ActiveSupport::TestCase
@events = []
end
def report(error, handled:, severity:, context:)
@events << [error, handled, severity, context]
def report(error, handled:, severity:, source:, context:)
@events << [error, handled, severity, source, context]
end
end
@ -31,14 +31,20 @@ class ErrorReporterTest < ActiveSupport::TestCase
@reporter.set_context(section: "admin")
error = ArgumentError.new("Oops")
@reporter.report(error, handled: true)
assert_equal [[error, true, :warning, { section: "admin" }]], @subscriber.events
assert_equal [[error, true, :warning, "application", { section: "admin" }]], @subscriber.events
end
test "passed context has priority over the execution context" do
@reporter.set_context(section: "admin")
error = ArgumentError.new("Oops")
@reporter.report(error, handled: true, context: { section: "public" })
assert_equal [[error, true, :warning, { section: "public" }]], @subscriber.events
assert_equal [[error, true, :warning, "application", { section: "public" }]], @subscriber.events
end
test "passed source is forwarded" do
error = ArgumentError.new("Oops")
@reporter.report(error, handled: true, source: "my_gem")
assert_equal [[error, true, :warning, "my_gem", {}]], @subscriber.events
end
test "#disable allow to skip a subscriber" do
@ -60,7 +66,7 @@ class ErrorReporterTest < ActiveSupport::TestCase
@reporter.handle do
raise error
end
assert_equal [[error, true, :warning, {}]], @subscriber.events
assert_equal [[error, true, :warning, "application", {}]], @subscriber.events
end
test "#handle can be scoped to an exception class" do
@ -117,7 +123,7 @@ class ErrorReporterTest < ActiveSupport::TestCase
raise error
end
end
assert_equal [[error, false, :error, {}]], @subscriber.events
assert_equal [[error, false, :error, "application", {}]], @subscriber.events
end
test "#record can be scoped to an exception class" do
@ -164,7 +170,7 @@ class ErrorReporterTest < ActiveSupport::TestCase
@error = error
end
def report(_error, handled:, severity:, context:)
def report(_error, handled:, severity:, context:, source:)
raise @error
end
end

View File

@ -13,8 +13,8 @@ class ExecutorTest < ActiveSupport::TestCase
@events = []
end
def report(error, handled:, severity:, context:)
@events << [error, handled, severity, context]
def report(error, handled:, severity:, source:, context:)
@events << [error, handled, severity, source, context]
end
end
@ -27,7 +27,7 @@ class ExecutorTest < ActiveSupport::TestCase
raise error
end
end
assert_equal [[error, false, :error, {}]], subscriber.events
assert_equal [[error, false, :error, "unhandled_error.active_support", {}]], subscriber.events
end
def test_wrap_invokes_callbacks