Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unwraps Sidekiq::JobRetry::Handled errors #185

Merged
merged 2 commits into from
Jul 28, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,10 @@ class FailingWorker
end
```

##### Sidekiq Retries

By default, Raygun4Ruby will unwrap `Sidekiq::JobRetry::Handled` exceptions and report the original error via `Exception#cause`. If you would prefer not to hear about retries, you can set `config.track_retried_sidekiq_jobs` to `false` in your Raygun configuration.

### Other Configuration options

For a complete list of configuration options see the [configuration.rb](https://github.com/MindscapeHQ/raygun4ruby/blob/master/lib/raygun/configuration.rb) file
Expand Down
6 changes: 5 additions & 1 deletion lib/raygun/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ def self.proc_config_option(name)
# Should we register an error handler with [Rails' built in API](https://edgeguides.rubyonrails.org/error_reporting.html)
config_option :register_rails_error_handler

# Should we track jobs that are retried in Sidekiq (ones that raise Sidekiq::JobRetry::Handled). Set to "false" to ignore.
config_option :track_retried_sidekiq_jobs

# Exception classes to ignore by default
IGNORE_DEFAULT = ['ActiveRecord::RecordNotFound',
'ActionController::RoutingError',
Expand Down Expand Up @@ -143,7 +146,8 @@ def initialize
breadcrumb_level: :info,
record_raw_data: false,
send_in_background: false,
error_report_send_timeout: 10
error_report_send_timeout: 10,
track_retried_sidekiq_jobs: true

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is true a good default value for this? I am not sure how annoying it is to have retries reported, but it is good that it can be disabled if required.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so - rationale being it's better to log more errors initially and then turn off any that are noisy rather than silently swallow them

)
end

Expand Down
10 changes: 10 additions & 0 deletions lib/raygun/sidekiq.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,16 @@ def self.call(exception, context_hash = {}, config = nil)
},
tags: ['sidekiq']
}

if exception.is_a?(Sidekiq::JobRetry::Handled) && exception.cause
if Raygun.configuration.track_retried_sidekiq_jobs
data.merge!(sidekiq_retried: true)
exception = exception.cause
else
return false
end
end

if exception.instance_variable_defined?(:@__raygun_correlation_id) && correlation_id = exception.instance_variable_get(:@__raygun_correlation_id)
data.merge!(correlation_id: correlation_id)
end
Expand Down
47 changes: 47 additions & 0 deletions test/unit/sidekiq_failure_test.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
require_relative "../test_helper.rb"

require "sidekiq"

# Convince Sidekiq it's on a server :)
module Sidekiq
class << self
Expand All @@ -15,6 +16,8 @@ def server?
class SidekiqFailureTest < Raygun::UnitTest

def setup
require "sidekiq/job_retry"

super
Raygun.configuration.send_in_background = false

Expand All @@ -32,6 +35,50 @@ def test_failure_backend_appears_to_work
assert response && response.success?, "Expected success, got #{response.class}: #{response.inspect}"
end

def test_failure_backend_unwraps_retries
WebMock.reset!

unwrapped_stub = stub_request(:post, 'https://api.raygun.com/entries').
with(body: /StandardError/).
to_return(status: 202)

begin
raise StandardError.new("Some job in Sidekiq failed, oh dear!")
rescue
raise Sidekiq::JobRetry::Handled
end

rescue Sidekiq::JobRetry::Handled => e

response = Raygun::SidekiqReporter.call(
e,
{ sidekick_name: "robin" },
{} # config
)

assert_requested unwrapped_stub
assert response && response.success?, "Expected success, got #{response.class}: #{response.inspect}"
end

def test_failured_backend_ignores_retries_if_configured
Raygun.configuration.track_retried_sidekiq_jobs = false

begin
raise StandardError.new("Some job in Sidekiq failed, oh dear!")
rescue
raise Sidekiq::JobRetry::Handled
end

rescue Sidekiq::JobRetry::Handled => e

refute Raygun::SidekiqReporter.call(e,
{ sidekick_name: "robin" },
{} # config
)
ensure
Raygun.configuration.track_retried_sidekiq_jobs = true
end

# See https://github.com/MindscapeHQ/raygun4ruby/issues/183
# (This is how Sidekiq pre 7.1.5 calls error handlers: https://github.com/sidekiq/sidekiq/blob/1ba89bbb22d2fd574b11702d8b6ed63ae59e2256/lib/sidekiq/config.rb#L269)
def test_failure_backend_appears_to_work_without_config_argument
Expand Down