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

Sentry::Cron::MonitorCheckIns breaks ActiveJob keyword arguments #2198

Closed
jamesbebbington opened this issue Dec 14, 2023 · 5 comments · Fixed by #2199
Closed

Sentry::Cron::MonitorCheckIns breaks ActiveJob keyword arguments #2198

jamesbebbington opened this issue Dec 14, 2023 · 5 comments · Fixed by #2199
Assignees

Comments

@jamesbebbington
Copy link

Issue Description

It appears that Sentry::Cron::MonitorCheckIns::Patch#perform is not forwarding keyword arguments.

Reproduction Steps

include Sentry::Cron::MonitorCheckIns in an ActiveJob, and attempt to use keyword arguments.

Expected Behavior

Given a job that doesn't include Sentry::Cron::MonitorCheckIns:

class CronJob < ApplicationJob

  def work(a, b, c)
    puts "a: #{a}, b: #{b}, c: #{c}"
  end

  def perform(a, b = 42, c: 99)
    work(a, b, c)
  end

end

it behaves as expected:

3.2.2 :001 > CronJob.perform_now(2, 43)
a: 2, b: 43, c: 99
3.2.2 :002 > CronJob.perform_now(2, 43, c: 100)
a: 2, b: 43, c: 100

Actual Behavior

Given a job that includes Sentry::Cron::MonitorCheckIns:

class CronJob < ApplicationJob

  include Sentry::Cron::MonitorCheckIns

  sentry_monitor_check_ins

  def work(a, b, c)
    puts "a: #{a}, b: #{b}, c: #{c}"
  end

  def perform(a, b = 42, c: 99)
    work(a, b, c)
  end

end

Passing a keyword argument raises an ArgumentError:

3.2.2 :001 > CronJob.perform_now(2, 43)
a: 2, b: 43, c: 99
3.2.2 :002 > CronJob.perform_now(2, 43, c: 100)
ArgumentError (wrong number of arguments (given 3, expected 1..2))

Ruby Version

3.2.2

SDK Version

5.15.0

Integration and Its Version

Rails 7.1.2, GoodJob 3.21.5

Sentry Config

No response

@sl0thentr0py
Copy link
Member

uhh sorry, should have written a better spec, I thought I tested this.

@jamesbebbington
Copy link
Author

No probs, many thanks for the quick fix!

@jamesbebbington
Copy link
Author

Sorry, has something gone wonky with getsentry-bot? I see it committed release: 5.15.1 yesterday, but the latest release is still showing as 5.15.0.

@sl0thentr0py
Copy link
Member

@jamesbebbington just released

@jamesbebbington
Copy link
Author

Oh I see, it needed a manual merge. Many thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants