Make notifications go off even when an error is raised, so that we capture the underlying performance data [#4505 state:resolved]

This is important when trying to keep track of many layers of interrelated calls

i.e.:

ActiveRecord::Base.transaction do
  MyModel.find(1) #ActiveRecord::NotFound
end # should capture the full time until the error propagation

Signed-off-by: José Valim <jose.valim@gmail.com>

2 files changed, 9 insertions, 5 deletions

diff --git a/activesupport/lib/active_support/notifications/instrumenter.rb b/activesupport/lib/active_support/notifications/instrumenter.rb
index f3d877efe7..ef3fdd1843 100644
--- a/activesupport/lib/active_support/notifications/instrumenter.rb
+++ b/activesupport/lib/active_support/notifications/instrumenter.rb
@@ -15,9 +15,13 @@ module ActiveSupport
       # and publish it.
       def instrument(name, payload={})
         time = Time.now
-        result = yield(payload) if block_given?
-        @notifier.publish(name, time, Time.now, @id, payload)
-        result
+        begin
+          yield(payload) if block_given?
+        ensure
+          # Notify in an ensure block so that we can be certain end
+          # events get sent even if an error occurs in the passed-in block
+          @notifier.publish(name, time, Time.now, @id, payload)
+        end
       end
 
       private
diff --git a/activesupport/test/notifications_test.rb b/activesupport/test/notifications_test.rb
index c2e1c588f0..251380a0d5 100644
--- a/activesupport/test/notifications_test.rb
+++ b/activesupport/test/notifications_test.rb
@@ -168,7 +168,7 @@ module Notifications
       assert_equal Hash[:payload => "notifications"], @events.last.payload
     end
 
-    def test_instrument_does_not_publish_when_exception_is_raised
+    def test_instrument_publishes_when_exception_is_raised
       begin
         instrument(:awesome, :payload => "notifications") do
           raise "FAIL"
@@ -178,7 +178,7 @@ module Notifications
       end
 
       drain
-      assert_equal 0, @events.size
+      assert_equal 1, @events.size
     end
 
     def test_event_is_pushed_even_without_block