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

Implemented unhandled error reporting for orphan fibers #2868

Merged
merged 3 commits into from
Mar 19, 2022

Conversation

djspiewak
Copy link
Member

@djspiewak djspiewak commented Mar 10, 2022

This enriches CallbackStack with the ability to report when no callbacks were invoked. Note that this isn't entirely perfect, since a fiber which joins and is later canceled within the join will empty out its callback, but the joined fiber may finish with an error at the exact same moment. If this happens and the joining fiber is the only fiber, the error would have been side-channeled but might end up being missed. I think this is a fairly reasonable tradeoff though, at least for the moment. The alternative is that we force a memory barrier in this case, which I think is a bit too much deoptimization for this edge case.

Closes #2842

@djspiewak
Copy link
Member Author

Test failures here are legit. I think it's a race condition in how I setup the test which is being surfaced randomly by the TestContext shuffling.

@djspiewak djspiewak force-pushed the bug/report-fiber-failure branch from 562856a to 3f1dfb5 Compare March 14, 2022 04:57
@djspiewak djspiewak added this to the v3.3.8 milestone Mar 18, 2022
@djspiewak djspiewak merged commit 1a0893c into typelevel:series/3.3.x Mar 19, 2022
@RobertoUa
Copy link

Is it possible to override reportFailure in the default ctx? For instance, I want to report an exception to Sentry.

@armanbilge
Copy link
Member

@RobertoUa I think that's what #2869 is doing, which will land in 3.4.0.

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.

4 participants