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

Add test case to demonstrate that recurring tasks don't report exceptions to the error reporter #219

Conversation

fractaledmind
Copy link

@rosa: I don't yet have a fix for this, but I'm hoping that a test case will help you to help me fix this.

@fractaledmind
Copy link
Author

Also of note, if I run a ErroringJob.perform_later from the console, that exception is not reported either

@rosa
Copy link
Member

rosa commented Sep 16, 2024

Sorry for totally ignoring this one! I don't know why I thought it was incomplete when you first opened it, so decided to wait until you finished it, and then completely forgot! I've come back to it now and I don't know why I thought it was incomplete 😳 It's not.

This behaviour was initially intended. I noticed some error handling libraries like sentry-rails or rollbar-gem that hook onto Active Job directly, so if I also reported this, we'd end up with duplicate error reports 🤔 But I'm not quite sure this is the right thing, perhaps it should be optional (via a new option, on_job_error like on_thread_error, or perhaps using on_thread_error for this)... Or maybe it should just be documented, which is what #139 intended to do, but that one was never completed 🤔

@rosa
Copy link
Member

rosa commented Sep 16, 2024

Ok, I went with documenting this in the end, for now, but perhaps this should change in the future to a config option or something else 🤔

@rosa rosa closed this Sep 16, 2024
@fractaledmind
Copy link
Author

Yeah, probably the right place to start. Annoying that major players don't use the Rails error subscriber now and force these kind of pragmatics on us. But it is what it is

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

Successfully merging this pull request may close these issues.

2 participants