Skip to content

Commit 1d55f54

Browse files
authored
Merge pull request rails#43581 from Shopify/as-notification-raise-single
Only wrap subscriber exceptions if there is more than one
2 parents cdef2f6 + acd462f commit 1d55f54

File tree

4 files changed

+27
-14
lines changed

4 files changed

+27
-14
lines changed

activerecord/test/cases/adapter_test.rb

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -213,11 +213,9 @@ def test_numeric_value_out_of_ranges_are_translated_to_specific_exception
213213
def test_exceptions_from_notifications_are_not_translated
214214
original_error = StandardError.new("This StandardError shouldn't get translated")
215215
subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") { raise original_error }
216-
wrapped_error = assert_raises(ActiveSupport::Notifications::InstrumentationSubscriberError) do
216+
actual_error = assert_raises(StandardError) do
217217
@connection.execute("SELECT * FROM posts")
218218
end
219-
actual_error = wrapped_error.exceptions.first
220-
221219
assert_equal original_error, actual_error
222220

223221
ensure

activerecord/test/cases/query_cache_test.rb

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -489,13 +489,11 @@ def test_query_cache_does_not_allow_sql_key_mutation
489489
payload[:sql].downcase!
490490
end
491491

492-
error = assert_raises ActiveSupport::Notifications::InstrumentationSubscriberError do
492+
assert_raises FrozenError do
493493
ActiveRecord::Base.cache do
494494
assert_queries(1) { Task.find(1); Task.find(1) }
495495
end
496496
end
497-
498-
assert error.exceptions.first.is_a?(FrozenError)
499497
ensure
500498
ActiveSupport::Notifications.unsubscribe subscriber
501499
end

activesupport/lib/active_support/notifications/fanout.rb

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,16 @@ def iterate_guarding_exceptions(listeners)
9393
exceptions ||= []
9494
exceptions << e
9595
end
96-
ensure
97-
raise InstrumentationSubscriberError.new(exceptions) unless exceptions.nil?
96+
97+
if exceptions
98+
if exceptions.size == 1
99+
raise exceptions.first
100+
else
101+
raise InstrumentationSubscriberError.new(exceptions), cause: exceptions.first
102+
end
103+
end
104+
105+
listeners
98106
end
99107

100108
def listeners_for(name)

activesupport/test/notifications/evented_notification_test.rb

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -92,18 +92,23 @@ def test_listen_to_everything
9292
], listener.events
9393
end
9494

95-
def test_listen_start_exception_consistency
95+
def test_listen_start_multiple_exception_consistency
9696
notifier = Fanout.new
9797
listener = Listener.new
9898
notifier.subscribe nil, BadStartListener.new
99+
notifier.subscribe nil, BadStartListener.new
99100
notifier.subscribe nil, listener
100101

101-
assert_raises InstrumentationSubscriberError do
102+
error = assert_raises InstrumentationSubscriberError do
102103
notifier.start "hello", 1, {}
103104
end
104-
assert_raises InstrumentationSubscriberError do
105+
assert_instance_of BadListenerException, error.cause
106+
107+
error = assert_raises InstrumentationSubscriberError do
105108
notifier.start "world", 1, {}
106109
end
110+
assert_instance_of BadListenerException, error.cause
111+
107112
notifier.finish "world", 1, {}
108113
notifier.finish "hello", 1, {}
109114

@@ -116,20 +121,24 @@ def test_listen_start_exception_consistency
116121
], listener.events
117122
end
118123

119-
def test_listen_finish_exception_consistency
124+
def test_listen_finish_multiple_exception_consistency
120125
notifier = Fanout.new
121126
listener = Listener.new
122127
notifier.subscribe nil, BadFinishListener.new
128+
notifier.subscribe nil, BadFinishListener.new
123129
notifier.subscribe nil, listener
124130

125131
notifier.start "hello", 1, {}
126132
notifier.start "world", 1, {}
127-
assert_raises InstrumentationSubscriberError do
133+
error = assert_raises InstrumentationSubscriberError do
128134
notifier.finish "world", 1, {}
129135
end
130-
assert_raises InstrumentationSubscriberError do
136+
assert_instance_of BadListenerException, error.cause
137+
138+
error = assert_raises InstrumentationSubscriberError do
131139
notifier.finish "hello", 1, {}
132140
end
141+
assert_instance_of BadListenerException, error.cause
133142

134143
assert_equal 4, listener.events.length
135144
assert_equal [

0 commit comments

Comments
 (0)