From f5c25964175413e14f5125fb8d9a85b861533ed7 Mon Sep 17 00:00:00 2001
From: Paul Sadauskas <psadauskas@gmail.com>
Date: Fri, 29 Nov 2019 23:21:07 -0700
Subject: [PATCH] Added ability to instrument when exceptions happen

Occasionally, the instrumented code will raise an exception, and we
would still want the subscribers to be notified. For example, if an HTTP
client fails by raising an exception, a subscriber doing logging should
still be able to log the failed request.

```
instrument("example.request") do
  client.post("http://url.that.fails.example/")
end
```

This will call the subscriber with an event that contains the exception
within the payload under a key `:exception`. This implementation also
re-raises the original error, so the callstack is unchanged.
---
 lib/dry/monitor/notifications.rb         |  7 ++++++-
 spec/integration/instrumentation_spec.rb | 23 +++++++++++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/lib/dry/monitor/notifications.rb b/lib/dry/monitor/notifications.rb
index b99506a..f26fc6c 100644
--- a/lib/dry/monitor/notifications.rb
+++ b/lib/dry/monitor/notifications.rb
@@ -41,7 +41,12 @@ def stop(event_id, payload)
 
       def instrument(event_id, payload = EMPTY_HASH)
         result, time = @clock.measure { yield payload } if block_given?
-
+      # We should always try to instrument, even the system-level exceptions
+      rescue Exception => e # rubocop:disable Lint/RescueException
+        payload = {} if payload.equal?(EMPTY_HASH)
+        payload[:exception] = e
+        raise
+      ensure
         process(event_id, payload) do |event, listener|
           if time
             listener.(event.payload(payload.merge(time: time)))
diff --git a/spec/integration/instrumentation_spec.rb b/spec/integration/instrumentation_spec.rb
index 26d4500..3b1698c 100644
--- a/spec/integration/instrumentation_spec.rb
+++ b/spec/integration/instrumentation_spec.rb
@@ -63,5 +63,28 @@
         outside_block: true, inside_block: true
       )
     end
+
+    MyError = Class.new(StandardError)
+    it 'still notifies when the instrumented block raises an exception' do
+      captured = []
+
+      notifications.subscribe(:sql) do |event|
+        captured << event
+      end
+
+      expect {
+        notifications.instrument(:sql) do
+          @line = __LINE__ + 1
+          raise MyError
+        end
+      }.to raise_error(MyError)
+
+      expect(captured).to_not be_empty
+      exception = captured.first.payload[:exception]
+      expect(exception).to match kind_of(MyError)
+      # verify the exception backtrace comes from the instrumented code, not
+      # the `#instrument` method
+      expect(exception.backtrace.first).to start_with("#{__FILE__}:#{@line}")
+    end
   end
 end