Merge pull request #665 from bugsnag/allow-method-callbacks

Allow Methods to be used as callbacks/middleware

CHANGELOG.md

@@ -1,7 +1,13 @@
 Changelog
 =========
 
-## TBD
+## TBC
+
+### 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
 

lib/bugsnag.rb

@@ -275,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/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

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