From d0eeee03be6c16e9455fab7286e250944a2e52bb Mon Sep 17 00:00:00 2001
From: Kevin McPhillips <github@kevinmcphillips.ca>
Date: Wed, 24 Nov 2021 19:54:59 -0500
Subject: [PATCH] Require the ErrorReporter#handle fallback to be a callable

---
 .../lib/active_support/error_reporter.rb      | 13 +++++-----
 activesupport/test/error_reporter_test.rb     | 24 +++++++++++++++----
 2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/activesupport/lib/active_support/error_reporter.rb b/activesupport/lib/active_support/error_reporter.rb
index 189ce2ba24..43683c1171 100644
--- a/activesupport/lib/active_support/error_reporter.rb
+++ b/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 @@ module ActiveSupport
       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 @@ module ActiveSupport
 
     # 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}"
diff --git a/activesupport/test/error_reporter_test.rb b/activesupport/test/error_reporter_test.rb
index 1d1779ec74..50f605c772 100644
--- a/activesupport/test/error_reporter_test.rb
+++ b/activesupport/test/error_reporter_test.rb
@@ -73,13 +73,27 @@ class ErrorReporterTest < ActiveSupport::TestCase
     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