From c6886a1ab0d0823bb065c7cd647812d8c1cdf0c6 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Thu, 22 Sep 2022 16:52:28 +0200 Subject: [PATCH] ActiveSupport::Reloader should not report exception Since it always delegate to an actual executor that does report them already, this cause exceptions to be reported twice. Fix: https://github.com/rails/rails/issues/46100 --- activesupport/lib/active_support/execution_wrapper.rb | 4 ++-- activesupport/lib/active_support/reloader.rb | 9 ++++++++- activesupport/test/reloader_test.rb | 11 +++++++++++ 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/activesupport/lib/active_support/execution_wrapper.rb b/activesupport/lib/active_support/execution_wrapper.rb index efd281e5f19..1c33c2d1e55 100644 --- a/activesupport/lib/active_support/execution_wrapper.rb +++ b/activesupport/lib/active_support/execution_wrapper.rb @@ -91,7 +91,7 @@ module ActiveSupport begin yield rescue => error - error_reporter.report(error, handled: false, source: source) + error_reporter&.report(error, handled: false, source: source) raise ensure instance.complete! @@ -108,7 +108,7 @@ module ActiveSupport end end - def self.error_reporter + def self.error_reporter # :nodoc: ActiveSupport.error_reporter end diff --git a/activesupport/lib/active_support/reloader.rb b/activesupport/lib/active_support/reloader.rb index 3882c0b80a8..72c0b80714e 100644 --- a/activesupport/lib/active_support/reloader.rb +++ b/activesupport/lib/active_support/reloader.rb @@ -68,8 +68,15 @@ module ActiveSupport # Run the supplied block as a work unit, reloading code as needed def self.wrap(**kwargs) + return yield if active? + executor.wrap(**kwargs) do - super + instance = run! + begin + yield + ensure + instance.complete! + end end end diff --git a/activesupport/test/reloader_test.rb b/activesupport/test/reloader_test.rb index 6aaebe0b5a3..3b33b7d9efe 100644 --- a/activesupport/test/reloader_test.rb +++ b/activesupport/test/reloader_test.rb @@ -85,6 +85,17 @@ class ReloaderTest < ActiveSupport::TestCase assert_equal [:before_unload, :unload, :after_unload, :body], called end + def test_report_errors_once + reports = ErrorCollector.record do + assert_raises RuntimeError do + reloader.wrap do + raise "Oops" + end + end + end + assert_equal 1, reports.size + end + private def new_reloader(&check) Class.new(ActiveSupport::Reloader).tap do |r|