Merge pull request rails#43719 from kmcphillips/error-reporter-fallback-callable

Require the ErrorReporter#handle fallback to be a callable

Author: rafaelfranca

activesupport/lib/active_support/error_reporter.rb

@@ -11,7 +11,7 @@ module ActiveSupport
   #
   # If an error is raised, it will be reported and swallowed.
   #
-  # Alternatively if you want to report the error but not swallow it, you can use `record`
+  # Alternatively if you want to report the error but not swallow it, you can use +record+
   #
   #   Rails.error.record do
   #     do_something!
@@ -31,10 +31,11 @@ module ActiveSupport
   # +severity+ can be one of +:error+, +:warning+ or +:info+. Handled errors default to the +:warning+
   # severity, and unhandled ones to +error+.
   #
-  # Both `handle` and `record` pass through the return value from the block. In the special case of `handle` handling an
-  # error, a fallback value can be provided that will be returned:
+  # Both +handle+ and +record+ pass through the return value from the block. In the case of +handle+
+  # rescuing an error, a fallback can be provided. The fallback must be a callable whose result will
+  # be returned when the block raises and is handled:
   #
-  #   user = Rails.error.handle(fallback: User.anonymous) do
+  #   user = Rails.error.handle(fallback: -> { User.anonymous }) do
   #     User.find_by(params)
   #   end
   class ErrorReporter
@@ -57,7 +58,7 @@ def handle(error_class = StandardError, severity: :warning, context: {}, fallbac
       yield
     rescue error_class => error
       report(error, handled: true, severity: severity, context: context)
-      fallback
+      fallback.call if fallback
     end
 
     def record(error_class = StandardError, severity: :error, context: {})
@@ -90,7 +91,7 @@ def set_context(...)
 
     # When the block based +handle+ and +record+ methods are not suitable, you can directly use +report+
     #
-    #  Rails.error.report(error, handled: true)
+    #   Rails.error.report(error, handled: true)
     def report(error, handled:, severity: handled ? :warning : :error, context: {})
       unless SEVERITIES.include?(severity)
         raise ArgumentError, "severity must be one of #{SEVERITIES.map(&:inspect).join(", ")}, got: #{severity.inspect}"

activesupport/test/error_reporter_test.rb

@@ -73,13 +73,27 @@ def report(error, handled:, severity:, context:)
     assert_nil result
   end
 
-  test "#handle returns a fallback value on handled raise" do
-    expected = "four"
-    result = @reporter.handle(fallback: expected) do
+  test "#handle returns the value of the fallback as a proc on handled raise" do
+    result = @reporter.handle(fallback: -> { 2 + 2 }) do
       raise StandardError
-      2 + 2
     end
-    assert_equal expected, result
+    assert_equal 4, result
+  end
+
+  test "#handle raises if the fallback is not a callable" do
+    assert_raises NoMethodError do
+      @reporter.handle(fallback: "four") do
+        raise StandardError
+      end
+    end
+  end
+
+  test "#handle raises the error up if fallback is a proc that then also raises" do
+    assert_raises ArgumentError do
+      @reporter.handle(fallback: -> { raise ArgumentError }) do
+        raise StandardError
+      end
+    end
   end
 
   test "#record report any unhandled error and re-raise them" do