From 1ea826504c3cc5d65c6e54ae02b86838a29a79e8 Mon Sep 17 00:00:00 2001 From: Steve Kirkland Date: Wed, 14 Apr 2021 13:45:09 +0100 Subject: [PATCH 01/19] Add license checker to CI --- .buildkite/pipeline.yml | 6 ++++++ config/.gitignore | 1 + docker-compose.yml | 8 ++++++++ dockerfiles/Dockerfile.audit | 5 +++++ scripts/license_finder.sh | 6 ++++++ 5 files changed, 26 insertions(+) create mode 100644 config/.gitignore create mode 100644 dockerfiles/Dockerfile.audit create mode 100755 scripts/license_finder.sh diff --git a/.buildkite/pipeline.yml b/.buildkite/pipeline.yml index db036ca1f..44fc5be9d 100644 --- a/.buildkite/pipeline.yml +++ b/.buildkite/pipeline.yml @@ -11,6 +11,12 @@ steps: - ruby-maze-runner:855461928731.dkr.ecr.us-west-1.amazonaws.com/ruby:base-ruby${BRANCH_NAME} - ruby-maze-runner:855461928731.dkr.ecr.us-west-1.amazonaws.com/ruby:base-ruby-latest + - name: ':copyright: License Audit' + plugins: + docker-compose#v3.7.0: + run: license_finder + command: /bin/bash -lc '/scan/scripts/license_finder.sh' + - wait - label: ':ruby: Ruby 1.9 unit tests' diff --git a/config/.gitignore b/config/.gitignore new file mode 100644 index 000000000..2869470e4 --- /dev/null +++ b/config/.gitignore @@ -0,0 +1 @@ +decisions.yml diff --git a/docker-compose.yml b/docker-compose.yml index 7940cbcec..6de99701b 100644 --- a/docker-compose.yml +++ b/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: . diff --git a/dockerfiles/Dockerfile.audit b/dockerfiles/Dockerfile.audit new file mode 100644 index 000000000..5a19c1a96 --- /dev/null +++ b/dockerfiles/Dockerfile.audit @@ -0,0 +1,5 @@ +FROM licensefinder/license_finder + +WORKDIR /scan + +CMD /scan/scripts/license_finder.sh diff --git a/scripts/license_finder.sh b/scripts/license_finder.sh new file mode 100755 index 000000000..b33d68ba0 --- /dev/null +++ b/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 From 52c21263c2152b43ab429ccdeddb764b62095976 Mon Sep 17 00:00:00 2001 From: Steve Kirkland Date: Thu, 15 Apr 2021 11:52:12 +0100 Subject: [PATCH 02/19] Pin rexml for Ruby 2.0.0 --- Gemfile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Gemfile b/Gemfile index 188f9df79..5ae1a09c0 100644 --- a/Gemfile +++ b/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 From dec5803c41bbe91f8d3cd14b8c925929072a53e5 Mon Sep 17 00:00:00 2001 From: Steve Kirkland Date: Thu, 15 Apr 2021 12:29:19 +0100 Subject: [PATCH 03/19] Move license audit to end of pipeline --- .buildkite/pipeline.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.buildkite/pipeline.yml b/.buildkite/pipeline.yml index 44fc5be9d..ccabb9449 100644 --- a/.buildkite/pipeline.yml +++ b/.buildkite/pipeline.yml @@ -11,12 +11,6 @@ steps: - ruby-maze-runner:855461928731.dkr.ecr.us-west-1.amazonaws.com/ruby:base-ruby${BRANCH_NAME} - ruby-maze-runner:855461928731.dkr.ecr.us-west-1.amazonaws.com/ruby:base-ruby-latest - - name: ':copyright: License Audit' - plugins: - docker-compose#v3.7.0: - run: license_finder - command: /bin/bash -lc '/scan/scripts/license_finder.sh' - - wait - label: ':ruby: Ruby 1.9 unit tests' @@ -1155,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' From 21ddd99bf38c733b8f9eaff5a06e59a00b0decbb Mon Sep 17 00:00:00 2001 From: aki Date: Thu, 3 Jun 2021 07:39:44 +0900 Subject: [PATCH 04/19] Change to deliver even when an error raised in the block argument --- lib/bugsnag.rb | 6 +++++- spec/bugsnag_spec.rb | 10 ++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/lib/bugsnag.rb b/lib/bugsnag.rb index 17736aa4d..6da2ca698 100644 --- a/lib/bugsnag.rb +++ b/lib/bugsnag.rb @@ -106,7 +106,11 @@ 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 => e + report.add_tab(:report_error, "Failed to report block (#{e.class}): #{e.message} \n#{e.backtrace[0..9].join("\n")}") + end if report.ignore? configuration.debug("Not notifying #{report.exceptions.last[:errorClass]} due to ignore being signified in user provided block") diff --git a/spec/bugsnag_spec.rb b/spec/bugsnag_spec.rb index 5d2e1aa5e..1898f3a1f 100644 --- a/spec/bugsnag_spec.rb +++ b/spec/bugsnag_spec.rb @@ -56,6 +56,16 @@ }) expect(breadcrumb.timestamp).to be_within(1).of(sent_time) end + + it 'is also be delivered when an error raised in the block argument' do + Bugsnag.notify('It crashed') do |report| + repor.context = 'test' + end + expect(Bugsnag).to have_sent_notification{ |payload, headers| + event = get_event_from_payload(payload) + expect(event['metaData']['custom']['report_error']).to match(/Failed to report block \(NameError\): undefined local variable or method/) + } + end end describe '#configure' do From 62fbe9536bf7be0de2070a091c649cba99467732 Mon Sep 17 00:00:00 2001 From: Oli Peate Date: Fri, 11 Jun 2021 13:54:45 +0100 Subject: [PATCH 05/19] 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 06/19] 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 1454b0b9878491123e81c871fd2f3378516e0087 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Tue, 15 Jun 2021 16:38:14 +0100 Subject: [PATCH 07/19] Log an error message instead of adding metadata This is consistent with how we report errors in other places, e.g. middleware and on_error callbacks --- lib/bugsnag.rb | 3 ++- spec/bugsnag_spec.rb | 18 ++++++++++++++---- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/lib/bugsnag.rb b/lib/bugsnag.rb index 6da2ca698..7632bbdf8 100644 --- a/lib/bugsnag.rb +++ b/lib/bugsnag.rb @@ -109,7 +109,8 @@ def notify(exception, auto_notify=false, &block) begin yield(report) if block_given? && !auto_notify rescue => e - report.add_tab(:report_error, "Failed to report block (#{e.class}): #{e.message} \n#{e.backtrace[0..9].join("\n")}") + configuration.warn("Error in notify block: #{e}") + configuration.warn("Error in notify block stacktrace: #{e.backtrace.inspect}") end if report.ignore? diff --git a/spec/bugsnag_spec.rb b/spec/bugsnag_spec.rb index 1898f3a1f..792b26f80 100644 --- a/spec/bugsnag_spec.rb +++ b/spec/bugsnag_spec.rb @@ -57,13 +57,23 @@ expect(breadcrumb.timestamp).to be_within(1).of(sent_time) end - it 'is also be delivered when an error raised in the block argument' do - Bugsnag.notify('It crashed') do |report| - repor.context = 'test' + 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['metaData']['custom']['report_error']).to match(/Failed to report block \(NameError\): undefined local variable or method/) + expect(event['exceptions'].first['message']).to eq('Manual notify notified even though it raised') } end end From caab4cb00fcc4ffa476f8b1216078b91395b3242 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Tue, 15 Jun 2021 16:39:05 +0100 Subject: [PATCH 08/19] Handle errors in auto_notify blocks --- lib/bugsnag.rb | 7 ++++++- spec/bugsnag_spec.rb | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/lib/bugsnag.rb b/lib/bugsnag.rb index 7632bbdf8..ebc3f5e56 100644 --- a/lib/bugsnag.rb +++ b/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 => 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") diff --git a/spec/bugsnag_spec.rb b/spec/bugsnag_spec.rb index 792b26f80..15a16065a 100644 --- a/spec/bugsnag_spec.rb +++ b/spec/bugsnag_spec.rb @@ -76,6 +76,26 @@ 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 From 6d0f9216b7e0efe593ff9c9bce4ad8b010fc24c1 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Tue, 15 Jun 2021 16:57:19 +0100 Subject: [PATCH 09/19] Explicitly resuce StandardError to fix Rubocop --- lib/bugsnag.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/bugsnag.rb b/lib/bugsnag.rb index ebc3f5e56..6fd810741 100644 --- a/lib/bugsnag.rb +++ b/lib/bugsnag.rb @@ -81,7 +81,7 @@ def notify(exception, auto_notify=false, &block) # If this is an auto_notify we yield the block before the any middleware is run begin yield(report) if block_given? && auto_notify - rescue => e + rescue StandardError => e configuration.warn("Error in internal notify block: #{e}") configuration.warn("Error in internal notify block stacktrace: #{e.backtrace.inspect}") end @@ -113,7 +113,7 @@ def notify(exception, auto_notify=false, &block) # block that is run as it is the users "most specific" block. begin yield(report) if block_given? && !auto_notify - rescue => e + rescue StandardError => e configuration.warn("Error in notify block: #{e}") configuration.warn("Error in notify block stacktrace: #{e.backtrace.inspect}") end From b2b3fcf1316f61925835671ad7e04c984c6e7a08 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 16 Jun 2021 09:20:19 +0100 Subject: [PATCH 10/19] Replace therubyracer with mini_racer ExecJS doesn't support therubyracer anymore so the delayed_job app doesn't run --- features/fixtures/delayed_job/app/Gemfile | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/features/fixtures/delayed_job/app/Gemfile b/features/fixtures/delayed_job/app/Gemfile index a9ed25d13..67ffaa332 100644 --- a/features/fixtures/delayed_job/app/Gemfile +++ b/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' From f3f3631f5e9cab1d169e50e5293bcc3c2685fbb3 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 16 Jun 2021 15:07:13 +0100 Subject: [PATCH 11/19] Add changelog entry --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ecefaee4..8b6289d87 100644 --- a/CHANGELOG.md +++ b/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 From e5d5e802404d6f838f9edbcf2d0f02ca6cc9c5e8 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 16 Jun 2021 15:22:44 +0100 Subject: [PATCH 12/19] 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 13/19] 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 14/19] 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 15/19] 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 From 13aa40e5fec00c019afe450cfe3eb46d3c38ab08 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Mon, 21 Jun 2021 15:44:14 +0100 Subject: [PATCH 16/19] Fix possible NoMethodError in railtie This can happen if Bugsnag is added to the Gemfile with 'require: false'. In this case, 'require "bugsnag"' in a Rails initializer will run the 'config.before_initialize' before the rest of the Railtie has been executed. This results in the 'event_subscription' method being undefined as it was declared below the line that calls it By moving it to the top of the file this is no longer a problem --- lib/bugsnag/integrations/railtie.rb | 69 ++++++++++++++--------------- 1 file changed, 34 insertions(+), 35 deletions(-) diff --git a/lib/bugsnag/integrations/railtie.rb b/lib/bugsnag/integrations/railtie.rb index c43f738bf..c11fc2165 100644 --- a/lib/bugsnag/integrations/railtie.rb +++ b/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 From f91fb2424d96e1bd26c99811993b4350db53e670 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 23 Jun 2021 14:34:44 +0100 Subject: [PATCH 17/19] Add #666 to changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cb58b0ae9..1de6b7d42 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ Changelog * 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) From ac30073600ba9dc195f703c418acf5b0bb23cd94 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 23 Jun 2021 14:34:50 +0100 Subject: [PATCH 18/19] Bump changelog version --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1de6b7d42..7d78c86cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ Changelog ========= -## TBC +## v6.21.0 (23 June 2021) ### Enhancements From e9dc9414f6c3b72a9f32953253ae55d7de62e139 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Wed, 23 Jun 2021 14:35:15 +0100 Subject: [PATCH 19/19] Bump version --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index a7c176ac7..7ecad1405 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -6.20.0 +6.21.0