Premature notification in Airbrake::Sidekiq::RetryableJobsFilter

[dgsangoma]

Premature notification in Airbrake::Sidekiq::RetryableJobsFilter

[dgsangoma]

I'll work on putting together a minimal setup that is able to reproduce the issue.

[kyrylo]

We read max_retries from Sidekiq class directly. If that's not configured, we use Sidekiq::JobRetry::DEFAULT_MAX_RETRY_ATTEMPTS:

airbrake/lib/airbrake/sidekiq/retryable_jobs_filter.rb

Line 49 in 6ca4b60

@max_retries ||= ::Sidekiq.options[:max_retries] || DEFAULT_MAX_RETRY_ATTEMPTS

It's quite possible that we don't take into account sidekiq_options retry: 60.
Thanks for the report!

[kyrylo]

It's quite possible that we don't take into account sidekiq_options retry: 60.

I couldn't confirm this. In your sample params, I can see the correct retry value ("retry": 3,). After reading the code and testing, I can see that we do respect the per-job retry option.

airbrake/lib/airbrake/sidekiq/retryable_jobs_filter.rb

Lines 41 to 42 in 6ca4b60

	elsif job['retry'].is_a?(Integer)
	job['retry']

I used the following code to test:

require 'sidekiq'
require 'airbrake'

::Sidekiq.options[:max_retries] = 1

Airbrake.configure do |c|
  c.project_id = 1
  c.project_key = 'asd'
  c.logger = Logger.new(STDOUT)
  c.performance_stats = false
  c.remote_config = false
end

Airbrake.add_filter(Airbrake::Sidekiq::RetryableJobsFilter.new)

class TestWorker
  include Sidekiq::Job
  sidekiq_options retry: 3
  sidekiq_retry_in do |count|
    5
  end

  def perform(name)
    p "Hey #{name}"
    raise
  end
end

TestWorker.perform_async('Joe')

Could you please provide a demo worker that demonstrates the problem that you mentioned?

[kyrylo]

Gentle ping @dgsangoma.

[dgsangoma]

I've made a small test app to try to reproduce the issue, but am struggling to reliably reproduce the issue in the test app.

[kyrylo]

Thanks! Could share the app and the instructions on how to run it? I can clone it and run it multiple times to see how it fails.

[dgsangoma]

I should clarify that I'm not sure I've managed to reproduce the issue yet in a small test Rails app, so there's some key detail about our production app that's missing from the test app that's required to reproduce the bug. I can confirm we're still seeing it intermittently in production on airbrake 13.0.3 / airbrake-ruby 6.2.0. I'll need to keep trying things to see if I can uncover what's missing that's required unless you have any suggestions for getting more insightful info out of our production app.

[kyrylo]

I don't really have any debugging tips for you. By manual code inspection, it should work, but I am not a Sidekiq user, and I don't have a profound knowledge of it. Maybe you defined some sort of Sidekiq config that alters the behavior.

If you take a look at the example I shared above, does it look the same as your workers?

[dgsangoma]

My worker is part of a large Rails app as opposed to a single file standalone worker like the example above. Global sidekiq configuration is managed in our app via a yml config file. We have require "airbrake/sidekiq" at the top of our Airbrake initializer. We've defined the sidekiq_retries_exhausted callback to do additional failure handling on the final retry. I'm sure there's other differences that are less obvious.

I guess I'll continue to experiment and maybe eventually I'll stumble onto the bug.

[kyrylo]

Thanks for the update. Maybe sidekiq_retries_exhausted is the clue. For now, I am going to close this issue because I cannot take any action here without a reproducible example, but please do ping me in case the situation changes 🙏