From 9e4ff69eb569b26f922ee661a3a9df6626d347b7 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 658298d..64ec356 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 d13ab40..d097465 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