Merge pull request rails#49951 from Shopify/error-reporter-debug

`ErrorReporter#unexpected` to report in production but raise in development

Author: byroot

activesupport/CHANGELOG.md

@@ -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*

activesupport/lib/active_support/error_reporter.rb

@@ -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 @@ def initialize(*subscribers, logger: nil)
     #   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 @@ 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 <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)

activesupport/test/error_reporter_test.rb

@@ -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)

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