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

Fix deprecation warning from Sidekiq error handler #796

Merged
merged 1 commit into from
Dec 18, 2023
Merged

Fix deprecation warning from Sidekiq error handler #796

merged 1 commit into from
Dec 18, 2023

Conversation

fukayatsu
Copy link
Contributor

@fukayatsu fukayatsu commented Oct 16, 2023

Goal

Fix deprecation warning: DEPRECATION: Sidekiq exception handlers now take three arguments...

2023-10-16T01:33:49.575Z pid=3856681 tid=2am91 INFO: DEPRECATION: Sidekiq exception handlers now take three arguments, see #<Proc:0x00007f51009a97c0 /home/me/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/bugsnag-6.26.0/lib/bugsnag/integrations/sidekiq.rb:53>

NOTE: This warning is shown when using sidekiq main branch.

This warning is related to

Design

sidekiq/sidekiq#6051 (comment).

Changeset

I added a third optional argument to sidekiq's error handler.

Testing

  1. docker run --rm -it -p 16379:6379 redis:7
  2. Save below script as test.rb and run ruby test.rb
if $0 == __FILE__
  require 'bundler/inline'

  gemfile do
    source 'https://rubygems.org'
    gem 'sidekiq', github: 'sidekiq/sidekiq', branch: 'main'

    # Deprecation warning shown
    # gem 'bugsnag'

    # No deprecation warning
    gem 'bugsnag', github: 'fukayatsu/bugsnag-ruby', branch: 'fix-sidekiq-deprecation-warning'
  end

  require 'sidekiq/cli'
  require 'bugsnag'

  ::Sidekiq.configure_server do |config|
    config.redis = { url: 'redis://127.0.0.1:16379' }
    Bugsnag::Sidekiq.configure_server(config)
  end


  sidekiq_thread = Thread.start do
    cli = Sidekiq::CLI.instance
    cli.parse(['-r', File.expand_path(__FILE__)])
    cli.run
  end
  sleep 1
  Sidekiq::Client.push('class' => 'HardJob', 'args' => ['foo', 1])

  sidekiq_thread.join(1)
else
  class HardJob
    include Sidekiq::Job

    def perform(name, count)
      puts name, count
      raise 'error'
    end
  end
end

@clr182
Copy link

clr182 commented Oct 16, 2023

Hi @fukayatsu
Thanks for reaching out and creating a PR for this deprecation warning. We will review this when priority allows

@clr182 clr182 added bug Confirmed bug backlog We hope to fix this feature/bug in the future labels Oct 16, 2023
@stevenharman
Copy link
Contributor

Confirming that this does resolve the deprecation warnings for Sidekiq 7.2+. We'd love to get this into a release so we can stop spamming our logs with deprecation warnings.

@imjoehaines imjoehaines changed the base branch from master to next December 18, 2023 10:03
Copy link
Contributor

@imjoehaines imjoehaines left a comment

Choose a reason for hiding this comment

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

Thank you, @fukayatsu!

@imjoehaines imjoehaines merged commit ee7ffbd into bugsnag:next Dec 18, 2023
@imjoehaines imjoehaines mentioned this pull request Jan 8, 2024
@fukayatsu fukayatsu deleted the fix-sidekiq-deprecation-warning branch January 15, 2024 06:08
@mclack mclack added released This feature/bug fix has been released and removed backlog We hope to fix this feature/bug in the future labels Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bug released This feature/bug fix has been released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants