From a530de77e65379b987f2ff68c96e21969a059767 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 23 Jul 2020 10:42:00 +0100 Subject: [PATCH 01/16] Supress frozen middleware errors in Railtie This should never happen in normal operation, but is possible when running RSpec tests See https://github.com/thoughtbot/factory_bot_rails/issues/303#issuecomment-434560625 and https://github.com/bugsnag/bugsnag-ruby/issues/534 Essentially this happens: 1. RSpec boots Rails 2. Rails boot process errors 3. Rails freezes middleware 4. RSpec moves on to the next test file 5. Bugsnag tries to add middleware but fails because it's frozen 6. The stacktrace blames Bugsnag for this test failure 7. goto 4 Supressing this error doesn't solve the problem as it's likely another frozen error will happen in some other library/component, but we want to avoid people thinking this is caused by Bugsnag. The stacktrace does point out the actual problem, but only in the very first error, which can get buried under the frozen errors that happen for each test file --- lib/bugsnag/integrations/railtie.rb | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/lib/bugsnag/integrations/railtie.rb b/lib/bugsnag/integrations/railtie.rb index 763ab7f16..015c66620 100644 --- a/lib/bugsnag/integrations/railtie.rb +++ b/lib/bugsnag/integrations/railtie.rb @@ -63,9 +63,19 @@ class Railtie < ::Rails::Railtie initializer "bugsnag.use_rack_middleware" do |app| begin - app.config.middleware.insert_after ActionDispatch::DebugExceptions, Bugsnag::Rack - rescue - app.config.middleware.use Bugsnag::Rack + begin + app.config.middleware.insert_after ActionDispatch::DebugExceptions, Bugsnag::Rack + rescue + app.config.middleware.use Bugsnag::Rack + end + rescue FrozenError + # This can happen when running RSpec if there is a crash after Rails has + # started booting but before we've added our middleware. If we don't ignore + # this error then the stacktrace blames Bugsnag, which isn't accurate as + # the middleware will only be frozen if an earlier error occurs + # See this comment for more info: + # https://github.com/thoughtbot/factory_bot_rails/issues/303#issuecomment-434560625 + Bugsnag.configuration.warn("Unable to add Bugsnag::Rack middleware as the middleware stack is frozen") end end From a91776e9bb655b1938cca341ce883521d7a8666d Mon Sep 17 00:00:00 2001 From: Mike Stewart Date: Mon, 26 Nov 2018 19:52:07 -0400 Subject: [PATCH 02/16] Set consistent context in delayed_job integration --- lib/bugsnag/middleware/delayed_job.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/bugsnag/middleware/delayed_job.rb b/lib/bugsnag/middleware/delayed_job.rb index 0476e1ef9..5539d6960 100644 --- a/lib/bugsnag/middleware/delayed_job.rb +++ b/lib/bugsnag/middleware/delayed_job.rb @@ -24,6 +24,7 @@ def call(report) job_data[:active_job] = job.payload_object.job_data if job.payload_object.respond_to?(:job_data) payload_data = construct_job_payload(job.payload_object) report.context = payload_data[:display_name] if payload_data.include?(:display_name) + report.context ||= payload_data[:class] if payload_data.include?(:class) job_data[:payload] = payload_data end From a798bed7807f5ab4f08922e8ee16d4b3c8053c15 Mon Sep 17 00:00:00 2001 From: Mike Stewart Date: Tue, 27 Nov 2018 13:38:01 -0400 Subject: [PATCH 03/16] Changelog entry for bugsnag/bugsnag-ruby#499 --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c416d366..875a17c86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,14 @@ Changelog ========= +## TBD + +### Enhancements + +* Set default Delayed Job error context to job class + | [#499](https://github.com/bugsnag/bugsnag-ruby/pull/499) + | [Mike Stewart](https://github.com/mike-stewart) + ## 6.15.0 (27 July 2020) ### Enhancements From 80e0c800992ea37b2398437a42ca0b29047d4112 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Mon, 27 Jul 2020 12:29:35 +0100 Subject: [PATCH 04/16] Add maze runner test for report context class name --- features/delayed_job.feature | 17 +++++++++++++++++ features/fixtures/delayed_job/app/Rakefile | 6 +++++- .../app/app/jobs/test_report_context_job.rb | 7 +++++++ 3 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 features/fixtures/delayed_job/app/app/jobs/test_report_context_job.rb diff --git a/features/delayed_job.feature b/features/delayed_job.feature index a6a004ebe..e9b062550 100644 --- a/features/delayed_job.feature +++ b/features/delayed_job.feature @@ -34,3 +34,20 @@ Scenario: A handled exception sends a report And the event "metaData.job.payload.display_name" equals "TestModel.notify_with_args" And the event "metaData.job.payload.method_name" equals "notify_with_args" And the payload field "events.0.metaData.job.payload.args.0" equals "Test" + +Scenario: The report context uses the class name if no display name is available + Given I run the service "delayed_job" with the command "bundle exec rake delayed_job_tests:report_context" + And I wait to receive a request + Then the request is valid for the error reporting API version "4.0" for the "Ruby Bugsnag Notifier" + And the event "unhandled" is true + And the event "severity" equals "error" + And the event "context" equals "TestReportContextJob" + And the event "severityReason.type" equals "unhandledExceptionMiddleware" + And the event "severityReason.attributes.framework" equals "DelayedJob" + And the exception "errorClass" equals "RuntimeError" + And the event "metaData.job.class" equals "Delayed::Backend::ActiveRecord::Job" + And the event "metaData.job.id" is not null + And the event "metaData.job.attempt" equals 1 + And the event "metaData.job.max_attempts" equals 1 + And the event "metaData.job.payload.display_name" is null + And the event "metaData.job.payload.method_name" is null diff --git a/features/fixtures/delayed_job/app/Rakefile b/features/fixtures/delayed_job/app/Rakefile index 719fc8574..efcae1adb 100644 --- a/features/fixtures/delayed_job/app/Rakefile +++ b/features/fixtures/delayed_job/app/Rakefile @@ -13,6 +13,10 @@ namespace :delayed_job_tests do task :notify_with_args do run_delayed_job_test('"TestModel.delay.notify_with_args(\"Test\")"') end + + task :report_context do + run_delayed_job_test('"Delayed::Job.enqueue TestReportContextJob.new"') + end end def run_delayed_job_test(command) @@ -21,4 +25,4 @@ def run_delayed_job_test(command) end system("rails runner #{command}") Process.wait -end \ No newline at end of file +end diff --git a/features/fixtures/delayed_job/app/app/jobs/test_report_context_job.rb b/features/fixtures/delayed_job/app/app/jobs/test_report_context_job.rb new file mode 100644 index 000000000..4efeecb3d --- /dev/null +++ b/features/fixtures/delayed_job/app/app/jobs/test_report_context_job.rb @@ -0,0 +1,7 @@ +class TestReportContextJob < ApplicationJob + queue_as :default + + def perform(*) + raise "oh dear" + end +end From c3b1c7805e5044b09b42137971a23fd66d44301a Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 23 Jul 2020 13:56:27 +0100 Subject: [PATCH 05/16] Use BUGSNAG_RELEASE_STAGE by default Previously we read 'BUGSNAG_RELEASE_STAGE' only in the Rails integration, so other apps had to use 'RACK_ENV' (if they run in Rack) or set it manually in code Now we will always use the 'BUGSNAG_RELEASE_STAGE' environment variable if it's set --- lib/bugsnag/configuration.rb | 1 + lib/bugsnag/integrations/railtie.rb | 2 +- spec/configuration_spec.rb | 15 +++++++++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/lib/bugsnag/configuration.rb b/lib/bugsnag/configuration.rb index b8e8954f8..bd1b3fc11 100644 --- a/lib/bugsnag/configuration.rb +++ b/lib/bugsnag/configuration.rb @@ -116,6 +116,7 @@ def initialize self.runtime_versions["ruby"] = RUBY_VERSION self.runtime_versions["jruby"] = JRUBY_VERSION if defined?(JRUBY_VERSION) self.timeout = 15 + self.release_stage = ENV['BUGSNAG_RELEASE_STAGE'] self.notify_release_stages = nil self.auto_capture_sessions = true diff --git a/lib/bugsnag/integrations/railtie.rb b/lib/bugsnag/integrations/railtie.rb index 015c66620..583a7bd93 100644 --- a/lib/bugsnag/integrations/railtie.rb +++ b/lib/bugsnag/integrations/railtie.rb @@ -25,7 +25,7 @@ class Railtie < ::Rails::Railtie # initializer. If not, the key will be validated in after_initialize. Bugsnag.configure(false) do |config| config.logger = ::Rails.logger - config.release_stage = ENV["BUGSNAG_RELEASE_STAGE"] || ::Rails.env.to_s + config.release_stage ||= ::Rails.env.to_s config.project_root = ::Rails.root.to_s config.middleware.insert_before Bugsnag::Middleware::Callbacks, Bugsnag::Middleware::Rails3Request config.runtime_versions["rails"] = ::Rails::VERSION::STRING diff --git a/spec/configuration_spec.rb b/spec/configuration_spec.rb index 9bc1d5345..8a2a3a942 100644 --- a/spec/configuration_spec.rb +++ b/spec/configuration_spec.rb @@ -24,6 +24,21 @@ end end + describe "release_stage" do + after(:each) do + ENV["BUGSNAG_RELEASE_STAGE"] = nil + end + + it "has no default value" do + expect(subject.release_stage).to be_nil + end + + it "uses the 'BUGSNAG_RELEASE_STAGE' environment variable if set" do + ENV["BUGSNAG_RELEASE_STAGE"] = "foobar" + expect(subject.release_stage).to eq("foobar") + end + end + describe "#notify_endpoint" do it "defaults to DEFAULT_NOTIFY_ENDPOINT" do expect(subject.notify_endpoint).to eq(Bugsnag::Configuration::DEFAULT_NOTIFY_ENDPOINT) From a626e1cc68cffec7b1b11a2c82bd5b504bad7e6b Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 23 Jul 2020 16:21:03 +0100 Subject: [PATCH 06/16] We no longer need to set the release stage env var This should now happen automatically for us, so the existing MR tests can verify that this feature works :^) --- .../fixtures/delayed_job/app/config/initializers/bugsnag.rb | 1 - features/fixtures/plain/app/app.rb | 3 +-- features/fixtures/rails3/app/config/initializers/bugsnag.rb | 1 - features/fixtures/rails4/app/config/initializers/bugsnag.rb | 1 - features/fixtures/rails5/app/config/initializers/bugsnag.rb | 1 - features/fixtures/rails6/app/config/initializers/bugsnag.rb | 1 - 6 files changed, 1 insertion(+), 7 deletions(-) diff --git a/features/fixtures/delayed_job/app/config/initializers/bugsnag.rb b/features/fixtures/delayed_job/app/config/initializers/bugsnag.rb index da2c39de9..fda4c546b 100644 --- a/features/fixtures/delayed_job/app/config/initializers/bugsnag.rb +++ b/features/fixtures/delayed_job/app/config/initializers/bugsnag.rb @@ -8,7 +8,6 @@ config.project_root = ENV["BUGSNAG_PROJECT_ROOT"] if ENV.include? "BUGSNAG_PROJECT_ROOT" config.ignore_classes << lambda { |ex| ex.class.to_s == ENV["BUGSNAG_IGNORE_CLASS"] } if ENV.include? "BUGSNAG_IGNORE_CLASS" config.auto_capture_sessions = ENV["BUGSNAG_AUTO_CAPTURE_SESSIONS"] == "true" - config.release_stage = ENV["BUGSNAG_RELEASE_STAGE"] if ENV.include? "BUGSNAG_RELEASE_STAGE" config.send_code = ENV["BUGSNAG_SEND_CODE"] != "false" config.send_environment = ENV["BUGSNAG_SEND_ENVIRONMENT"] == "true" end diff --git a/features/fixtures/plain/app/app.rb b/features/fixtures/plain/app/app.rb index dc66f28f7..37524117e 100644 --- a/features/fixtures/plain/app/app.rb +++ b/features/fixtures/plain/app/app.rb @@ -22,7 +22,6 @@ def configure_using_environment conf.proxy_password = ENV["BUGSNAG_PROXY_PASSWORD"] if ENV.include? "BUGSNAG_PROXY_PASSWORD" conf.proxy_port = ENV["BUGSNAG_PROXY_PORT"] if ENV.include? "BUGSNAG_PROXY_PORT" conf.proxy_user = ENV["BUGSNAG_PROXY_USER"] if ENV.include? "BUGSNAG_PROXY_USER" - conf.release_stage = ENV["BUGSNAG_RELEASE_STAGE"] if ENV.include? "BUGSNAG_RELEASE_STAGE" conf.send_environment = ENV["BUGSNAG_SEND_ENVIRONMENT"] != "false" conf.send_code = ENV["BUGSNAG_SEND_CODE"] != "false" conf.timeout = ENV["BUGSNAG_TIMEOUT"] if ENV.include? "BUGSNAG_TIMEOUT" @@ -35,4 +34,4 @@ def add_at_exit Bugsnag.notify($!, true) end end -end \ No newline at end of file +end diff --git a/features/fixtures/rails3/app/config/initializers/bugsnag.rb b/features/fixtures/rails3/app/config/initializers/bugsnag.rb index 8a1ac08ea..baa9c5d3f 100644 --- a/features/fixtures/rails3/app/config/initializers/bugsnag.rb +++ b/features/fixtures/rails3/app/config/initializers/bugsnag.rb @@ -8,7 +8,6 @@ config.project_root = ENV["BUGSNAG_PROJECT_ROOT"] if ENV.include? "BUGSNAG_PROJECT_ROOT" config.ignore_classes << lambda { |ex| ex.class.to_s == ENV["BUGSNAG_IGNORE_CLASS"] } if ENV.include? "BUGSNAG_IGNORE_CLASS" config.auto_capture_sessions = ENV["BUGSNAG_AUTO_CAPTURE_SESSIONS"] == "true" unless ENV["USE_DEFAULT_AUTO_CAPTURE_SESSIONS"] == "true" - config.release_stage = ENV["BUGSNAG_RELEASE_STAGE"] if ENV.include? "BUGSNAG_RELEASE_STAGE" config.send_code = ENV["BUGSNAG_SEND_CODE"] != "false" config.send_environment = ENV["BUGSNAG_SEND_ENVIRONMENT"] == "true" config.meta_data_filters << 'filtered_parameter' diff --git a/features/fixtures/rails4/app/config/initializers/bugsnag.rb b/features/fixtures/rails4/app/config/initializers/bugsnag.rb index 8a1ac08ea..baa9c5d3f 100644 --- a/features/fixtures/rails4/app/config/initializers/bugsnag.rb +++ b/features/fixtures/rails4/app/config/initializers/bugsnag.rb @@ -8,7 +8,6 @@ config.project_root = ENV["BUGSNAG_PROJECT_ROOT"] if ENV.include? "BUGSNAG_PROJECT_ROOT" config.ignore_classes << lambda { |ex| ex.class.to_s == ENV["BUGSNAG_IGNORE_CLASS"] } if ENV.include? "BUGSNAG_IGNORE_CLASS" config.auto_capture_sessions = ENV["BUGSNAG_AUTO_CAPTURE_SESSIONS"] == "true" unless ENV["USE_DEFAULT_AUTO_CAPTURE_SESSIONS"] == "true" - config.release_stage = ENV["BUGSNAG_RELEASE_STAGE"] if ENV.include? "BUGSNAG_RELEASE_STAGE" config.send_code = ENV["BUGSNAG_SEND_CODE"] != "false" config.send_environment = ENV["BUGSNAG_SEND_ENVIRONMENT"] == "true" config.meta_data_filters << 'filtered_parameter' diff --git a/features/fixtures/rails5/app/config/initializers/bugsnag.rb b/features/fixtures/rails5/app/config/initializers/bugsnag.rb index 8a1ac08ea..baa9c5d3f 100644 --- a/features/fixtures/rails5/app/config/initializers/bugsnag.rb +++ b/features/fixtures/rails5/app/config/initializers/bugsnag.rb @@ -8,7 +8,6 @@ config.project_root = ENV["BUGSNAG_PROJECT_ROOT"] if ENV.include? "BUGSNAG_PROJECT_ROOT" config.ignore_classes << lambda { |ex| ex.class.to_s == ENV["BUGSNAG_IGNORE_CLASS"] } if ENV.include? "BUGSNAG_IGNORE_CLASS" config.auto_capture_sessions = ENV["BUGSNAG_AUTO_CAPTURE_SESSIONS"] == "true" unless ENV["USE_DEFAULT_AUTO_CAPTURE_SESSIONS"] == "true" - config.release_stage = ENV["BUGSNAG_RELEASE_STAGE"] if ENV.include? "BUGSNAG_RELEASE_STAGE" config.send_code = ENV["BUGSNAG_SEND_CODE"] != "false" config.send_environment = ENV["BUGSNAG_SEND_ENVIRONMENT"] == "true" config.meta_data_filters << 'filtered_parameter' diff --git a/features/fixtures/rails6/app/config/initializers/bugsnag.rb b/features/fixtures/rails6/app/config/initializers/bugsnag.rb index 8a1ac08ea..baa9c5d3f 100644 --- a/features/fixtures/rails6/app/config/initializers/bugsnag.rb +++ b/features/fixtures/rails6/app/config/initializers/bugsnag.rb @@ -8,7 +8,6 @@ config.project_root = ENV["BUGSNAG_PROJECT_ROOT"] if ENV.include? "BUGSNAG_PROJECT_ROOT" config.ignore_classes << lambda { |ex| ex.class.to_s == ENV["BUGSNAG_IGNORE_CLASS"] } if ENV.include? "BUGSNAG_IGNORE_CLASS" config.auto_capture_sessions = ENV["BUGSNAG_AUTO_CAPTURE_SESSIONS"] == "true" unless ENV["USE_DEFAULT_AUTO_CAPTURE_SESSIONS"] == "true" - config.release_stage = ENV["BUGSNAG_RELEASE_STAGE"] if ENV.include? "BUGSNAG_RELEASE_STAGE" config.send_code = ENV["BUGSNAG_SEND_CODE"] != "false" config.send_environment = ENV["BUGSNAG_SEND_ENVIRONMENT"] == "true" config.meta_data_filters << 'filtered_parameter' From 785043423e86aaeb8132ad39a54c9f3b0b41b0d3 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Mon, 27 Jul 2020 14:29:37 +0100 Subject: [PATCH 07/16] Add #613 to changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 875a17c86..7295e1a39 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ Changelog | [#499](https://github.com/bugsnag/bugsnag-ruby/pull/499) | [Mike Stewart](https://github.com/mike-stewart) +* The `BUGSNAG_RELEASE_STAGE` environment variable can now be used to set the release stage. Previously this was only used in Rails applications + | [#613](https://github.com/bugsnag/bugsnag-ruby/pull/613) + ## 6.15.0 (27 July 2020) ### Enhancements From bd03d1dfdf7bb9537d4f6227391bd28fd7108144 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 30 Jul 2020 17:23:53 +0100 Subject: [PATCH 08/16] Fix order app_type is resolved MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixes a couple of issues with `app_type` — we now always use the value set by a user (if one is set) and integrations should now always set the correct value For example, before this change Que, when running in a rails app, would have an app type of 'rails'. This is because the Que integration has to run when it's loaded and so sets its app type before Rails does --- .rubocop_todo.yml | 2 +- CHANGELOG.md | 5 +++ lib/bugsnag/configuration.rb | 49 ++++++++++++++++++++++++- lib/bugsnag/integrations/delayed_job.rb | 2 +- lib/bugsnag/integrations/mailman.rb | 2 +- lib/bugsnag/integrations/que.rb | 3 +- lib/bugsnag/integrations/rack.rb | 4 +- lib/bugsnag/integrations/railtie.rb | 4 +- lib/bugsnag/integrations/rake.rb | 6 ++- lib/bugsnag/integrations/resque.rb | 4 +- lib/bugsnag/integrations/shoryuken.rb | 2 +- lib/bugsnag/integrations/sidekiq.rb | 2 +- spec/configuration_spec.rb | 22 +++++++++++ spec/integrations/mailman_spec.rb | 2 +- spec/integrations/que_spec.rb | 5 +-- spec/integrations/resque_spec.rb | 2 +- spec/integrations/shoryuken_spec.rb | 8 ++-- 17 files changed, 101 insertions(+), 23 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index fcb383927..5a4a547dc 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -480,7 +480,7 @@ Metrics/BlockNesting: # Offense count: 4 # Configuration parameters: CountComments. Metrics/ClassLength: - Max: 174 + Max: 184 # Offense count: 15 # Configuration parameters: IgnoredMethods. diff --git a/CHANGELOG.md b/CHANGELOG.md index 7295e1a39..e4bfd8d6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,11 @@ Changelog * The `BUGSNAG_RELEASE_STAGE` environment variable can now be used to set the release stage. Previously this was only used in Rails applications | [#613](https://github.com/bugsnag/bugsnag-ruby/pull/613) +## Fixes + +* The `app_type` configuration option should no longer be overwritten by Bugsnag and integrations should set this more consistently + | [#619](https://github.com/bugsnag/bugsnag-ruby/pull/619) + ## 6.15.0 (27 July 2020) ### Enhancements diff --git a/lib/bugsnag/configuration.rb b/lib/bugsnag/configuration.rb index bd1b3fc11..44b196e94 100644 --- a/lib/bugsnag/configuration.rb +++ b/lib/bugsnag/configuration.rb @@ -24,7 +24,6 @@ class Configuration attr_accessor :send_code attr_accessor :project_root attr_accessor :app_version - attr_accessor :app_type attr_accessor :meta_data_filters attr_accessor :logger attr_accessor :middleware @@ -198,6 +197,54 @@ def default_delivery_method=(delivery_method) @default_delivery_method = delivery_method end + ## + # Get the type of application executing the current code + # + # This is usually used to represent if you are running in a Rails server, + # Sidekiq job, Rake task etc... Bugsnag will automatically detect most + # application types for you + # + # @return [String, nil] + def app_type + @app_type || @detected_app_type + end + + ## + # Set the type of application executing the current code + # + # If an app_type is set, this will be used instead of the automatically + # detected app_type that Bugsnag would otherwise use + # + # @param app_type [String] + # @return [void] + def app_type=(app_type) + @app_type = app_type + end + + ## + # Get the detected app_type, which is used when one isn't set explicitly + # + # @api private + # + # @return [String, nil] + def detected_app_type + @detected_app_type + end + + ## + # Set the detected app_type, which is used when one isn't set explicitly + # + # This allows Bugsnag's integrations to say 'this is a Rails app' while + # allowing the user to overwrite this if they wish + # + # @api private + # + # @param app_type [String] + # @return [void] + def detected_app_type=(app_type) + @detected_app_type = app_type + end + ## # Indicates whether the notifier should send a notification based on the # configured release stage. diff --git a/lib/bugsnag/integrations/delayed_job.rb b/lib/bugsnag/integrations/delayed_job.rb index 16b232ea7..9cfec1a20 100644 --- a/lib/bugsnag/integrations/delayed_job.rb +++ b/lib/bugsnag/integrations/delayed_job.rb @@ -13,7 +13,7 @@ class Bugsnag < ::Delayed::Plugin callbacks do |lifecycle| lifecycle.around(:invoke_job) do |job, *args, &block| begin - ::Bugsnag.configuration.app_type = 'delayed_job' + ::Bugsnag.configuration.detected_app_type = 'delayed_job' ::Bugsnag.configuration.set_request_data(:delayed_job, job) block.call(job, *args) rescue Exception => exception diff --git a/lib/bugsnag/integrations/mailman.rb b/lib/bugsnag/integrations/mailman.rb index d49530fcd..b50b56e07 100644 --- a/lib/bugsnag/integrations/mailman.rb +++ b/lib/bugsnag/integrations/mailman.rb @@ -11,7 +11,7 @@ class Mailman def initialize Bugsnag.configuration.internal_middleware.use(Bugsnag::Middleware::Mailman) - Bugsnag.configuration.app_type = "mailman" + Bugsnag.configuration.detected_app_type = "mailman" end ## diff --git a/lib/bugsnag/integrations/que.rb b/lib/bugsnag/integrations/que.rb index b9cb7c8a9..39687a289 100644 --- a/lib/bugsnag/integrations/que.rb +++ b/lib/bugsnag/integrations/que.rb @@ -37,7 +37,8 @@ end end - Bugsnag.configuration.app_type ||= "que" + Bugsnag.configuration.detected_app_type = "que" + if defined?(::Que::Version) Bugsnag.configuration.runtime_versions["que"] = ::Que::Version elsif defined?(::Que::VERSION) diff --git a/lib/bugsnag/integrations/rack.rb b/lib/bugsnag/integrations/rack.rb index e7569b2a0..46a3ade29 100644 --- a/lib/bugsnag/integrations/rack.rb +++ b/lib/bugsnag/integrations/rack.rb @@ -30,7 +30,9 @@ def initialize(app) config.middleware.insert_before(Bugsnag::Middleware::Callbacks, Bugsnag::Middleware::ClearanceUser) if defined?(Clearance) # Set environment data for payload - config.app_type ||= "rack" + # Note we only set the detected app_type if it's not already set. This + # ensures we don't overwrite the value set by the Railtie + config.detected_app_type ||= "rack" config.runtime_versions["rack"] = ::Rack.release if defined?(::Rack) config.runtime_versions["sinatra"] = ::Sinatra::VERSION if defined?(::Sinatra) end diff --git a/lib/bugsnag/integrations/railtie.rb b/lib/bugsnag/integrations/railtie.rb index 583a7bd93..d58dd3b8f 100644 --- a/lib/bugsnag/integrations/railtie.rb +++ b/lib/bugsnag/integrations/railtie.rb @@ -43,7 +43,9 @@ class Railtie < ::Rails::Railtie Bugsnag::Rails::DEFAULT_RAILS_BREADCRUMBS.each { |event| event_subscription(event) } - Bugsnag.configuration.app_type = "rails" + # Make sure we don't overwrite the value set by another integration because + # Rails is a less specific app_type (e.g. Que sets this earlier than us) + Bugsnag.configuration.detected_app_type ||= "rails" end # Configure meta_data_filters after initialization, so that rails initializers diff --git a/lib/bugsnag/integrations/rake.rb b/lib/bugsnag/integrations/rake.rb index dcd426451..6d23668d9 100644 --- a/lib/bugsnag/integrations/rake.rb +++ b/lib/bugsnag/integrations/rake.rb @@ -11,7 +11,8 @@ module RakeTask # Executes the rake task with Bugsnag setup with contextual data. def execute(args = nil) - Bugsnag.configuration.app_type ||= "rake" + Bugsnag.configuration.detected_app_type = "rake" + old_task = Bugsnag.configuration.request_data[:bugsnag_running_task] Bugsnag.configuration.set_request_data :bugsnag_running_task, self Bugsnag.configuration.runtime_versions["rake"] = ::Rake::VERSION @@ -42,7 +43,8 @@ class Rake::Task ## # Executes the rake task with Bugsnag setup with contextual data. def execute_with_bugsnag(args=nil) - Bugsnag.configuration.app_type ||= "rake" + Bugsnag.configuration.detected_app_type = "rake" + old_task = Bugsnag.configuration.request_data[:bugsnag_running_task] Bugsnag.configuration.set_request_data :bugsnag_running_task, self Bugsnag.configuration.runtime_versions["rake"] = ::Rake::VERSION diff --git a/lib/bugsnag/integrations/resque.rb b/lib/bugsnag/integrations/resque.rb index 913dd43c4..8ce9b754d 100644 --- a/lib/bugsnag/integrations/resque.rb +++ b/lib/bugsnag/integrations/resque.rb @@ -60,13 +60,13 @@ def save if Resque::Worker.new(:bugsnag_fork_check).fork_per_job? Resque.after_fork do - Bugsnag.configuration.app_type = "resque" + Bugsnag.configuration.detected_app_type = "resque" Bugsnag.configuration.default_delivery_method = :synchronous Bugsnag.configuration.runtime_versions["resque"] = ::Resque::VERSION end else Resque.before_first_fork do - Bugsnag.configuration.app_type = "resque" + Bugsnag.configuration.detected_app_type = "resque" Bugsnag.configuration.default_delivery_method = :synchronous Bugsnag.configuration.runtime_versions["resque"] = ::Resque::VERSION end diff --git a/lib/bugsnag/integrations/shoryuken.rb b/lib/bugsnag/integrations/shoryuken.rb index 052ad3498..ccd07c882 100644 --- a/lib/bugsnag/integrations/shoryuken.rb +++ b/lib/bugsnag/integrations/shoryuken.rb @@ -11,7 +11,7 @@ class Shoryuken def initialize Bugsnag.configure do |config| - config.app_type ||= "shoryuken" + config.detected_app_type = "shoryuken" config.default_delivery_method = :synchronous end end diff --git a/lib/bugsnag/integrations/sidekiq.rb b/lib/bugsnag/integrations/sidekiq.rb index f392bcc1e..b2590e5b1 100644 --- a/lib/bugsnag/integrations/sidekiq.rb +++ b/lib/bugsnag/integrations/sidekiq.rb @@ -13,7 +13,7 @@ class Sidekiq def initialize Bugsnag.configuration.internal_middleware.use(Bugsnag::Middleware::Sidekiq) - Bugsnag.configuration.app_type = "sidekiq" + Bugsnag.configuration.detected_app_type = "sidekiq" Bugsnag.configuration.default_delivery_method = :synchronous Bugsnag.configuration.runtime_versions["sidekiq"] = ::Sidekiq::VERSION end diff --git a/spec/configuration_spec.rb b/spec/configuration_spec.rb index 8a2a3a942..b4c8acd1c 100644 --- a/spec/configuration_spec.rb +++ b/spec/configuration_spec.rb @@ -39,6 +39,28 @@ end end + describe "app_type" do + it "should default to nil" do + expect(subject.app_type).to be_nil + end + + it "should be settable directly" do + subject.app_type = :test + expect(subject.app_type).to eq(:test) + end + + it "should allow a detected app_type to be set" do + subject.detected_app_type = :test + expect(subject.app_type).to eq(:test) + end + + it "should allow the app_type to be set over a default" do + subject.detected_app_type = :test + subject.app_type = :wow + expect(subject.app_type).to eq(:wow) + end + end + describe "#notify_endpoint" do it "defaults to DEFAULT_NOTIFY_ENDPOINT" do expect(subject.notify_endpoint).to eq(Bugsnag::Configuration::DEFAULT_NOTIFY_ENDPOINT) diff --git a/spec/integrations/mailman_spec.rb b/spec/integrations/mailman_spec.rb index 42f22d58b..ba8ddad27 100644 --- a/spec/integrations/mailman_spec.rb +++ b/spec/integrations/mailman_spec.rb @@ -33,7 +33,7 @@ def require(path) int_middleware = double('internal_middleware') expect(config).to receive(:internal_middleware).and_return(int_middleware) expect(int_middleware).to receive(:use).with(Bugsnag::Middleware::Mailman) - expect(config).to receive(:app_type=).with("mailman") + expect(config).to receive(:detected_app_type=).with("mailman") integration = Bugsnag::Mailman.new diff --git a/spec/integrations/que_spec.rb b/spec/integrations/que_spec.rb index 5708179b2..ff525740a 100644 --- a/spec/integrations/que_spec.rb +++ b/spec/integrations/que_spec.rb @@ -52,8 +52,7 @@ def require(path) allow(Que).to receive(:respond_to?).with(:error_notifier=).and_return(true) config = double('config') allow(Bugsnag).to receive(:configuration).and_return(config) - expect(config).to receive(:app_type) - expect(config).to receive(:app_type=).with('que') + expect(config).to receive(:detected_app_type=).with('que') runtime = {} expect(config).to receive(:runtime_versions).and_return(runtime) allow(config).to receive(:clear_request_data) @@ -63,7 +62,7 @@ def require(path) #Kick off load './lib/bugsnag/integrations/que.rb' - + expect(runtime).to eq("que" => "9.9.9") end diff --git a/spec/integrations/resque_spec.rb b/spec/integrations/resque_spec.rb index ac04ad9e5..dc6dabd3c 100644 --- a/spec/integrations/resque_spec.rb +++ b/spec/integrations/resque_spec.rb @@ -44,7 +44,7 @@ def require(path) expect(::Resque::Worker).to receive(:new).with(:bugsnag_fork_check).and_return(fork_check) expect(fork_check).to receive(:fork_per_job?).and_return(true) expect(::Resque).to receive(:after_fork).and_yield - expect(Bugsnag.configuration).to receive(:app_type=).with("resque") + expect(Bugsnag.configuration).to receive(:detected_app_type=).with("resque") runtime = {} expect(Bugsnag.configuration).to receive(:runtime_versions).and_return(runtime) expect(Bugsnag.configuration).to receive(:default_delivery_method=).with(:synchronous) diff --git a/spec/integrations/shoryuken_spec.rb b/spec/integrations/shoryuken_spec.rb index b6a6c4750..df4d7f882 100644 --- a/spec/integrations/shoryuken_spec.rb +++ b/spec/integrations/shoryuken_spec.rb @@ -28,9 +28,8 @@ def require(path) it "calls configure when initialised" do config = double("config") - - expect(config).to receive(:app_type).and_return(nil) - expect(config).to receive(:app_type=).with("shoryuken") + + expect(config).to receive(:detected_app_type=).with("shoryuken") expect(config).to receive(:default_delivery_method=).with(:synchronous) expect(Bugsnag).to receive(:configure).and_yield(config) Bugsnag::Shoryuken.new @@ -50,8 +49,7 @@ def require(path) func.call(report) end config = double('config') - allow(config).to receive(:app_type).and_return(nil) - allow(config).to receive(:app_type=).with("shoryuken") + allow(config).to receive(:detected_app_type=).with("shoryuken") allow(config).to receive(:default_delivery_method=).with(:synchronous) allow(config).to receive(:clear_request_data) expect(Bugsnag).to receive(:before_notify_callbacks).and_return(callbacks) From 40c13ff4b8c3106e91e4703c039de761ac343ceb Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Fri, 31 Jul 2020 16:28:54 +0100 Subject: [PATCH 09/16] Report runtime versions where they were missing This adds runtime versions for: - Delayed Job - Mailman - Shoryuken Mailman & Shoryuken are straightforward, but Delayed Job doesn't have a constant with its version in. Instead we can read it from the loaded Gems --- features/delayed_job.feature | 3 ++ lib/bugsnag/integrations/delayed_job.rb | 12 ++++++ lib/bugsnag/integrations/mailman.rb | 1 + lib/bugsnag/integrations/shoryuken.rb | 1 + spec/integrations/mailman_spec.rb | 49 ++++++++++++---------- spec/integrations/shoryuken_spec.rb | 55 ++++++++++++++----------- 6 files changed, 74 insertions(+), 47 deletions(-) diff --git a/features/delayed_job.feature b/features/delayed_job.feature index e9b062550..531512b79 100644 --- a/features/delayed_job.feature +++ b/features/delayed_job.feature @@ -17,6 +17,7 @@ Scenario: An unhandled RuntimeError sends a report with arguments And the event "metaData.job.payload.display_name" equals "TestModel.fail_with_args" And the event "metaData.job.payload.method_name" equals "fail_with_args" And the payload field "events.0.metaData.job.payload.args.0" equals "Test" + And the event "device.runtimeVersions.delayed_job" equals "4.1.8" Scenario: A handled exception sends a report Given I run the service "delayed_job" with the command "bundle exec rake delayed_job_tests:notify_with_args" @@ -34,6 +35,7 @@ Scenario: A handled exception sends a report And the event "metaData.job.payload.display_name" equals "TestModel.notify_with_args" And the event "metaData.job.payload.method_name" equals "notify_with_args" And the payload field "events.0.metaData.job.payload.args.0" equals "Test" + And the event "device.runtimeVersions.delayed_job" equals "4.1.8" Scenario: The report context uses the class name if no display name is available Given I run the service "delayed_job" with the command "bundle exec rake delayed_job_tests:report_context" @@ -51,3 +53,4 @@ Scenario: The report context uses the class name if no display name is available And the event "metaData.job.max_attempts" equals 1 And the event "metaData.job.payload.display_name" is null And the event "metaData.job.payload.method_name" is null + And the event "device.runtimeVersions.delayed_job" equals "4.1.8" diff --git a/lib/bugsnag/integrations/delayed_job.rb b/lib/bugsnag/integrations/delayed_job.rb index 9cfec1a20..69399389f 100644 --- a/lib/bugsnag/integrations/delayed_job.rb +++ b/lib/bugsnag/integrations/delayed_job.rb @@ -10,11 +10,23 @@ module Delayed module Plugins class Bugsnag < ::Delayed::Plugin + ## + # DelayedJob doesn't have an easy way to fetch its version, but we can use + # Gem.loaded_specs to get the version instead + def self.delayed_job_version + ::Gem.loaded_specs['delayed_job'].version.to_s + rescue StandardError + # Explicitly return nil to prevent Rubocop complaining of a suppressed exception + nil + end + callbacks do |lifecycle| lifecycle.around(:invoke_job) do |job, *args, &block| begin ::Bugsnag.configuration.detected_app_type = 'delayed_job' + ::Bugsnag.configuration.runtime_versions['delayed_job'] = delayed_job_version if defined?(::Gem) ::Bugsnag.configuration.set_request_data(:delayed_job, job) + block.call(job, *args) rescue Exception => exception ::Bugsnag.notify(exception, true) do |report| diff --git a/lib/bugsnag/integrations/mailman.rb b/lib/bugsnag/integrations/mailman.rb index b50b56e07..2316c13b8 100644 --- a/lib/bugsnag/integrations/mailman.rb +++ b/lib/bugsnag/integrations/mailman.rb @@ -12,6 +12,7 @@ class Mailman def initialize Bugsnag.configuration.internal_middleware.use(Bugsnag::Middleware::Mailman) Bugsnag.configuration.detected_app_type = "mailman" + Bugsnag.configuration.runtime_versions["mailman"] = ::Mailman::VERSION end ## diff --git a/lib/bugsnag/integrations/shoryuken.rb b/lib/bugsnag/integrations/shoryuken.rb index ccd07c882..4256bba0d 100644 --- a/lib/bugsnag/integrations/shoryuken.rb +++ b/lib/bugsnag/integrations/shoryuken.rb @@ -13,6 +13,7 @@ def initialize Bugsnag.configure do |config| config.detected_app_type = "shoryuken" config.default_delivery_method = :synchronous + config.runtime_versions["shoryuken"] = ::Shoryuken::VERSION end end diff --git a/spec/integrations/mailman_spec.rb b/spec/integrations/mailman_spec.rb index ba8ddad27..71e0e33f9 100644 --- a/spec/integrations/mailman_spec.rb +++ b/spec/integrations/mailman_spec.rb @@ -5,6 +5,7 @@ unless defined?(::Mailman) @mocked_mailman = true class Mailman + VERSION = '9.8.7' end module Kernel alias_method :old_require, :require @@ -19,38 +20,42 @@ def require(path) config = double('mailman-config') allow(Mailman).to receive(:config).and_return(config) expect(config).to receive(:respond_to?).with(:middleware).and_return(true) - middleware = double('mailman-config-middleware') + middleware = spy('mailman-config-middleware') expect(config).to receive(:middleware).and_return(middleware) - expect(middleware).to receive(:add).with(any_args) - #Kick off + # Kick off require './lib/bugsnag/integrations/mailman' + + expect(middleware).to have_received(:add).with(Bugsnag::Mailman) end it "can be called" do - config = double('config') - allow(Bugsnag).to receive(:configuration).and_return(config) - int_middleware = double('internal_middleware') - expect(config).to receive(:internal_middleware).and_return(int_middleware) - expect(int_middleware).to receive(:use).with(Bugsnag::Middleware::Mailman) - expect(config).to receive(:detected_app_type=).with("mailman") - integration = Bugsnag::Mailman.new - mail = double('mail') - expect(config).to receive(:set_request_data).with(:mailman_msg, mail) - expect(mail).to receive(:to_s).and_return(mail) - allow(config).to receive(:clear_request_data) + # Initialising the middleware should set some config options + expect(Bugsnag.configuration.internal_middleware.last).to eq(Bugsnag::Middleware::Mailman) + expect(Bugsnag.configuration.app_type).to eq('mailman') + expect(Bugsnag.configuration.runtime_versions['mailman']).to eq(Mailman::VERSION) + mail = 'To: My Friend; From: Your Pal; Subject: Hello!' exception = RuntimeError.new('oops') - report = double('report') - expect(Bugsnag).to receive(:notify).with(exception, true).and_yield(report) - expect(report).to receive(:severity=).with('error') - expect(report).to receive(:severity_reason=).with({ - :type => Bugsnag::Report::UNHANDLED_EXCEPTION_MIDDLEWARE, - :attributes => Bugsnag::Mailman::FRAMEWORK_ATTRIBUTES - }) - expect{integration.call(mail) {raise exception}}.to raise_error(exception) + + expect { integration.call(mail) { raise exception } }.to raise_error(exception) + + expect(Bugsnag).to have_sent_notification { |payload, headers| + event = get_event_from_payload(payload) + + expect(event['unhandled']).to be(true) + expect(event['severity']).to eq('error') + expect(event['app']['type']).to eq('mailman') + expect(event['device']['runtimeVersions']['mailman']).to eq(Mailman::VERSION) + expect(event['metaData']['mailman']).to eq({ 'message' => mail }) + + expect(event['severityReason']).to eq({ + 'type' => Bugsnag::Report::UNHANDLED_EXCEPTION_MIDDLEWARE, + 'attributes' => { 'framework' => 'Mailman' } + }) + } end after do diff --git a/spec/integrations/shoryuken_spec.rb b/spec/integrations/shoryuken_spec.rb index df4d7f882..2e437460a 100644 --- a/spec/integrations/shoryuken_spec.rb +++ b/spec/integrations/shoryuken_spec.rb @@ -5,7 +5,8 @@ before do unless defined?(::Shoryuken) @mocked_shoryuken = true - class ::Shoryuken + class Shoryuken + VERSION = '1.2.3' end module Kernel alias_method :old_require, :require @@ -27,36 +28,40 @@ def require(path) end it "calls configure when initialised" do - config = double("config") - - expect(config).to receive(:detected_app_type=).with("shoryuken") - expect(config).to receive(:default_delivery_method=).with(:synchronous) - expect(Bugsnag).to receive(:configure).and_yield(config) Bugsnag::Shoryuken.new + + expect(Bugsnag.configuration.app_type).to eq("shoryuken") + expect(Bugsnag.configuration.delivery_method).to eq(:synchronous) + expect(Bugsnag.configuration.runtime_versions["shoryuken"]).to eq(Shoryuken::VERSION) end it "calls correct sequence when called" do - queue = 'queue' - body = 'body' + queue = 'a queue name' + body = 'the body of a queued message' + exception = RuntimeError.new('oops') + + expect { + Bugsnag::Shoryuken.new.call('_', queue, '_', body) { raise exception } + }.to raise_error(exception) + + expect(Bugsnag).to have_sent_notification { |payload, headers| + event = get_event_from_payload(payload) - callbacks = double('callbacks') - expect(callbacks).to receive(:<<) do |func| - report = double('report') - expect(report).to receive(:add_tab).with(:shoryuken, { - queue: queue, - body: body + expect(event['unhandled']).to be(true) + expect(event['severity']).to eq('error') + expect(event['app']['type']).to eq('shoryuken') + expect(event['device']['runtimeVersions']['shoryuken']).to eq(Shoryuken::VERSION) + + expect(event['metaData']['shoryuken']).to eq({ + 'body' => body, + 'queue' => queue, }) - func.call(report) - end - config = double('config') - allow(config).to receive(:detected_app_type=).with("shoryuken") - allow(config).to receive(:default_delivery_method=).with(:synchronous) - allow(config).to receive(:clear_request_data) - expect(Bugsnag).to receive(:before_notify_callbacks).and_return(callbacks) - allow(Bugsnag).to receive(:configure).and_yield(config) - allow(Bugsnag).to receive(:configuration).and_return(config) - shoryuken = Bugsnag::Shoryuken.new - expect { |b| shoryuken.call('_', queue, '_', body, &b )}.to yield_control + + expect(event['severityReason']).to eq({ + 'type' => Bugsnag::Report::UNHANDLED_EXCEPTION_MIDDLEWARE, + 'attributes' => { 'framework' => 'Shoryuken' } + }) + } end after do From b85f5e3c7f3e802cbed2c3e5606de0adb189698a Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Fri, 31 Jul 2020 17:27:58 +0100 Subject: [PATCH 10/16] Update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e4bfd8d6a..ec3a9c888 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ Changelog * The `BUGSNAG_RELEASE_STAGE` environment variable can now be used to set the release stage. Previously this was only used in Rails applications | [#613](https://github.com/bugsnag/bugsnag-ruby/pull/613) +* Add support for runtime versions to Delayed Job, Mailman and Shoryuken integrations + | [#620](https://github.com/bugsnag/bugsnag-ruby/pull/620) + ## Fixes * The `app_type` configuration option should no longer be overwritten by Bugsnag and integrations should set this more consistently From 6be1f85aa5d498c8ec88212cb255a735edc89131 Mon Sep 17 00:00:00 2001 From: Tom Richards Date: Tue, 22 Oct 2019 00:23:46 -0400 Subject: [PATCH 11/16] Remove spec files from bundled gem --- bugsnag.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bugsnag.gemspec b/bugsnag.gemspec index b5274eff3..69ed50c7b 100644 --- a/bugsnag.gemspec +++ b/bugsnag.gemspec @@ -10,7 +10,7 @@ Gem::Specification.new do |s| s.homepage = "https://github.com/bugsnag/bugsnag-ruby" s.licenses = ["MIT"] - s.files = `git ls-files`.split("\n").reject {|file| file.start_with? "example/"} + s.files = `git ls-files -z lib`.split("\x0") s.extra_rdoc_files = [ "LICENSE.txt", "README.md", From 303791d67a7a76dd6f1947c18a248ea407c657e4 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Mon, 3 Aug 2020 10:17:10 +0100 Subject: [PATCH 12/16] Include gemspec and VERSION files These are needed to be able to install the Gem from the built gemfile --- bugsnag.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bugsnag.gemspec b/bugsnag.gemspec index 69ed50c7b..a66724405 100644 --- a/bugsnag.gemspec +++ b/bugsnag.gemspec @@ -10,7 +10,7 @@ Gem::Specification.new do |s| s.homepage = "https://github.com/bugsnag/bugsnag-ruby" s.licenses = ["MIT"] - s.files = `git ls-files -z lib`.split("\x0") + s.files = `git ls-files -z lib bugsnag.gemspec VERSION`.split("\x0") s.extra_rdoc_files = [ "LICENSE.txt", "README.md", From 45b1df6ac2c22f18e1225b1c6fd643405b18708f Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Mon, 3 Aug 2020 14:05:56 +0100 Subject: [PATCH 13/16] Add changelog entry --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec3a9c888..1d85e7df5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ Changelog * Add support for runtime versions to Delayed Job, Mailman and Shoryuken integrations | [#620](https://github.com/bugsnag/bugsnag-ruby/pull/620) +* Reduce the size of the bundled gem + | [#571](https://github.com/bugsnag/bugsnag-ruby/pull/571) + | [t-richards](https://github.com/t-richards) + ## Fixes * The `app_type` configuration option should no longer be overwritten by Bugsnag and integrations should set this more consistently From b32525d71c278be41c3d005728bfc08bdb9fb51d Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Tue, 4 Aug 2020 16:59:34 +0100 Subject: [PATCH 14/16] Delay serialization until delivery in thread queue --- CHANGELOG.md | 3 +++ lib/bugsnag.rb | 30 ++++++++++++++++++---------- lib/bugsnag/delivery/thread_queue.rb | 21 ++++++++++++++++--- 3 files changed, 41 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d85e7df5..525b7a255 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,9 @@ Changelog | [#571](https://github.com/bugsnag/bugsnag-ruby/pull/571) | [t-richards](https://github.com/t-richards) +* Move serialization of Reports onto the background thread when using the thread_queue delivery method + | [#623](https://github.com/bugsnag/bugsnag-ruby/pull/623) + ## Fixes * The `app_type` configuration option should no longer be overwritten by Bugsnag and integrations should set this more consistently diff --git a/lib/bugsnag.rb b/lib/bugsnag.rb index 7d215b3d6..0a7da0bb2 100644 --- a/lib/bugsnag.rb +++ b/lib/bugsnag.rb @@ -303,15 +303,25 @@ def abort_reason(exception, auto_notify) def deliver_notification(report) configuration.info("Notifying #{configuration.notify_endpoint} of #{report.exceptions.last[:errorClass]}") - payload = report_to_json(report) - options = {:headers => report.headers} - - Bugsnag::Delivery[configuration.delivery_method].deliver( - configuration.notify_endpoint, - payload, - configuration, - options - ) + options = { headers: report.headers } + + delivery_method = Bugsnag::Delivery[configuration.delivery_method] + + if delivery_method.respond_to?(:serialize_and_deliver) + delivery_method.serialize_and_deliver( + configuration.notify_endpoint, + proc { report_to_json(report) }, + configuration, + options + ) + else + delivery_method.deliver( + configuration.notify_endpoint, + report_to_json(report), + configuration, + options + ) + end leave_breadcrumb( report.summary[:error_class], @@ -353,7 +363,7 @@ def check_endpoint_setup # encoding errors and redacting metadata according to "meta_data_filters" # # @param report [Report] - # @return string + # @return [String] def report_to_json(report) cleaned = cleaner.clean_object(report.as_json) trimmed = Bugsnag::Helpers.trim_if_needed(cleaned) diff --git a/lib/bugsnag/delivery/thread_queue.rb b/lib/bugsnag/delivery/thread_queue.rb index 1e3649675..50423cedb 100644 --- a/lib/bugsnag/delivery/thread_queue.rb +++ b/lib/bugsnag/delivery/thread_queue.rb @@ -9,8 +9,14 @@ class ThreadQueue < Synchronous class << self ## - # Queues a given payload to be delivered asynchronously. - def deliver(url, body, configuration, options={}) + # Queues a given payload to be delivered asynchronously + # + # @param url [String] + # @param get_payload [Proc] A Proc that will return the payload. + # @param configuration [Bugsnag::Configuration] + # @param options [Hash] + # @return [void] + def serialize_and_deliver(url, get_payload, configuration, options={}) @configuration = configuration start_once! @@ -21,7 +27,16 @@ def deliver(url, body, configuration, options={}) end # Add delivery to the worker thread - @queue.push proc { super(url, body, configuration, options) } + @queue.push(proc do + begin + payload = get_payload.call + rescue StandardError => e + configuration.warn("Notification to #{url} failed, #{e.inspect}") + configuration.warn(e.backtrace) + end + + Synchronous.deliver(url, payload, configuration, options) unless payload.nil? + end) end private From ceb85640a62309491804753558db3be6dfd9a375 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Fri, 7 Aug 2020 17:16:59 +0100 Subject: [PATCH 15/16] Pin Rubocop version more strictly --- Gemfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index 93e21543b..da7d7bf3c 100644 --- a/Gemfile +++ b/Gemfile @@ -32,7 +32,7 @@ group :coverage, optional: true do end group :rubocop, optional: true do - gem 'rubocop', '~> 0.83' + gem 'rubocop', '~> 0.88.0' end group :sidekiq, optional: true do From 819b0a3524339b219e1af5cedfa5d34829fc40f0 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Fri, 7 Aug 2020 17:06:40 +0100 Subject: [PATCH 16/16] Bump version to 6.16.0 --- CHANGELOG.md | 2 +- VERSION | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 525b7a255..3713c9523 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ Changelog ========= -## TBD +## 6.16.0 (12 August 2020) ### Enhancements diff --git a/VERSION b/VERSION index a3748c562..bfec95bf0 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -6.15.0 +6.16.0