mirror of https://github.com/rails/rails
ErrorReporter#unexpected to report in production but raise in development
It's a common useful pattern for situation where something isn't supposed to happen, but if it does we can recover from it. So in such situation you don't want such issue to be hidden in development or test, as it's likely a bug, but do not want to fail a request if it happens in production. In other words, it behaves like `#record` in development and test environments, and like `raise` in production. Fix: https://github.com/rails/rails/pull/49638 Fix: https://github.com/rails/rails/pull/49339 Co-Authored-By: Andrew Novoselac <andrew.novoselac@shopify.com> Co-Authored-By: Dustin Brown <dbrown9@gmail.com>
This commit is contained in:
parent
c2a33059d8
commit
2f19782dce
|
@ -1,3 +1,21 @@
|
|||
* Add `ErrorReported#unexpected` to report precondition violations.
|
||||
|
||||
For example:
|
||||
|
||||
```ruby
|
||||
def edit
|
||||
if published?
|
||||
Rails.error.unexpected("[BUG] Attempting to edit a published article, that shouldn't be possible")
|
||||
return false
|
||||
end
|
||||
# ...
|
||||
end
|
||||
```
|
||||
|
||||
The above will raise an error in development and test, but only report the error in production.
|
||||
|
||||
*Jean Boussier*
|
||||
|
||||
* Make the order of read_multi and write_multi notifications for `Cache::Store#fetch_multi` operations match the order they are executed in.
|
||||
|
||||
*Adam Renberg Tamm*
|
||||
|
|
|
@ -26,12 +26,16 @@ module ActiveSupport
|
|||
class ErrorReporter
|
||||
SEVERITIES = %i(error warning info)
|
||||
DEFAULT_SOURCE = "application"
|
||||
DEFAULT_RESCUE = [StandardError].freeze
|
||||
|
||||
attr_accessor :logger
|
||||
attr_accessor :logger, :debug_mode
|
||||
|
||||
UnexpectedError = Class.new(Exception)
|
||||
|
||||
def initialize(*subscribers, logger: nil)
|
||||
@subscribers = subscribers.flatten
|
||||
@logger = logger
|
||||
@debug_mode = false
|
||||
end
|
||||
|
||||
# Evaluates the given block, reporting and swallowing any unhandled error.
|
||||
|
@ -72,7 +76,7 @@ module ActiveSupport
|
|||
# source of the error. Subscribers can use this value to ignore certain
|
||||
# errors. Defaults to <tt>"application"</tt>.
|
||||
def handle(*error_classes, severity: :warning, context: {}, fallback: nil, source: DEFAULT_SOURCE)
|
||||
error_classes = [StandardError] if error_classes.blank?
|
||||
error_classes = DEFAULT_RESCUE if error_classes.empty?
|
||||
yield
|
||||
rescue *error_classes => error
|
||||
report(error, handled: true, severity: severity, context: context, source: source)
|
||||
|
@ -108,13 +112,47 @@ module ActiveSupport
|
|||
# source of the error. Subscribers can use this value to ignore certain
|
||||
# errors. Defaults to <tt>"application"</tt>.
|
||||
def record(*error_classes, severity: :error, context: {}, source: DEFAULT_SOURCE)
|
||||
error_classes = [StandardError] if error_classes.blank?
|
||||
error_classes = DEFAULT_RESCUE if error_classes.empty?
|
||||
yield
|
||||
rescue *error_classes => error
|
||||
report(error, handled: false, severity: severity, context: context, source: source)
|
||||
raise
|
||||
end
|
||||
|
||||
# Either report the given error when in production, or raise it when in development or test.
|
||||
#
|
||||
# When called in production, after the error is reported, this method will return
|
||||
# nil and execution will continue.
|
||||
#
|
||||
# When called in development, the original error is wrapped in a different error class to ensure
|
||||
# it's not being rescued higher in the stack and will be surfaced to the developer.
|
||||
#
|
||||
# This method is intended for reporting violated assertions about preconditions, or similar
|
||||
# cases that can and should be gracefully handled in production, but that aren't supposed to happen.
|
||||
#
|
||||
# The error can be either an exception instance or a String.
|
||||
#
|
||||
# example:
|
||||
#
|
||||
# def edit
|
||||
# if published?
|
||||
# Rails.error.unexpected("[BUG] Attempting to edit a published article, that shouldn't be possible")
|
||||
# return false
|
||||
# end
|
||||
# # ...
|
||||
# end
|
||||
#
|
||||
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
|
||||
raise UnexpectedError, "#{error.class.name}: #{error.message}", error.backtrace, cause: error
|
||||
else
|
||||
report(error, handled: true, severity: severity, context: context, source: source)
|
||||
end
|
||||
end
|
||||
|
||||
# Register a new error subscriber. The subscriber must respond to
|
||||
#
|
||||
# report(Exception, handled: Boolean, severity: (:error OR :warning OR :info), context: Hash, source: String)
|
||||
|
|
|
@ -168,6 +168,38 @@ class ErrorReporterTest < ActiveSupport::TestCase
|
|||
assert_equal 4, result
|
||||
end
|
||||
|
||||
test "#unexpected swallows errors by default" do
|
||||
error = RuntimeError.new("Oops")
|
||||
assert_nil @reporter.unexpected(error)
|
||||
assert_equal [[error, true, :warning, "application", {}]], @subscriber.events
|
||||
assert_not_predicate error.backtrace, :empty?
|
||||
end
|
||||
|
||||
test "#unexpected accepts an error message" do
|
||||
assert_nil @reporter.unexpected("Oops")
|
||||
assert_equal 1, @subscriber.events.size
|
||||
|
||||
error, *event_details = @subscriber.events.first
|
||||
assert_equal [true, :warning, "application", {}], event_details
|
||||
|
||||
assert_equal "Oops", error.message
|
||||
assert_equal RuntimeError, error.class
|
||||
assert_not_predicate error.backtrace, :empty?
|
||||
end
|
||||
|
||||
test "#unexpected re-raise errors in development and test" do
|
||||
@reporter.debug_mode = true
|
||||
error = RuntimeError.new("Oops")
|
||||
raise_line = __LINE__ + 2
|
||||
raised_error = assert_raises ActiveSupport::ErrorReporter::UnexpectedError do
|
||||
@reporter.unexpected(error)
|
||||
end
|
||||
assert_includes raised_error.message, "RuntimeError: Oops"
|
||||
assert_not_nil raised_error.cause
|
||||
assert_same error, raised_error.cause
|
||||
assert_includes raised_error.backtrace.first, "#{__FILE__}:#{raise_line}"
|
||||
end
|
||||
|
||||
test "can have multiple subscribers" do
|
||||
second_subscriber = ErrorSubscriber.new
|
||||
@reporter.subscribe(second_subscriber)
|
||||
|
|
|
@ -61,8 +61,12 @@ module Rails
|
|||
broadcast_logger.formatter = Rails.logger.formatter
|
||||
Rails.logger = broadcast_logger
|
||||
end
|
||||
end
|
||||
|
||||
unless config.consider_all_requests_local
|
||||
initializer :initialize_error_reporter, group: :all do
|
||||
if config.consider_all_requests_local
|
||||
Rails.error.debug_mode = true
|
||||
else
|
||||
Rails.error.logger = Rails.logger
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue