From 36bca21c6ec9f8364e1151aa4c118daaa2820331 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Tue, 7 Nov 2023 13:56:01 +0100 Subject: [PATCH] ErrorReporter#debug to swallow in production but raise in development It's a common useful pattern for situation where something isn't supposed to fail, but if it does we can recover from it. So in such situation you don't want such failure to be swallowed 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 `handle` in production. Fix: https://github.com/rails/rails/pull/49638 Fix: https://github.com/rails/rails/pull/49339 Co-Authored-By: Andrew Novoselac Co-Authored-By: Dustin Brown --- .../lib/active_support/error_reporter.rb | 26 ++++++++++++++++--- activesupport/test/error_reporter_test.rb | 17 ++++++++++++ railties/lib/rails/application/bootstrap.rb | 6 ++++- 3 files changed, 45 insertions(+), 4 deletions(-) diff --git a/activesupport/lib/active_support/error_reporter.rb b/activesupport/lib/active_support/error_reporter.rb index 71a94bffc2fdb..e42653e96df6e 100644 --- a/activesupport/lib/active_support/error_reporter.rb +++ b/activesupport/lib/active_support/error_reporter.rb @@ -26,12 +26,14 @@ module ActiveSupport class ErrorReporter SEVERITIES = %i(error warning info) DEFAULT_SOURCE = "application" + DEFAULT_RESCUE = [StandardError].freeze - attr_accessor :logger + attr_accessor :logger, :debug_mode 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 +74,7 @@ def initialize(*subscribers, logger: nil) # source of the error. Subscribers can use this value to ignore certain # errors. Defaults to "application". 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 +110,31 @@ def handle(*error_classes, severity: :warning, context: {}, fallback: nil, sourc # source of the error. Subscribers can use this value to ignore certain # errors. Defaults to "application". 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 + # Evaluates the given block, reporting any unhandled error. + # The error is then either re-raised if +config.consider_all_requests_local == true+ + # or otherwise swallowed. + # + # In other words in behaves like +record+ in development and test, and like +handle+ + # in production. + def debug(*error_classes, severity: :warning, context: {}, fallback: nil, source: DEFAULT_SOURCE) + error_classes = DEFAULT_RESCUE if error_classes.empty? + yield + rescue *error_classes => error + report(error, handled: true, severity: severity, context: context, source: source) + if @debug_mode + raise + else + fallback.call if fallback + 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) diff --git a/activesupport/test/error_reporter_test.rb b/activesupport/test/error_reporter_test.rb index 07e9faa9d1052..fa6a01b01d445 100644 --- a/activesupport/test/error_reporter_test.rb +++ b/activesupport/test/error_reporter_test.rb @@ -168,6 +168,23 @@ class ErrorReporterTest < ActiveSupport::TestCase assert_equal 4, result end + test "#debug swallows errors by default" do + result = @reporter.debug(fallback: -> { 2 + 2 }) do + raise StandardError + end + assert_equal 4, result + end + + test "#debug re-raise errors in development and test" do + @reporter.debug_mode = true + error = assert_raises RumtimeError do + @reporter.debug(fallback: -> { nil }) do + raise "Oops" + end + end + assert_includes error.message, "Oops" + end + test "can have multiple subscribers" do second_subscriber = ErrorSubscriber.new @reporter.subscribe(second_subscriber) diff --git a/railties/lib/rails/application/bootstrap.rb b/railties/lib/rails/application/bootstrap.rb index 6b70edafc0ce2..83a8da92988f6 100644 --- a/railties/lib/rails/application/bootstrap.rb +++ b/railties/lib/rails/application/bootstrap.rb @@ -61,8 +61,12 @@ module Bootstrap 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