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.

Sign up for GitHub

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

Jump to bottom

Handle exception only once in case of Job in the coroutines hierarchy #689

Closed
qwwdfsad opened this issue Oct 9, 2018 · 4 comments
Closed

Handle exception only once in case of Job in the coroutines hierarchy #689

qwwdfsad opened this issue Oct 9, 2018 · 4 comments
Labels
design

Comments

@qwwdfsad
Copy link
Collaborator

qwwdfsad commented Oct 9, 2018

Current exception handling mechanism can report the same exception multiple times.

For example, the exception is reported by every child if their parent can't handle it:

val job = Job()
launch(job) {
    delay(Long.MAX_VALUE)
}

launch(job) {
    throw IOException()
}
job.join()

IOException will be reported twice, though only the second coroutine is responsible for it.

Second problem is even less intuitive: when attaching to already failed job, exception always will be reported. This is the case from #686.
@qwwdfsad qwwdfsad added the design label Oct 9, 2018
@qwwdfsad qwwdfsad changed the title Handle exception only once in case of Job in the coroutines hierarchy Handle exception only once in case of Job in the coroutines hierarchy Oct 10, 2018
@qwwdfsad
Copy link
Collaborator Author

qwwdfsad commented May 14, 2019 •
edited
Loading

More examples:

CoroutineScope(Job()).launch {
    val job = Job(coroutineContext[Job])
    launch(job) {
    println("A")
        throw Exception()
    }
}

qwwdfsad added a commit that referenced this issue May 16, 2019
@qwwdfsad
Improve coroutine exception handling logic
d3fd575 
  * 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

First part of fix for #689
@qwwdfsad qwwdfsad mentioned this issue May 16, 2019
Merged
qwwdfsad added a commit that referenced this issue May 17, 2019
@qwwdfsad
Improve coroutine exception handling logic
447fc97 
  * 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 added a commit that referenced this issue May 17, 2019
@qwwdfsad
Improve coroutine exception handling logic
0f54026 
  * 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 added a commit that referenced this issue May 20, 2019
@qwwdfsad
Recursively check whether parent handles exception to avoid duplicate…
e81a6d8 
…s reporting when Job() is present in the hierarchy

Fixes #689
qwwdfsad added a commit that referenced this issue May 24, 2019
@qwwdfsad
Recursively check whether parent handles exception to avoid duplicate…
0285057 
…s reporting when Job() is present in the hierarchy

Fixes #689
qwwdfsad added a commit that referenced this issue May 24, 2019
@qwwdfsad
Improve coroutine exception handling logic
b0f6e05 
  * 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
@Mygod
Copy link

Mygod commented Jul 24, 2019

From my crashlytics, it seems like something like this still happens in 1.3.0-RC...

@qwwdfsad
Copy link
Collaborator Author

What exactly? The same exception is handled twice?

@Mygod
Copy link

Mygod commented Jul 24, 2019

Never mind. It seems to be an error on my end.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
design
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@qwwdfsad @Mygod