Merge pull request #494 from bugsnag/next

v6.9.0

.rubocop_todo.yml

@@ -244,6 +244,7 @@ Lint/RescueException:
     - 'lib/bugsnag/integrations/rake.rb'
     - 'lib/bugsnag/integrations/shoryuken.rb'
     - 'lib/bugsnag/integrations/sidekiq.rb'
+    - 'lib/bugsnag/integrations/delayed_job.rb'
 
 # Offense count: 6
 # Cop supports --auto-correct.

CHANGELOG.md

@@ -1,6 +1,18 @@
 Changelog
 =========
 
+## 6.9.0 (12 Nov 2018)
+
+### Enhancements
+
+* Ensure the correct error handler is used in newer versions of Sidekiq
+  | [#434](https://github.com/bugsnag/bugsnag-ruby/pull/434)
+
+* Rewrite Delayed::Job integration to fix potential issues and add more
+  collected data
+  | [#492](https://github.com/bugsnag/bugsnag-ruby/pull/492)
+  | [Simon Maynard](https://github.com/snmaynard)
+
 ## 6.8.0 (11 Jul 2018)
 
 This release includes general performance improvements to payload trimming and

README.md

@@ -1,8 +1,5 @@
 # Bugsnag exception reporter for Ruby
 [![build status](https://travis-ci.org/bugsnag/bugsnag-ruby.svg?branch=master)](https://travis-ci.org/bugsnag/bugsnag-ruby)
-![Gem Version](https://badge.fury.io/rb/bugsnag.svg)
-[![SemVer](https://api.dependabot.com/badges/compatibility_score?dependency-name=bugsnag&package-manager=bundler&version-scheme=semver)](https://dependabot.com/compatibility-score.html?dependency-name=bugsnag&package-manager=bundler&version-scheme=semver)
-
 
 
 The Bugsnag exception reporter for Ruby gives you instant notification of exceptions thrown from your **[Rails](https://www.bugsnag.com/platforms/rails)**, **Sinatra**, **Rack** or **plain Ruby** app. Any uncaught exceptions will trigger a notification to be sent to your Bugsnag project.

VERSION

@@ -1 +1 @@
-6.8.0
+6.9.0

features/delayed_job.feature

@@ -16,13 +16,14 @@ Scenario: An unhandled RuntimeError sends a report with arguments
   And the request contained the api key "a35a2a72bd230ac0aa0f52715bbdc6aa"
   And the event "unhandled" is true
   And the event "severity" is "error"
-  And the event "context" is "jobs:work"
+  And the event "context" is "TestModel.fail_with_args"
   And the event "severityReason.type" is "unhandledExceptionMiddleware"
   And the event "severityReason.attributes.framework" is "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.attempts" equals "1 / 1"
+  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" 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" is an array with 1 element
@@ -31,14 +32,22 @@ Scenario: An unhandled RuntimeError sends a report with arguments
 Scenario: A handled exception sends a report
   Given I set environment variable "RUBY_VERSION" to "2.5"
   And I start the service "delayed_job"
-  And I run the command "bundle exec rails runner 'TestModel.delay.notify'" on the service "delayed_job"
+  And I run the command "bundle exec rails runner 'TestModel.delay.notify_with_args(\"Test\")'" on the service "delayed_job"
   And I wait for 5 seconds
   Then I should receive a request
   And the request used the Ruby notifier
   And the request used payload v4 headers
   And the request contained the api key "a35a2a72bd230ac0aa0f52715bbdc6aa"
   And the event "unhandled" is false
   And the event "severity" is "warning"
-  And the event "context" is "jobs:work"
+  And the event "context" is "TestModel.notify_with_args"
   And the event "severityReason.type" is "handledException"
   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" 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" is an array with 1 element
+  And the payload field "events.0.metaData.job.payload.args.0" equals "Test"

features/fixtures/delayed_job/app/app/models/test_model.rb

@@ -4,7 +4,7 @@ def self.fail_with_args(a)
     raise "uh oh"
   end
 
-  def self.notify
+  def self.notify_with_args(a)
     Bugsnag.notify(RuntimeError.new("Handled exception"))
   end
 end

features/fixtures/docker-compose.yml

@@ -50,7 +50,7 @@ services:
       - MAZE_APP_VERSION
       - MAZE_AUTO_CAPTURE_SESSIONS
       - MAZE_AUTO_NOTIFY
-      - MAZE_ENDPOINT
+      - BUGSNAG_ENDPOINT
       - MAZE_IGNORE_CLASS
       - MAZE_IGNORE_MESSAGE
       - MAZE_META_DATA_FILTERS

features/sidekiq.feature

@@ -18,6 +18,7 @@ Scenario Outline: An unhandled RuntimeError sends a report
   And the request contained the api key "a35a2a72bd230ac0aa0f52715bbdc6aa"
   And the event "unhandled" is true
   And the event "severity" is "error"
+  And the event "context" is "UnhandledError@default"
   And the event "severityReason.type" is "unhandledExceptionMiddleware"
   And the event "severityReason.attributes.framework" is "Sidekiq"
   And the exception "errorClass" equals "RuntimeError"
@@ -62,6 +63,7 @@ Scenario Outline: A handled RuntimeError can be notified
   And the request used payload v4 headers
   And the request contained the api key "a35a2a72bd230ac0aa0f52715bbdc6aa"
   And the event "unhandled" is false
+  And the event "context" is "HandledError@default"
   And the event "severity" is "warning"
   And the event "severityReason.type" is "handledException"
   And the exception "errorClass" equals "RuntimeError"
@@ -93,4 +95,4 @@ Scenario Outline: A handled RuntimeError can be notified
   | 2.5          | ~> 2            | false              |
   | 2.5          | ~> 3            | true               |
   | 2.5          | ~> 4            | true               |
-  | 2.5          | ~> 5            | true               |
+  | 2.5          | ~> 5            | true               |

features/steps/ruby_notifier_steps.rb

@@ -7,23 +7,6 @@
   }
 end
 
-When("I wait for the app to respond on port {string}") do |port|
-  max_attempts = ENV.include?('MAX_MAZE_CONNECT_ATTEMPTS')? ENV['MAX_MAZE_CONNECT_ATTEMPTS'].to_i : 10
-  attempts = 0
-  up = false
-  until (attempts >= max_attempts) || up
-    attempts += 1
-    begin
-      uri = URI("http://localhost:#{port}/")
-      response = Net::HTTP.get_response(uri)
-      up = (response.code == "200")
-    rescue EOFError
-    end
-    sleep 1
-  end
-  raise "App not ready in time!" unless up
-end
-
 When("I navigate to the route {string} on port {string}") do |route, port|
   steps %Q{
     When I open the URL "http://localhost:#{port}#{route}"
@@ -46,22 +29,6 @@
     When I set environment variable "#{env_var}" to "#{credentials}@#{current_ip}:#{MOCK_API_PORT}"
   }
 end
-
-Then("the request used payload v4 headers") do
-  steps %Q{
-    Then the "bugsnag-api-key" header is not null
-    And the "bugsnag-payload-version" header equals "4.0"
-    And the "bugsnag-sent-at" header is a timestamp
-  }
-end
-
-Then("the request contained the api key {string}") do |api_key|
-  steps %Q{
-    Then the "bugsnag-api-key" header equals "#{api_key}"
-    And the payload field "apiKey" equals "#{api_key}"
-  }
-end
-
 Then("the request used the Ruby notifier") do
   bugsnag_regex = /^http(s?):\/\/www.bugsnag.com/
   steps %Q{

features/support/env.rb

@@ -21,11 +21,11 @@ def output_logs
 def install_fixture_gems
   gem_dir = File.expand_path('../../../', __FILE__)
   Dir.chdir(gem_dir) do
-    `rm bugsnag-*.gem`
+    `rm bugsnag-*.gem` unless Dir.glob('bugsnag-*.gem').empty?
     `gem build bugsnag.gemspec`
-    Dir.foreach('features/fixtures') do |entry|
+    Dir.entries('features/fixtures').reject { |entry| ['.', '..'].include?(entry) }.each do |entry|
       target_dir = "features/fixtures/#{entry}"
-      if File.directory? (target_dir)
+      if File.directory?(target_dir)
         `cp bugsnag-*.gem #{target_dir}`
         `gem unpack #{target_dir}/bugsnag-*.gem --target #{target_dir}/temp-bugsnag-lib`
       end

lib/bugsnag.rb

@@ -27,6 +27,7 @@
 require "bugsnag/middleware/rake"
 require "bugsnag/middleware/callbacks"
 require "bugsnag/middleware/classify_error"
+require "bugsnag/middleware/delayed_job"
 
 module Bugsnag
   LOCK = Mutex.new

lib/bugsnag/integrations/delayed_job.rb

@@ -5,92 +5,35 @@
   raise LoadError, "bugsnag requires delayed_job > 3.x"
 end
 
-unless defined? Delayed::Plugins::Bugsnag
-  module Delayed
-    module Plugins
-      class Bugsnag < Plugin
-
-        FRAMEWORK_ATTRIBUTES = {
-          :framework => "DelayedJob"
-        }
-
-        module Notify
-          def error(job, error)
-            overrides = {
-              :job => {
-                :class => job.class.name,
-                :id => job.id,
-              },
-              :severity_reason => {
-                :type => ::Bugsnag::Report::UNHANDLED_EXCEPTION_MIDDLEWARE,
-                :attributes => FRAMEWORK_ATTRIBUTES,
-              },
-            }
-            if job.respond_to?(:queue) && (queue = job.queue)
-              overrides[:job][:queue] = queue
-            end
-            if job.respond_to?(:attempts)
-              max_attempts = (job.respond_to?(:max_attempts) && job.max_attempts) || Delayed::Worker.max_attempts
-              overrides[:job][:attempts] = "#{job.attempts + 1} / #{max_attempts}"
-              # +1 as "attempts" is zero-based and does not include the current failed attempt
-            end
-            if payload = job.payload_object
-              p = {
-                :class => payload.class.name,
-              }
-              p[:id]           = payload.id           if payload.respond_to?(:id)
-              p[:display_name] = payload.display_name if payload.respond_to?(:display_name)
-              p[:method_name]  = payload.method_name  if payload.respond_to?(:method_name)
-
-              if payload.respond_to?(:args)
-                p[:args] = payload.args
-              elsif payload.respond_to?(:to_h)
-                p[:args] = payload.to_h
-              end
-
-              if payload.is_a?(::Delayed::PerformableMethod) && (object = payload.object)
-                p[:object] = {
-                  :class => object.class.name,
-                }
-                p[:object][:id] = object.id if object.respond_to?(:id)
-              end
-              add_active_job_details(p, payload)
-              overrides[:job][:payload] = p
-            end
-
-            ::Bugsnag.notify(error, true) do |report|
+::Bugsnag.configuration.internal_middleware.use(::Bugsnag::Middleware::DelayedJob)
+
+module Delayed
+  module Plugins
+    class Bugsnag < ::Delayed::Plugin
+      callbacks do |lifecycle|
+        lifecycle.around(:invoke_job) do |job, *args, &block|
+          begin
+            ::Bugsnag.configuration.app_type = 'delayed_job'
+            ::Bugsnag.configuration.set_request_data(:delayed_job, job)
+            block.call(job, *args)
+          rescue Exception => exception
+            ::Bugsnag.notify(exception, true) do |report|
               report.severity = "error"
               report.severity_reason = {
                 :type => ::Bugsnag::Report::UNHANDLED_EXCEPTION_MIDDLEWARE,
-                :attributes => FRAMEWORK_ATTRIBUTES
+                :attributes => {
+                  :framework => "DelayedJob"
+                }
               }
-              report.meta_data.merge! overrides
-            end
-
-            super if defined?(super)
-          end
-
-          def add_active_job_details(p, payload)
-            if payload.respond_to?(:job_data) && payload.job_data.respond_to?(:[])
-              [:job_class, :arguments, :queue_name, :job_id].each do |key|
-                if (value = payload.job_data[key.to_s])
-                  p[key] = value
-                end
-              end
             end
-          end
-        end
-
-        callbacks do |lifecycle|
-          lifecycle.before(:invoke_job) do |job|
-            payload = job.payload_object
-            payload = payload.object if payload.is_a? Delayed::PerformableMethod
-            payload.extend Notify
+            raise exception
+          ensure
+            ::Bugsnag.configuration.clear_request_data
           end
         end
       end
     end
   end
-
-  Delayed::Worker.plugins << Delayed::Plugins::Bugsnag
 end
+
+Delayed::Worker.plugins << Delayed::Plugins::Bugsnag

lib/bugsnag/integrations/sidekiq.rb

@@ -5,9 +5,11 @@ module Bugsnag
   # Extracts and attaches Sidekiq job and queue information to an error report
   class Sidekiq
 
-    FRAMEWORK_ATTRIBUTES = {
-      :framework => "Sidekiq"
-    }
+    unless const_defined?(:FRAMEWORK_ATTRIBUTES)
+      FRAMEWORK_ATTRIBUTES = {
+        :framework => "Sidekiq"
+      }
+    end
 
     def initialize
       Bugsnag.configuration.internal_middleware.use(Bugsnag::Middleware::Sidekiq)
@@ -19,31 +21,44 @@ def call(worker, msg, queue)
       begin
         # store msg/queue in thread local state to be read by Bugsnag::Middleware::Sidekiq
         Bugsnag.configuration.set_request_data :sidekiq, { :msg => msg, :queue => queue }
-
         yield
       rescue Exception => ex
-        raise ex if [Interrupt, SystemExit, SignalException].include? ex.class
-        Bugsnag.notify(ex, true) do |report|
-          report.severity = "error"
-          report.severity_reason = {
-            :type => Bugsnag::Report::UNHANDLED_EXCEPTION_MIDDLEWARE,
-            :attributes => FRAMEWORK_ATTRIBUTES
-          }
-        end
+        self.class.notify(ex) unless self.class.sidekiq_supports_error_handlers
         raise
       ensure
         Bugsnag.configuration.clear_request_data
       end
     end
+
+    def self.notify(exception)
+      return if [Interrupt, SystemExit, SignalException].include? exception.class
+      Bugsnag.notify(exception, true) do |report|
+        report.severity = "error"
+        report.severity_reason = {
+          :type => Bugsnag::Report::UNHANDLED_EXCEPTION_MIDDLEWARE,
+          :attributes => FRAMEWORK_ATTRIBUTES
+        }
+      end
+    end
+
+    def self.sidekiq_supports_error_handlers
+      Gem::Version.new(::Sidekiq::VERSION) >= Gem::Version.new('3.0.0')
+    end
+
+    def self.configure_server(server)
+      if Bugsnag::Sidekiq.sidekiq_supports_error_handlers
+        server.error_handlers << proc do |ex, _context|
+          Bugsnag::Sidekiq.notify(ex)
+        end
+      end
+
+      server.server_middleware do |chain|
+        chain.add ::Bugsnag::Sidekiq
+      end
+    end
   end
 end
 
 ::Sidekiq.configure_server do |config|
-  config.server_middleware do |chain|
-    if Gem::Version.new(Sidekiq::VERSION) >= Gem::Version.new('3.3.0')
-      chain.prepend ::Bugsnag::Sidekiq
-    else
-      chain.add ::Bugsnag::Sidekiq
-    end
-  end
+  Bugsnag::Sidekiq.configure_server(config)
 end