Skip to content

ActiveJob missing error reporting on retries #2597

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

Closed
solnic opened this issue Apr 14, 2025 · 3 comments · Fixed by #2598
Closed

ActiveJob missing error reporting on retries #2597

solnic opened this issue Apr 14, 2025 · 3 comments · Fixed by #2598

Comments

@solnic
Copy link
Collaborator

solnic commented Apr 14, 2025

As described here hooking into perform_now is not enough to capture all exceptions that happen during execution of a job because retry logic is implemented as a rescue handler, which simply bypasses entire code flow from the perform_now.

It turned out we need to patch retry_job in order to be able to report exceptions on each retry but this is needed only up until Rails 8.0.2 (included). Luckily latest Rails' version of ActiveJob uses ActiveSupport.error.reporter to report an exception before retrying a job, which means we can provide our own error subscriber and no patching will be needed for Rails > 8.0.2 (not released in the moment of writing this but it's in main).

It turned out there's enqueue_retry.active_job that we can simply rely on for this.

@sfcgeorge
Copy link

As a note I still see Sentry in backtrace frames when using retry_on. It even gets as far as SentryReporter.record but then no error shows up.

I noticed one difference:

  • Without retry_on the error gets reported (with all the lovely extra data from the integration) as "unhandled"
  • With retry_on there's no error, if I manually Sentry.capture_exception(e) it gets reported but not as "unhandled"

I don't know how unhandled works (maybe a red herring) but I wonder if that's what's stopping it from being automatically reported.

Else yeah the Rails 8 thing sounds good, for earlier versions maybe the enqueue_retry.active_job AS notification.

@solnic
Copy link
Collaborator Author

solnic commented Apr 30, 2025

As a note I still see Sentry in backtrace frames when using retry_on. It even gets as far as SentryReporter.record but then no error shows up.

@sfcgeorge this is because our perform_now patch always calls SentryReporter.record as it creates a transaction with spans and it may capture and report an exception depending on the active_job_report_after_job_retries setting - if it's false and retry_on is not set then it will report, otherwise it won't which is precisely what the related PR fixes.

With retry_on there's no error, if I manually Sentry.capture_exception(e) it gets reported but not as "unhandled"

Yes this makes sense because you handle the exception and capture it manually.

I don't know how unhandled works (maybe a red herring) but I wonder if that's what's stopping it from being automatically reported.

handled vs unhandled just means that Sentry marks an exception as unhandled if it was the SDK that captured and reported an exception, meaning that your app actually crashes without your own explicit code that handles the crash. It is not related to the issue #2598 is fixing though.

Else yeah the Rails 8 thing sounds good, for earlier versions maybe the enqueue_retry.active_job AS notification.

I missed that there's enqueue_retry.active_job somehow, thank you! I've reworked #2598 to use this and tests are passing! 🎉

@sfcgeorge
Copy link

@solnic thanks for the great explanation and fix! 🎉

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Apr 30, 2025
solnic added a commit that referenced this issue May 8, 2025
* Set active_job_report_after_job_retries to true by default

Refs #2598
Refs #2597

* Use test jobs defined in support file

* Introduce active_job_report_on_retry_error

* Fix spec

* Adjust spec skipping logic

* Update CHANGELOG.md

* Update CHANGELOG.md
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.

4 participants