-
Notifications
You must be signed in to change notification settings - Fork 1.9k
App exits after catching exception thrown by coroutine #753
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
Comments
PS: I see in the exception guide that launch "propagates exceptions automatically" but async "exposes them to users". Indeed, changing the code snippet to use async instead of launch makes it work as I expect:
But but but... this still seems pretty weird to me (remember I'm a newbie at Kotlin):
Yes I know it's possible to .await() on async and its type will be just Unit which is "void" but still strange when a particular coroutine "by its nature" doesn't return / produce anything at all (and it can be argued that "returns nothing" isn't quite the same as "returns void").
Since Kotlin doesn't have checked exceptions (yes I currently use Java), when writing a coroutine we won't be "told" by the compiler that "hey this can result in an exception, be careful"
There is no exception here. Yes one did occur, but it was caught and not re-thrown. That's there in the code, look, the exception does not escape, it's done for, dead-ended:
I'm sure there are reasons why things are as they are -- -- would it be possible to explain the rationale behind the "automatic rethrow even if caught and ignored" that's done by launch? I don't see anything in the official docs. |
See #576 |
Um the discussion in #576 is around what happens when a child coroutine crashes... ... such as from an unhandled exception. But in this case (in my snippet) - there is no unhandled exception that the coroutine's code allows to escape. The exception is caught and is not rethrown. The coroutine doesn't crash. There is also a snippet in #576 from @elizarov that looks like this:
Which also uses "launch" (not "async") but supposedly will not crash from the exception in child. I just tried updating my snippet to be in line with that sample snippet:
No difference, the app still crashes. It prints "Error from await" and that exception bubbles up and kills the app and doesn't even get caught in the second catch block (as "x2"). That my coroutine creates another one (on Dispatchers.IO) is purely an implementation detail of this top level coroutine. This (top level coroutine) already "knows" that this other coroutine can throw, and is prepared to deal with consequences by having a catch block around the await(), doing "something" with the exception and continuing on its merry way: Retrying a network request after a few seconds. Loading some possibly stale but still "better than nothing" data from disk. Faking it. Whatever. That's purely up to the (top level) coroutine. But with the way things are - the caller (which creates the top level coroutine) has to be aware of the coroutine's implementation details: "oh there is a try / catch in there, better use async() and avoid launch() which will crash". The coroutine's implementation can change over time too - today it doesn't make any coroutines itself, in tomorrow it might and there can be exceptions. And then what if this is in a library code? Am I missing something (clearly I am, being a notice here)? |
Hi @kmansoft So you can use an isolated scope, but you apply its rule for all childs in the scope, or you can use the |
Re: currently launch, async, actor' and produceshare the same behaviour, there is no way to recover a failed child coroutine Sorry but that doesn't seem to be the case - as I wrote above, changing GlobalScope.launch to GlobalScope.async at the top level coroutine creation - does stop the "automatic rethrow termination of coroutine". Re: Go - yes, I know what you mean, I could make sure to catch all exceptions inside coroutines and return them some other way, such as Result<String, Exception> (String or whatever). In fact I already tried that using this: https://github.com/kittinunf/Result and then wrote my own little helper class. Both worked fine, but that's more code. I was just expecting that since Kotlin can propagate exceptions from inside a nested coroutine to the caller's "await", across threads, asynchronously - that I could use this built-in mechanism for fine grained exceptions handling. Oh well. I guess. Thanks for your time. |
@kmansoft please consider this code, I hope this helps you. fun main(args: Array<String>) = runBlocking {
val job = launch {
val deferred: Deferred<Unit> = GlobalScope.async { error("Out of scope error") }
try {
deferred.await()
} catch (e: Exception) {
println("Handling error...")
e.printStackTrace()
}
}
job.join()
println("That's all")
} |
@fvasco it does, but... Seems that "the trick" is that you used GlobalScope.async on the child job (not just async). But this (not using GlobalScope) still crashes from "bubbed up" exception same as before: val job = GlobalScope.launch(Dispatchers.Main) {
val deferred = async(Dispatchers.IO) {
MyLog.i(TAG, "In async")
error("Out of scope error")
"foobar"
}
try {
deferred.await()
} catch (e: Exception) {
MyLog.i(TAG, "Handling error...", e)
}
}
In other words, one can avoid the child coroutine crashing the parent coroutine if the child is launched into a separate scope - not into the parent's scope? But isn't doing that have an effect on cancellations? That is, when using global scope, the child will not be cancelled when the parent is canceled? And speaking of cancellations.... I understand that they're implemented internally by throwing a special exception. But if that's the case - then any code that uses await() in try / catch and does some sort of error recovery / feedback in the UI if it gets an exception -> will get CancellationException in its catch block, correct? Think I've actually seen this while experimenting. So if I'm not mistaken, then handling errors from await() would need to look like this: val child = async {
// do something, maybe throw on errors
}
val value = try {
child.await()
} catch (x1: CancellationException) {
// Got cancelled, just peacefully return without doing anything else
return
}
catch (x2: Exception /* or Throwable */) {
// There is a real error, show to user
textView.text = x2.toString()
}
I'm just trying to understand - what is/are good patterns for coroutines so I could: 1) run some child jobs 2) with error (exception) propagation and 3) cancellation It seems that (1) is really easy and there are some slick features (like using delay() for animations - sweet!, or running parent on Dispatchers.Main and child jobs on Dispatchers.IO - cool). But I'm having trouble pulling it all together with (2) and (3). |
As @fvasco suggested in his first reply, you should use Supervisor scopes. Their purpose is exactly what you're looking for: to not crash the parent in case of child's failure. But cancelling the supervisor does cancel the children. suspend fun main() {
val job = GlobalScope.launch {
supervisorScope { // <-- This prevent the parent from crashing in case of error in a child
val deferred = async { error("Ooops") }
try {
deferred.await()
} catch (e: Exception) {
println("Handling error ($e)")
}
}
}
job.join()
println("That's all")
}
Yes. But you may ignore cancellation state of the coroutine with |
Re: But cancelling the supervisor does cancel the children. OK this is a useful property combination. Thank you. Re: Yes. But you may ignore cancellation state of the coroutine with withContext(NonCancellable) {} The point was not to have non-cancelable coroutines. My point was - if the "parent" expects that a child can fail and is prepared to deal with that, by using a catch block around "await child" -- -- then this catch block will also trigger on CancellationException if the job (and the child) is canceled. This means that the catch block has two have two separate stanzas, one for Cancellation and one for Exceptions (real failures from child, not cancellations). I gave an example of this in my previous commend, with exceptions "x1" and "x2". Is there a way to simplify that? Again all I'm looking for is a simple (DRY, boilerplate free as much as possible) pattern where a coroutine can delegate to other coroutines and handle their exceptions, and where cancellations also work and don't require additional code (to differentiate from genuine failures). ? |
It depends. In all cases (cancellation of child, cancellation of parent and failure of child),
Keep in mind that in both cases if coroutine is cancelled or failed the important point is : "it did not succeed". If you want to have different behavior depending on why it did not succeed, you have to explicitly handle it. (personally I would also show the user that the action cannot be completed because of a cancellation)
Try as much as you can to use inline fun ignoreCancellationException(block: () -> Unit) {
try {
action()
} catch (e: CancellationException) {
// ignore
}
}
suspend fun doSomethingMaybeThrowOnErrors() { ... }
suspend fun usage() {
ignoreCancellationException { doSomethingMaybeThrowOnErrors() }
} |
This is true of course. But the "awkwardness" (to me) is that when a child has an exception, the runtime by default treats it as a catastrophic failure in the whole scope. Consider "plain" (synchronous) functions: fun loadFromNetwork(...) {
val httpRequest = httpClient.execute(...)
// Can throw IOException
// Cannot be handled here - this function doesn't know enough
// Just lets IOException escape, the caller will decide what to do
}
fun loadFromFile(...) {
val fileStream = FileInputStream(...)
// Can throw IOException
// Cannot be handled here - this function doesn't know enough
// Just lets IOException escape, the caller will decide what to do
}
// The caller knows more:
fun loadData() {
try {
loadFromNetwork()
} catch (x1: IOException) {
// Loading from network failed, load from file if it exists
if (dataFile.exists()) {
loadFromFile()
//
// This is OK, the network is down, but we still have (older) data from file
//
} catch (x2: IOException) {
// Neither worked
// ....
return
} else { /* file does not exist, etc. */ }
}
} Here the parent has catch blocks - so exceptions are handled. But it might not have, and exceptions would escape - and then it'd be up to the caller of loadData, or its caller, etc. and a catastrophic failure would happen only if there were no catch blocks anywhere up the call chain. In other words, an exception in some child function is not necessarily a catastrophic failure. If this exception is handled somewhere up the call chain, then it is handled and "forgotten", has no further effect. Either way the function that originated the exception (and maybe more functions) is (are) unwound (= what you wrote about how "we cannot get a value from a coroutine that did not suceed"). Now with coroutines - even when there is a catch in the caller, an exception in a child, by default, is a catastrophic failure. Your earlier suggestion to use supervisorScope works, but it's more boilerplate code. I already have a catch() in the parent, so it's strange to have to do more stuff to indicate to the language / runtime that the parent is able to handle / ignore the exception. OK, so maybe that's just a matter of defaults. Fine. And maybe this particular default was chosen to make other cases more elegant. Fine.
Can you explain how to do this? It's just that all the more or less complete examples I've seen (so far) use patterns like this: GlobalScope.launch(Dispatchers.IO) {
..
}
launch(Dispatchers.Main .... ) {
}
async(Dispatchers ... ) {
}
launch {
}
async {
} Let't say I already have a suspend fun (which runs on Main let's say) - and I want to have coroutines that run on IO. How do I do this with "suspend function instead of using async everywhere"? |
It works exactly the same with suspending functions. This is also the reason of my advice "favor suspending function". At the end the very purpose of Kotlin coroutines is suspending functions. Without suspending function, there would be no benefit of Kotlin coroutines over futures/promises.
This kind of pattern is legacy from future-oriented paradigm and libraries. Coroutines aim to provide a new approach by adding Here's an example: suspend fun workOnIO() = withContext(Dispatchers.IO) {
try {
// work
} catch(e: ExpectedException) {
// ignore or handle this exception
} finally {
// release resources
}
}
suspend fun parallelDecomposition() = coroutineScope {
val a = async { ... }
val b = async { ... }
combine(a.await(), b.await())
}
class ViewController : CoroutineScope {
private val job = SupervisorJob()
override val coroutineContext = job + Dispatchers.Main
fun handleAction() {
launch {
workOnIO()
try {
parallelDecomposition()
} catch (e: ExpectedException) {
return@launch
}
updateUI()
}
}
fun dispose() {
job.cancel()
}
} You may also read one of my comment here: #342 (comment) Were I already gave quite thourough explanation about "why suspend is preferable"
it depends on what you want exactly. Why do you need a "coroutine" to run on IO? Is it an actor? Is it supposed to have an independent life scope? Or is it just an |
An async to do something and return a result (or exception or have the whole thing cancel). And I am expecting to find an an easy, language- and runtime- supported method of doing this "sequentially" in terms of syntax, but "asynchronously" in terms of "what really happens" - which fits what you wrote in #342
Thanks, I'll try it. Like I said, it seems that most tutorials (that I've seen) used launch / async so I initially started exploring in that direction. Thank you for the clarification and for your time. |
You probably also want to follow #763 which address the problem of |
@jcornaz I just tried it, yes nice that there is less "syntax noise" but (still!)... The "recipe" you gave is very close to one found here: https://proandroiddev.com/async-code-using-kotlin-coroutines-233d201099ff Let me step back and restate what I'm trying to do. Consider this all synchronous code, no coroutines, async / await's or anything: fun showOrder(accountId: String, orderId: OrderId) {
val account = try {
getAccount(accountId)
} catch (x: Execption) {
// Network error, whatever, inform the user
accountView = "Error getting account info: " + x.toString()
return
}
if (isCancelled) {
return
}
accountView = account.formatAsText()
val order = try {
account.getOrder(orderId)
} catch (x: Exception) {
// Error, inform the user
orderView = "Error geting order info: " + x.toString()
return
}
if (isCancelled) {
return
}
orderView = order.formatAsText()
} Things to note:
The problem here is of course that getAccount / getOrder need to execute on an IO thread, and updating the UI needs to execute on the "main" (UI, GUI toolkit) thread. Kotlin coroutines to the rescue! I can just run some parts of my code on an IO thread and other parts on the UI thread - but write the logic as if it were sequential and synchronous. val job = SupervisorJob()
GlobalScope.launch(job + Dispatchers.Main) {
try {
// Get account
val awaitAccount = withContext(Dispatchers.IO) {
async {
getAccount()
}
}
val valAccount = try {
awaitAccount.await()
} catch (x: Exception) {
// Error getting account
accountView = "Error getting account info: " + x.toString()
return@launch
}
// Got account, update the UI
accountView = valAccount.formatAsText()
// Get order
val awaitOrder = withContext(Dispatchers.IO) {
async {
account.getOrder()
}
}
val valOrder = try {
awaitOrder.await()
} catch (x: Exception) {
// Error getting order
orderView = "Error getting order info: " + x.toString()
return@launch
}
// Got order, update the UI
orderView = valOrder.formatAsText()
} catch (x: Exception) {
// Log / show the exception
}
} This magically executes every piece of code on the appropriate thread. Wonderful. But for some reason, any exceptions thrown by getAccount / getOrder are now not caught in the catch blocks around the respective await()'s. This is "astonitishing" again, there clearly are catch blocks around the awaits, and it's awaits which propagate exceptions, right? Exceptions are caught only in the top-level catch ("Log / show the exception") now and are thrown out of async{} blocks. Before adding withContext, they were getting thrown out of await()'s. And so, new questions if you don't mind:
Are there any guides that explain in more depth what's going on? I have read this: https://kotlinlang.org/docs/reference/coroutines/exception-handling.html ... but it just says that "async exposes exceptions to uses" and doesn't explain why the introduction of withContext exposes them out of async{} and not from await(), |
You still use If I translate your first (blocking) version of suspend fun showOrder(accountId: String, orderId: OrderId) = withContext(Dispatchers.IO) { // <-- Let's use IO by default here
val account = try {
getAccount(accountId) // <-- It's fine, we are on IO.
} catch (x: Execption) {
// Network error, whatever, inform the user
withContext(Dispatchers.Main) {
// This has to run on UI I presume...
accountView = "Error getting account info: " + x.toString()
}
return@withContext
}
// No need to check cancellation state here. Any suspending function like `withContext` would throw a cancellation exception
withContext(Dispatchers.Main) {
// This has to run on UI I presume...
accountView = account.formatAsText()
}
val order = try {
account.getOrder(orderId) // <-- It's fine, we are on IO.
} catch (x: Exception) {
// Error, inform the user
withContext(Dispatchers.Main) {
// This has to run on UI I presume...
orderView = "Error geting order info: " + x.toString()
}
return@withContext
}
// No need to check cancellation state here. Any suspending function like `withContext` would throw a cancellation exception
withContext(Dispatchers.Main) {
// This has to run on UI I presume...
orderView = order.formatAsText()
}
} You see that it is very much the same. The only difference are the Of course you need to adapt the usage, since you can only call a suspending function from a coroutine: fun handleUserAction() {
job = launch { showOrder(accountId, orderId) }
}
fun cancelAction() {
job?.cancel()
} |
this is not about My point is "you don't need it". One need |
Yes it is, just one thing, you switched the threads around. As a side benefit you got better defined cancellation points (i.e. every time the code tries to reach into the UI state).
I see it now - yesterday I misread your I now changed my code to use only GlobalScope.launch(job + Dispatchers.Main) {
val account = try {
withContext(Dispatchers.IO) {
network.getAccount()
}
} catch (x: Throwable) {
// This does get caught
}
} It now works much more to my taste. Not "astounding" anymore. Exceptions get propagated and caught in the nearest And then the whole Two remaining problems are that exceptions from inside But for those I bet a utility function would work just fine. |
I'm closing this issue. If there are still open problems, please report them separately. |
I'm new to Kotlin coroutines, too. But as far as I understand, E.g. we have to combine results of two suspend functions: suspend fun computeValue1(): Int {
delay(1000) // Imitate some heavy computation.
return 1
}
suspend fun computeValue2(): Int {
delay(2000) // Imitate some heavy computation.
return 2
} If we just call the suspend functions to combine their results, they will be executed sequentially, so we will have to wait 3 seconds ( val result = computeValue1() + computeValue1() // 3 seconds. But if we wrap the suspend functions with val value1Deferred = async { computeValue1() }
val value2Deferred = async { computeValue2() }
val result = value1Deferred.await() + value2Deferred.await() // 2 seconds. The next question is As it mentioned in #552, the parent coroutine shouldn't wait all child So, as far as I understand, structured concurrency has been introduced to So I have created my solution to work with
// Supervisor scope, which won't fail on some child failure.
// I have created a separate scope to have a possibility to cancel it when needed.
val supervisorScope = CoroutineScope(Dispatchers.Default + SupervisorJob())
supervisorScope.launch {...} // Child 1.
supervisorScope.launch {...} // Child 2.
// Child 3.
// If an exception occurs in some async, then it will be propagated to Child 3,
// cancel it and be consumed with Child 3's CoroutineExceptionHandler.
// Neither supervisorScope's Job, nor Child 1/ Child 2 Jobs won't be affected with
// Child 3 exception.
//
// Here we have a CoroutineExceptionHandler, because we don't want an exception
// to be propagated to Thread.uncaughtExceptionHandler
// (in JVM it will just print a log).
supervisorScope.launch(Dispatchers.IO + CoroutineExceptionHandler {...}) {
val value1Deferred = async { computeValue1() }
val value2Deferred = async { computeValue2() }
val result = value1Deferred.await() + value2Deferred.await() // 2 seconds.
// Do something with the result.
}
// viewModelScope == CoroutineScope(Dispatchers.Main + SupervisorJob())
viewModelScope.launch {...} // Child 1.
viewModelScope.launch {...} // Child 2.
// Child 3.
// If an exception occurs in some async, then it will be propagated to Child 3,
// cancel it and be consumed with Child 3's CoroutineExceptionHandler.
// Neither viewModelScope's Job, nor Child 1/ Child 2 Jobs won't be affected with
// Child 3 exception.
//
// Here we have a CoroutineExceptionHandler, because we don't want an exception
// to be propagated to Thread.uncaughtExceptionHandler
// (in Android it will kill the app).
viewModelScope.launch(Dispatchers.IO + CoroutineExceptionHandler {...}) {
val value1Deferred = async { computeValue1() }
val value2Deferred = async { computeValue2() }
val result = value1Deferred.await() + value2Deferred.await() // 2 seconds.
// Do something with the result.
} Conclusion: |
Hi @a-kari, However this isn't a library issue, for this kind or discussion I suggest you to prefer different resources, like the official Kotlin forum. |
Any news? Does still 'catch' not work well in kotlin coroutines? I like async-await pattern in other languages. The kotlin coroutines are too complicate to use and not work well in many cases. |
@vipcxj If you have specific cases where |
I'm new to Kotlin and coroutines so maybe I'm doing something wrong, but...
The test code below throws an exception which is propagated to the caller in await().
The caller catches the exception, logs it, and ignores. So far so good.
But - immediately after this, the app exits like this, looks like the exception gets rethrown.
This is quite unexpected to me (but again I'm a novice). Do exceptions propagated by await() always get rethrown? What if I want to ignore it (show message to user, log, etc. but don't want the exception to keep bubbling up)?
I'm using Android Studio 3.2, Kotlin 1.3.0-rc-190-Studio3.2-1 and kotlinx-coroutines-android 1.0.0-RC1
The text was updated successfully, but these errors were encountered: