-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Do not report exceptions raised in CoroutineDispatcher.dispatch as in… #4181
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a nice start, though a review of the other places where dispatch
, isDispatchNeeded
, and/or intercepted()
happens shows that it's in general very easy to leave kotlinx-coroutines
in an incorrect state if dispatchers throw: #4209
If you're up for it, we can try to provide sensible behavior for dispatcher errors in all places as part of this pull request, or we can limit the scope to that one specific crash and leave the more robust and general solution for another time.
kotlinx-coroutines-core/common/src/internal/DispatchedContinuation.kt
Outdated
Show resolved
Hide resolved
…h as internal errors Prevent reporting DispatchException itself
…h as internal errors Fix test: add new exception to the list of known exceptions
33d85f4
to
7d0cbc4
Compare
Historical note: we've discussed it internally and decided that #4209 is out of this PR's scope. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job!
Please address/accept my comment and it is good to go.
Co-authored-by: Vsevolod Tolstopyatov <[email protected]>
…ternal errors
Fix #4091