Skip to content

Improve coroutine exception handling logic #1199

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

Merged
merged 2 commits into from
May 24, 2019
Merged

Conversation

qwwdfsad
Copy link
Member

@qwwdfsad qwwdfsad commented May 16, 2019

  • Wrap exceptions into CE if parent cancels its child. This is possible because now we can be sure that no one is invoking 'cancel(Throwable)', thus if parent cancels its children, an exception is definitely handled.
  • This changes allows exceptions in hierarchies to be handled only once even when coroutines in the hierarchy are capable of handling exceptions
  • Breaking change: CompletableJob.completeExceptionally(exception) uses its parameter as additional debug information for children ('cause' of CE), thus changing the result of its children. For example, if CJ was a parent of CompleteableDeferred and CJ was completed exceptionally with exception E1, now CD.await() will throw CE instead of E1
  • Recursively check whether parent handles exception to avoid duplicates reporting when Job() is present in the hierarchy

Fixes for #689

@qwwdfsad qwwdfsad force-pushed the exception-handling branch from d3fd575 to 447fc97 Compare May 17, 2019 08:32
  * Wrap exceptions into CE if parent cancels its child. This is possible because now we can be sure that no one is invoking `cancel(Throwable)`, thus if parent cancels its children, an exception is definitely handled.
  * Now exception is handled only once

First part of fix for #689
@qwwdfsad qwwdfsad force-pushed the exception-handling branch from 447fc97 to 0f54026 Compare May 17, 2019 08:55
@qwwdfsad qwwdfsad requested a review from elizarov May 20, 2019 16:43
@qwwdfsad qwwdfsad marked this pull request as ready for review May 20, 2019 16:43
Copy link
Contributor

@elizarov elizarov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to go with one small improvement

…s reporting when Job() is present in the hierarchy

Fixes #689
@qwwdfsad qwwdfsad force-pushed the exception-handling branch from e81a6d8 to 0285057 Compare May 24, 2019 08:49
@qwwdfsad qwwdfsad merged commit 8fbd8b7 into develop May 24, 2019
@qwwdfsad qwwdfsad deleted the exception-handling branch May 24, 2019 09:15
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