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

implement report_after_job_retries in ActiveJob adapter #2499

Open
modosc opened this issue Dec 17, 2024 · 3 comments · May be fixed by #2500
Open

implement report_after_job_retries in ActiveJob adapter #2499

modosc opened this issue Dec 17, 2024 · 3 comments · May be fixed by #2500
Assignees

Comments

@modosc
Copy link
Contributor

modosc commented Dec 17, 2024

we are using sentry-rails to report errors from ActiveJob (which is running on sidekiq). we want the ability to only report on errors when retries are exhausted. there doesn't seem to be a way to do this directly in the ActiveJob integration (sentry-sidekiq, sentry-delayed_job, and sentry-resque all support this configuration option).

is it possible to implement this directly in the ActiveJob adapter?

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Dec 17, 2024
@sl0thentr0py
Copy link
Member

Yes, we should implement :report_after_job_retries for active job too, feel free to make a PR if you want it quicker!

@modosc
Copy link
Contributor Author

modosc commented Dec 18, 2024

@sl0thentr0py i gave it a shot in #2500 but ran into several issues:

  1. tests work locally but not in CI or in CI but not locally, similar to Fix ActiveJob tests for Rails 7.2 and 8.0 #2487
  2. rails 5.0 didn't support retry_on and i'm unclear on how you expect that to be handled (print a deprecation warning? throw an error?).

anyway - i'm closing the pr out but i'll make sure the branch still exists. i think the approach is valid (and it seems to work) but i can't spend anymore time figuring out the tests.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Dec 18, 2024
@sl0thentr0py
Copy link
Member

@modosc thanks a lot! I will reopen the PR and get it into a mergeable state myself. You don't have to do further work on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

2 participants