From 62fbe9536bf7be0de2070a091c649cba99467732 Mon Sep 17 00:00:00 2001 From: Oli Peate Date: Fri, 11 Jun 2021 13:54:45 +0100 Subject: [PATCH 1/6] Allow Methods to be used as callbacks / middleware Resolves https://github.com/bugsnag/bugsnag-ruby/issues/661 --- lib/bugsnag.rb | 2 +- lib/bugsnag/configuration.rb | 6 ++--- lib/bugsnag/middleware_stack.rb | 12 +++++----- spec/on_error_spec.rb | 39 +++++++++++++++++++++++++++++++++ 4 files changed, 49 insertions(+), 10 deletions(-) diff --git a/lib/bugsnag.rb b/lib/bugsnag.rb index 17736aa4d..7cd178bcd 100644 --- a/lib/bugsnag.rb +++ b/lib/bugsnag.rb @@ -265,7 +265,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) diff --git a/lib/bugsnag/configuration.rb b/lib/bugsnag/configuration.rb index 8cd8f0d17..ea8abc1ec 100644 --- a/lib/bugsnag/configuration.rb +++ b/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) diff --git a/lib/bugsnag/middleware_stack.rb b/lib/bugsnag/middleware_stack.rb index d9dcd95bc..3bca6f28a 100644 --- a/lib/bugsnag/middleware_stack.rb +++ b/lib/bugsnag/middleware_stack.rb @@ -131,8 +131,8 @@ def run(report) # # @return [Array] 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 @@ -140,12 +140,12 @@ def middleware_procs 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 diff --git a/spec/on_error_spec.rb b/spec/on_error_spec.rb index ab982944d..ce85c331f 100644 --- a/spec/on_error_spec.rb +++ b/spec/on_error_spec.rb @@ -329,4 +329,43 @@ end) end end + + describe "using methods as callbacks" do + it "runs callbacks on notify" do + def callback(report) + report.add_tab(:important, { hello: "world" }) + end + + Bugsnag.add_on_error(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) + ensure + undef :callback + end + + it "can remove an already registered callback" do + def callback(report) + report.add_tab(:important, { hello: "world" }) + end + + Bugsnag.add_on_error(method(:callback)) + Bugsnag.remove_on_error(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) + ensure + undef :callback + end + end end From a0f912c6c8cfcffc3ec3e647e02f4f11e39de420 Mon Sep 17 00:00:00 2001 From: Oli Peate Date: Fri, 11 Jun 2021 14:02:44 +0100 Subject: [PATCH 2/6] Update CHANGELOG --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ecefaee4..78f09368d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,13 @@ Changelog ========= +## Unreleased + +### Enhancements + +* Allow a `Method` or any object responding to `#call` to be used as a callback or middleware + | [#662](https://github.com/bugsnag/bugsnag-ruby/pull/662) + ## v6.20.0 (29 March 2021) ### Enhancements From e5d5e802404d6f838f9edbcf2d0f02ca6cc9c5e8 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 16 Jun 2021 15:22:44 +0100 Subject: [PATCH 3/6] Allow tests to run on Ruby 1.9 --- spec/on_error_spec.rb | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/spec/on_error_spec.rb b/spec/on_error_spec.rb index ce85c331f..b4aba355b 100644 --- a/spec/on_error_spec.rb +++ b/spec/on_error_spec.rb @@ -332,11 +332,13 @@ describe "using methods as callbacks" do it "runs callbacks on notify" do - def callback(report) - report.add_tab(:important, { hello: "world" }) + class OnErrorCallbackHaver + def callback(report) + report.add_tab(:important, { hello: "world" }) + end end - Bugsnag.add_on_error(method(:callback)) + Bugsnag.add_on_error(OnErrorCallbackHaver.new.method(:callback)) Bugsnag.notify(RuntimeError.new("Oh no!")) @@ -345,17 +347,19 @@ def callback(report) expect(event["metaData"]["important"]).to eq({ "hello" => "world" }) end) - ensure - undef :callback end it "can remove an already registered callback" do - def callback(report) - report.add_tab(:important, { hello: "world" }) + class OnErrorCallbackHaver + def callback(report) + report.add_tab(:important, { hello: "world" }) + end end - Bugsnag.add_on_error(method(:callback)) - Bugsnag.remove_on_error(method(:callback)) + instance = OnErrorCallbackHaver.new + + Bugsnag.add_on_error(instance.method(:callback)) + Bugsnag.remove_on_error(instance.method(:callback)) Bugsnag.notify(RuntimeError.new("Oh no!")) @@ -364,8 +368,6 @@ def callback(report) expect(event["metaData"]["important"]).to be_nil end) - ensure - undef :callback end end end From e010f0bc7c49850cf3d3c98e2b9388c554c8c5bb Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 16 Jun 2021 15:23:26 +0100 Subject: [PATCH 4/6] Add attribution to changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 78f09368d..10b6d7d2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,12 +1,13 @@ Changelog ========= -## Unreleased +## TBC ### Enhancements * Allow a `Method` or any object responding to `#call` to be used as a callback or middleware | [#662](https://github.com/bugsnag/bugsnag-ruby/pull/662) + | [odlp](https://github.com/odlp) ## v6.20.0 (29 March 2021) From 48dccc6223ba1fd4a247f6b9a89962a4939dfd1f Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 16 Jun 2021 15:24:46 +0100 Subject: [PATCH 5/6] Specifically mention `on_error` callbacks --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 10b6d7d2d..572ca9bbf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ Changelog ### Enhancements -* Allow a `Method` or any object responding to `#call` to be used as a callback or middleware +* 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) From b5ee27e8e0c36a2b602c49ae4afb3d78fb5e481c Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 16 Jun 2021 15:30:32 +0100 Subject: [PATCH 6/6] Add a test for #call as a callback --- spec/on_error_spec.rb | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/spec/on_error_spec.rb b/spec/on_error_spec.rb index b4aba355b..15bcdb5aa 100644 --- a/spec/on_error_spec.rb +++ b/spec/on_error_spec.rb @@ -370,4 +370,45 @@ def callback(report) 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