Merge pull request #664 from bugsnag/fix-notify

Deliver even when an error raised in the block argument

CHANGELOG.md

@@ -1,6 +1,14 @@
 Changelog
 =========
 
+## TBD
+
+### Fixes
+
+* Deliver when an error is raised in the block argument to `notify`
+  | [#660](https://github.com/bugsnag/bugsnag-ruby/pull/660)
+  | [aki77](https://github.com/aki77)
+
 ## v6.20.0 (29 March 2021)
 
 ### Enhancements

lib/bugsnag.rb

@@ -79,7 +79,12 @@ def notify(exception, auto_notify=false, &block)
       report = Report.new(exception, configuration, auto_notify)
 
       # If this is an auto_notify we yield the block before the any middleware is run
-      yield(report) if block_given? && auto_notify
+      begin
+        yield(report) if block_given? && auto_notify
+      rescue StandardError => e
+        configuration.warn("Error in internal notify block: #{e}")
+        configuration.warn("Error in internal notify block stacktrace: #{e.backtrace.inspect}")
+      end
 
       if report.ignore?
         configuration.debug("Not notifying #{report.exceptions.last[:errorClass]} due to ignore being signified in auto_notify block")
@@ -106,7 +111,12 @@ def notify(exception, auto_notify=false, &block)
 
         # If this is not an auto_notify then the block was provided by the user. This should be the last
         # block that is run as it is the users "most specific" block.
-        yield(report) if block_given? && !auto_notify
+        begin
+          yield(report) if block_given? && !auto_notify
+        rescue StandardError => e
+          configuration.warn("Error in notify block: #{e}")
+          configuration.warn("Error in notify block stacktrace: #{e.backtrace.inspect}")
+        end
 
         if report.ignore?
           configuration.debug("Not notifying #{report.exceptions.last[:errorClass]} due to ignore being signified in user provided block")

spec/bugsnag_spec.rb

@@ -56,6 +56,46 @@
       })
       expect(breadcrumb.timestamp).to be_within(1).of(sent_time)
     end
+
+    it 'can deliver when an error raised in the block argument' do
+      Bugsnag.notify(RuntimeError.new('Manual notify notified even though it raised')) do |report|
+        raise 'This is the error message'
+      end
+
+      expected_messages = [
+        /^Error in notify block: This is the error message$/,
+        /^Error in notify block stacktrace: \[/
+      ].each
+
+      expect(Bugsnag.configuration.logger).to have_received(:warn).with('[Bugsnag]').twice do |&block|
+        expect(block.call).to match(expected_messages.next)
+      end
+
+      expect(Bugsnag).to have_sent_notification{ |payload, headers|
+        event = get_event_from_payload(payload)
+        expect(event['exceptions'].first['message']).to eq('Manual notify notified even though it raised')
+      }
+    end
+
+    it 'can deliver when an error raised in the block argument and auto_notify is true' do
+      Bugsnag.notify(RuntimeError.new('Auto notify notified even though it raised'), true) do |report|
+        raise 'This is an auto_notify error'
+      end
+
+      expected_messages = [
+        /^Error in internal notify block: This is an auto_notify error$/,
+        /^Error in internal notify block stacktrace: \[/
+      ].each
+
+      expect(Bugsnag.configuration.logger).to have_received(:warn).with('[Bugsnag]').twice do |&block|
+        expect(block.call).to match(expected_messages.next)
+      end
+
+      expect(Bugsnag).to have_sent_notification{ |payload, headers|
+        event = get_event_from_payload(payload)
+        expect(event['exceptions'].first['message']).to eq('Auto notify notified even though it raised')
+      }
+    end
   end
 
   describe '#configure' do