Merge pull request rails#46131 from Shopify/avoid-double-error-report

`Rails.error.report` now marks errors as reported to avoid reporting them twice

Author: byroot

activesupport/CHANGELOG.md

@@ -1,3 +1,12 @@
+*   `Rails.error.report` now marks errors as reported to avoid reporting them twice.
+
+    In some cases, users might want to report errors explicitly with some extra context
+    before letting it bubble up.
+
+    This also allows to safely catch and report errors outside of the execution context.
+
+    *Jean Boussier*
+
 *   Add `Rails.application.message_verifiers` as a central point to configure
     and create message verifiers for an application.
 

activesupport/lib/active_support/error_reporter.rb

@@ -166,6 +166,8 @@ def set_context(...)
     #   Rails.error.report(error)
     #
     def report(error, handled: true, severity: handled ? :warning : :error, context: {}, source: DEFAULT_SOURCE)
+      return if error.instance_variable_get(:@__rails_error_reported)
+
       unless SEVERITIES.include?(severity)
         raise ArgumentError, "severity must be one of #{SEVERITIES.map(&:inspect).join(", ")}, got: #{severity.inspect}"
       end
@@ -187,6 +189,10 @@ def report(error, handled: true, severity: handled ? :warning : :error, context:
         end
       end
 
+      unless error.frozen?
+        error.instance_variable_set(:@__rails_error_reported, true)
+      end
+
       nil
     end
   end

activesupport/test/error_reporter_test.rb

@@ -178,6 +178,24 @@ class ErrorReporterTest < ActiveSupport::TestCase
     assert_equal :error, @subscriber.events.dig(0, 2)
   end
 
+  test "report errors only once" do
+    assert_difference -> { @subscriber.events.size }, +1 do
+      @reporter.report(@error, handled: false)
+    end
+
+    assert_no_difference -> { @subscriber.events.size } do
+      3.times do
+        @reporter.report(@error, handled: false)
+      end
+    end
+  end
+
+  test "can report frozen exceptions" do
+    assert_difference -> { @subscriber.events.size }, +1 do
+      @reporter.report(@error.freeze, handled: false)
+    end
+  end
+
   class FailingErrorSubscriber
     Error = Class.new(StandardError)
 

activesupport/test/executor_test.rb

@@ -29,6 +29,7 @@ def test_wrap_report_errors
     end
     assert_equal [error, false, :error, "application.active_support", {}], subscriber.events.last
 
+    error = DummyError.new("Oops")
     assert_raises DummyError do
       executor.wrap(source: "custom") do
         raise error