Release v6.21.0

.buildkite/pipeline.yml

@@ -1149,3 +1149,9 @@ steps:
       RACK_VERSION: '2'
     concurrency: 4
     concurrency_group: 'ruby/integrations-maze-runner-tests'
+
+  - name: ':copyright: License Audit'
+    plugins:
+      docker-compose#v3.7.0:
+        run: license_finder
+    command: /bin/bash -lc '/scan/scripts/license_finder.sh'

CHANGELOG.md

@@ -1,6 +1,22 @@
 Changelog
 =========
 
+## v6.21.0 (23 June 2021)
+
+### Enhancements
+
+* Allow a `Method` or any object responding to `#call` to be used as an `on_error` callback or middleware
+  | [#662](https://github.com/bugsnag/bugsnag-ruby/pull/662)
+  | [odlp](https://github.com/odlp)
+
+### 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)
+* Fix potential `NoMethodError` in `Bugsnag::Railtie` when using `require: false` in a Gemfile
+  | [#666](https://github.com/bugsnag/bugsnag-ruby/pull/666)
+
 ## v6.20.0 (29 March 2021)
 
 ### Enhancements

Gemfile

@@ -34,6 +34,8 @@ group :test, optional: true do
 
   # WEBrick is no longer in the stdlib in Ruby 3.0
   gem 'webrick' if ruby_version >= Gem::Version.new('3.0.0')
+
+  gem 'rexml', '< 3.2.5' if ruby_version == Gem::Version.new('2.0.0')
 end
 
 group :coverage, optional: true do

VERSION

@@ -1 +1 @@
-6.20.0
+6.21.0

config/.gitignore

@@ -0,0 +1 @@
+decisions.yml

docker-compose.yml

@@ -1,5 +1,13 @@
 version: '3.6'
 services:
+
+  license_finder:
+    build:
+      dockerfile: dockerfiles/Dockerfile.audit
+      context: .
+    volumes:
+      - ./:/scan
+
   ruby-maze-runner:
     build:
       context: .

dockerfiles/Dockerfile.audit

@@ -0,0 +1,5 @@
+FROM licensefinder/license_finder
+
+WORKDIR /scan
+
+CMD /scan/scripts/license_finder.sh

features/fixtures/delayed_job/app/Gemfile

@@ -8,7 +8,7 @@ end
 gem 'bugsnag', path: '/bugsnag'
 gem 'delayed_job'
 gem 'delayed_job_active_record'
-gem 'therubyracer'
+gem 'mini_racer'
 
 # Bundle edge Rails instead: gem 'rails', github: 'rails/rails'
 gem 'rails', '~> 5.0.7'
@@ -22,8 +22,6 @@ gem 'sass-rails', '~> 5.0'
 gem 'uglifier', '>= 1.3.0'
 # Use CoffeeScript for .coffee assets and views
 gem 'coffee-rails', '~> 4.2'
-# See https://github.com/rails/execjs#readme for more supported runtimes
-# gem 'therubyracer', platforms: :ruby
 
 # Use jquery as the JavaScript library
 gem 'jquery-rails'

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")
@@ -265,7 +275,7 @@ def leave_breadcrumb(name, meta_data={}, type=Bugsnag::Breadcrumbs::MANUAL_BREAD
     # Returning false from an on_error callback will cause the error to be ignored
     # and will prevent any remaining callbacks from being called
     #
-    # @param callback [Proc]
+    # @param callback [Proc, Method, #call]
     # @return [void]
     def add_on_error(callback)
       configuration.add_on_error(callback)

lib/bugsnag/configuration.rb

@@ -485,7 +485,7 @@ def disable_sessions
     # Returning false from an on_error callback will cause the error to be ignored
     # and will prevent any remaining callbacks from being called
     #
-    # @param callback [Proc]
+    # @param callback [Proc, Method, #call]
     # @return [void]
     def add_on_error(callback)
       middleware.use(callback)
@@ -494,10 +494,10 @@ def add_on_error(callback)
     ##
     # Remove the given callback from the list of on_error callbacks
     #
-    # Note that this must be the same Proc instance that was passed to
+    # Note that this must be the same instance that was passed to
     # {#add_on_error}, otherwise it will not be removed
     #
-    # @param callback [Proc]
+    # @param callback [Proc, Method, #call]
     # @return [void]
     def remove_on_error(callback)
       middleware.remove(callback)

lib/bugsnag/integrations/railtie.rb

@@ -9,11 +9,44 @@
 
 module Bugsnag
   class Railtie < ::Rails::Railtie
-
     FRAMEWORK_ATTRIBUTES = {
       :framework => "Rails"
     }
 
+    ##
+    # Subscribes to an ActiveSupport event, leaving a breadcrumb when it triggers
+    #
+    # @api private
+    # @param event [Hash] details of the event to subscribe to
+    def event_subscription(event)
+      ActiveSupport::Notifications.subscribe(event[:id]) do |*, event_id, data|
+        filtered_data = data.slice(*event[:allowed_data])
+        filtered_data[:event_name] = event[:id]
+        filtered_data[:event_id] = event_id
+
+        if event[:id] == "sql.active_record"
+          if data.key?(:binds)
+            binds = data[:binds].each_with_object({}) { |bind, output| output[bind.name] = '?' if defined?(bind.name) }
+            filtered_data[:binds] = JSON.dump(binds) unless binds.empty?
+          end
+
+          # Rails < 6.1 included connection_id in the event data, but now
+          # includes the connection object instead
+          if data.key?(:connection) && !data.key?(:connection_id)
+            # the connection ID is the object_id of the connection object
+            filtered_data[:connection_id] = data[:connection].object_id
+          end
+        end
+
+        Bugsnag.leave_breadcrumb(
+          event[:message],
+          filtered_data,
+          event[:type],
+          :auto
+        )
+      end
+    end
+
     rake_tasks do
       require "bugsnag/integrations/rake"
       load "bugsnag/tasks/bugsnag.rake"
@@ -80,39 +113,5 @@ class Railtie < ::Rails::Railtie
         Bugsnag.configuration.warn("Unable to add Bugsnag::Rack middleware as the middleware stack is frozen")
       end
     end
-
-    ##
-    # Subscribes to an ActiveSupport event, leaving a breadcrumb when it triggers
-    #
-    # @api private
-    # @param event [Hash] details of the event to subscribe to
-    def event_subscription(event)
-      ActiveSupport::Notifications.subscribe(event[:id]) do |*, event_id, data|
-        filtered_data = data.slice(*event[:allowed_data])
-        filtered_data[:event_name] = event[:id]
-        filtered_data[:event_id] = event_id
-
-        if event[:id] == "sql.active_record"
-          if data.key?(:binds)
-            binds = data[:binds].each_with_object({}) { |bind, output| output[bind.name] = '?' if defined?(bind.name) }
-            filtered_data[:binds] = JSON.dump(binds) unless binds.empty?
-          end
-
-          # Rails < 6.1 included connection_id in the event data, but now
-          # includes the connection object instead
-          if data.key?(:connection) && !data.key?(:connection_id)
-            # the connection ID is the object_id of the connection object
-            filtered_data[:connection_id] = data[:connection].object_id
-          end
-        end
-
-        Bugsnag.leave_breadcrumb(
-          event[:message],
-          filtered_data,
-          event[:type],
-          :auto
-        )
-      end
-    end
   end
 end

lib/bugsnag/middleware_stack.rb

@@ -131,21 +131,21 @@ def run(report)
     #
     # @return [Array<Proc>]
     def middleware_procs
-      # Split the middleware into separate lists of Procs and Classes
-      procs, classes = @middlewares.partition {|middleware| middleware.is_a?(Proc) }
+      # Split the middleware into separate lists of callables (e.g. Proc, Lambda, Method) and Classes
+      callables, classes = @middlewares.partition {|middleware| middleware.respond_to?(:call) }
 
       # Wrap the classes in a proc that, when called, news up the middleware and
       # passes the next middleware in the queue
       middleware_instances = classes.map do |middleware|
         proc {|next_middleware| middleware.new(next_middleware) }
       end
 
-      # Wrap the list of procs in a proc that, when called, wraps them in an
+      # Wrap the list of callables in a proc that, when called, wraps them in an
       # 'OnErrorCallbacks' instance that also has a reference to the next middleware
-      wrapped_procs = proc {|next_middleware| OnErrorCallbacks.new(next_middleware, procs) }
+      wrapped_callables = proc {|next_middleware| OnErrorCallbacks.new(next_middleware, callables) }
 
-      # Return the combined middleware and wrapped procs
-      middleware_instances.push(wrapped_procs)
+      # Return the combined middleware and wrapped callables
+      middleware_instances.push(wrapped_callables)
     end
   end
 end

scripts/license_finder.sh

@@ -0,0 +1,6 @@
+#!/bin/bash
+curl https://raw.githubusercontent.com/bugsnag/license-audit/master/config/decision_files/global.yml -o config/decisions.yml
+curl https://raw.githubusercontent.com/bugsnag/license-audit/master/config/decision_files/bugsnag-ruby.yml >> config/decisions.yml
+
+bundle install
+license_finder --decisions-file=config/decisions.yml

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

spec/on_error_spec.rb

@@ -329,4 +329,86 @@
       end)
     end
   end
+
+  describe "using methods as callbacks" do
+    it "runs callbacks on notify" do
+      class OnErrorCallbackHaver
+        def callback(report)
+          report.add_tab(:important, { hello: "world" })
+        end
+      end
+
+      Bugsnag.add_on_error(OnErrorCallbackHaver.new.method(:callback))
+
+      Bugsnag.notify(RuntimeError.new("Oh no!"))
+
+      expect(Bugsnag).to(have_sent_notification do |payload, _headers|
+        event = get_event_from_payload(payload)
+
+        expect(event["metaData"]["important"]).to eq({ "hello" => "world" })
+      end)
+    end
+
+    it "can remove an already registered callback" do
+      class OnErrorCallbackHaver
+        def callback(report)
+          report.add_tab(:important, { hello: "world" })
+        end
+      end
+
+      instance = OnErrorCallbackHaver.new
+
+      Bugsnag.add_on_error(instance.method(:callback))
+      Bugsnag.remove_on_error(instance.method(:callback))
+
+      Bugsnag.notify(RuntimeError.new("Oh no!"))
+
+      expect(Bugsnag).to(have_sent_notification do |payload, _headers|
+        event = get_event_from_payload(payload)
+
+        expect(event["metaData"]["important"]).to be_nil
+      end)
+    end
+  end
+
+  describe "using an object that responds to #call" do
+    it "runs callbacks on notify" do
+      class RespondsToCall
+        def call(report)
+          report.add_tab(:important, { hi: "earth" })
+        end
+      end
+
+      Bugsnag.add_on_error(RespondsToCall.new)
+
+      Bugsnag.notify(RuntimeError.new("Oh no!"))
+
+      expect(Bugsnag).to(have_sent_notification do |payload, _headers|
+        event = get_event_from_payload(payload)
+
+        expect(event["metaData"]["important"]).to eq({ "hi" => "earth" })
+      end)
+    end
+
+    it "can remove an already registered callback" do
+      class RespondsToCall
+        def callback(report)
+          report.add_tab(:important, { hi: "earth" })
+        end
+      end
+
+      instance = RespondsToCall.new
+
+      Bugsnag.add_on_error(instance)
+      Bugsnag.remove_on_error(instance)
+
+      Bugsnag.notify(RuntimeError.new("Oh no!"))
+
+      expect(Bugsnag).to(have_sent_notification do |payload, _headers|
+        event = get_event_from_payload(payload)
+
+        expect(event["metaData"]["important"]).to be_nil
+      end)
+    end
+  end
 end