kotlinx.coroutines
kotlinx.coroutines copied to clipboard
Kotlin
Provide a `runCatching` that does not handle a `CancellationException` but re-throws it instead.
Problem Description
The Kotlin StdLib contains a function named runCatching. It tries the provided lambda and catches any Throwable that is not caught or thrown by the lambda.
However, using runCatching in Coroutines can have a somewhat hidden (and unexpected) side-effect of it catching CancellationExceptions and not re-throwing them. This could interfere with the Coroutines' Structured Concurrency, with its cancellation handling.
Possible Solution
E.g. create a new function in kotlinx.coroutines that looks something like this (not sure about the name 😄):
public inline suspend fun <R> runSuspendCatching(block: () -> R): Result<R> {
return try {
Result.success(block())
} catch(c: CancellationException) {
throw c
} catch (e: Throwable) {
Result.failure(e)
}
}
The runSuspendCatching will catch any exception and wrap it in a Result, except for CancellationExceptions that will be re-thrown instead. This will prevent unwanted interference with the cancellation handling of Coroutines.
Notes
- The
suspendkeyword is added to prevent this new function from being called in regular non-suspending/blocking code. - When code calls the StdLib's
runCatchingin a Coroutine/suspendable-context, a lint warning should warn the developer that handling cancellations may be compromised andrunSuspendCatchingshould be used instead.
Thanks for the proposal!
Could you please elaborate why is it unexpected behaviour? E.g. why one may expect to use runCatching that actually rethrows some of the exceptions? What are the use-cases and why runSuspendCatching is preferred over something more conventional?
@qwwdfsad
If the developer uses the StdLib runCatching { ... } and the runCatching wraps a suspendable piece of code (i.e. it could throw a CancellationException), the runCatching should re-throw any CancellationException. If not the suspendable caller of runCatching may not get cancelled and the Structured Concurrency of Coroutines breaks down.
@Anton-Spaans-Intrepid
Do you think that the current runCathing function should rethrow any InterruptedException?
@fvasco Good question; maybe :-) But this is about CancellationExceptions when using runCatching in a Coroutine.
@qwwdfsad
Here is an example of where using runSuspendCatching can fix the cancellation-handling issue caused by runCatching:
fun main() {
val screenScope = CoroutineScope(Dispatchers.IO)
println("Start")
screenScope.launch {
println("Launched")
val result = runCatching {
delay(1000)
4
}.getOrElse { 0 }
writeToDatabase(result)
}
Thread.sleep(500)
// User leaves the screen.
screenScope.cancel()
println("Job was cancelled: ${screenScope.coroutineContext[Job]?.isCancelled}")
println("Done")
}
suspend fun writeToDatabase(result: Int) {
println("Writing $result to database")
}
Running the above main function prints out:
Start
Launched
Writing 0 to database
Job was cancelled: true
Done
This is not right. The screenScope was cancelled, the call to writeToDatabase should not have happened. But it did happen, because the runCatching did not re-throw the CancellationException thrown by the delay(1000) statement.
If we change the code like this (replacing runCatching with runSuspendCatching):
fun main() {
val screenScope = CoroutineScope(Dispatchers.IO)
println("Start")
screenScope.launch {
println("Launched")
val result = runSuspendCatching {
delay(1000)
4
}.getOrElse { 0 }
writeToDatabase(result)
}
Thread.sleep(500)
// User leaves the screen.
screenScope.cancel()
println("Job was cancelled: ${screenScope.coroutineContext[Job]?.isCancelled}")
println("Done")
}
suspend fun writeToDatabase(result: Int) {
println("Writing $result to database")
}
this is printed out:
Start
Launched
Job was cancelled: true
Done
This is correct. The call to writeToDatabase was not made at all, which is the desired behavior (try replacing runSuspendCatching with just a plain run and remove the getOrElse; you'll get the same result).
FWIW, the catch operator of Flow already specifically handles CancellationException in that it will re-throw it if the Throwable instance is of type CancellationException.
I also asked about a similar runCatching variant for suspend functions in https://medium.com/@mhernand40/thank-you-for-the-reply-dd01cf846ab7.
@Anton-Spaans-Intrepid Let's take a look at a more realistic example, please. What are the use-cases for runCatching in an application code? Can you give a code snippet that motivated you to come with this proposal?
@elizarov I also have this use case. My repository is private so I can't post the source, but just in general I've had to repeat the same pattern where I want to handle some exception in a suspending function somehow. For example I can log, or I can send over to a crash reporter. However, what if this exception is handling is a CancellationException? I think in most cases you want to ignore this exception when logging or your crash reporter.
With CancellationException I'll want to rethrow it or just ignore it. It would be nice to just have some runCatching function that does this behavior for you. You can of course write it pretty simply yourself (that's what I ended up doing), but I think it'd be nice if the library included it for you. I think in general there's a lack of documentation on best practices on handling CancellationException from the library. Maybe you can include such a thing and also link to this utility function?
Code snippet would be something like:
suspend functionThatCanThrow() {
//some long running operation
}
suspend main() {
try {
}
catch(ex: Exception) {
if(ex is CancellationException) {
throw ex
} else {
LOG.errror(ex, "error calling function")
}
}
}
runCatching is expected to run catching. It would be very strange if some exceptions would not be caught by it. I can imagine the usefulness of a new function Result<*>.except<reified T : Throwable>() or Result<*>.throwIfIsInstance<reified T : Throwable>() that would rethrow. For example:
inline fun <reified T : Throwable> Result<*>.except(): Result<*> =
onFailure { if (it is T) throw it }
It would allow you to:
runCatching { mightThrowCancellationExceptionThatShouldNotBeCaught() }
.except<CancellationException>()
.onFailure { throwable /* Will never be of type CancellationException */ -> }
My use case is that I materialize various errors from lower layers in an Android view model for its observers and I do some logging. This just so happens to be in a coroutine context (in suspend funs running in the view model coroutine scope), so this might throw CancellationException at any time. I don't want to catch and materialise those, so in my use case I just want to rethrow them so that all parents can handle them too: they might need to do some finalising before they can cancel properly.
Also, what's with OutOfMemoryErrors and similar? It's an antipattern (not sure whether it's disputed or not) to catch Throwable in Java, and I'd guess it also is in Kotlin.
Example: I don't think that it's really expected by the user to ignore OutOfMemoryErrors and NOT dump the JVM heap, even if you configured the JVM to dump.
Catching Throwable will do just that (again: Not 100% sure about Kotlin, not the expert, yet).
@vogthenn by that reasoning I wonder what the original use case is for runCatching, because you make it sound like an antipattern without legitimate uses.
@erikhuizinga https://github.com/Kotlin/KEEP/blob/master/proposals/stdlib/result.md Looks like intended use cases are postponing failure and internal within coroutines.
It seems that the KEEP's use case 'Functional error handling' (specifically the example with runCatching { doSomethingSync() }) is one that easily introduces the side effect being discussed in the current issue: it catches all throwables instead of the ones that are expected.
The proposed idiomatic solution in that KEEP is to be explicit about any expected outcome: either always return a result (but don't throw), or return a nullable result (but don't throw), or materialise various outcomes, e.g. use sealed types of expected failures or results (but again don't throw). If applied ad nauseam, then the KEEP is right: you would only expect exceptions from the environment (e.g. out of memory) or from failure-heavy operations or libraries (e.g. CancellationException).
Are the aforementioned use case and the proposed ways of handling errors in the function's signature contradictory?
Using runCatching {} is tempting to handle the I/O errors described in https://elizarov.medium.com/kotlin-and-exceptions-8062f589d07. There are some cases where listing all possible exceptions is complex, especially when they come from deeply nested, potentially JDK/third party SDKs.
It'd be nice to be more explicit as to how to use exceptions. In the current state, APIs like runCatching {} and Flow.catch {} make it easy to use exception for control flow which can be dangerous as showcased here.
PS: also, the fact that Result was only available from runCatching {} for some time was an incentive to use the dangerous runCatching {}
It'd be nice to be more explicit as to how to use exceptions. In the current state, APIs like
runCatching {}andFlow.catch {}make it easy to use exception for control flow which can be dangerous as showcased here.
As noted in https://github.com/Kotlin/kotlinx.coroutines/issues/1814#issuecomment-588344232 Flow.catch {} does re-throw CancellationException.
As noted in #1814 (comment) Flow.catch {} does re-throw CancellationException.
Indeed. I understand that Flow is in the coroutine world and runCatching is not, so it somehow makes sense. But it also makes things more confusing that one of them is "cancellation aware" while the other is not.
Thank you Anton, really good suggestion.
I have a question: how to deal with the TimeoutCancellationException? In most cases, the timeout should be treated as a failure result. But TimeoutCancellationException inherits from the CancellationException, so it will be re-thrown.
The following example fixes that behaviour:
public suspend inline fun <R> runSuspendCatching(block: () -> R): Result<R> {
return try {
Result.success(block())
} catch (t: TimeoutCancellationException) {
Result.failure(t)
} catch (c: CancellationException) {
throw c
} catch (e: Throwable) {
Result.failure(e)
}
}
What do you guys think about this?
It seems that the KEEP's use case 'Functional error handling' (specifically the example with
runCatching { doSomethingSync() }) is one that easily introduces the side effect being discussed in the current issue: it catches all throwables instead of the ones that are expected.The proposed idiomatic solution in that KEEP is to be explicit about any expected outcome: either always return a result (but don't throw), or return a nullable result (but don't throw), or materialise various outcomes, e.g. use sealed types of expected failures or results (but again don't throw). If applied ad nauseam, then the KEEP is right: you would only expect exceptions from the environment (e.g. out of memory) or from failure-heavy operations or libraries (e.g.
CancellationException).Are the aforementioned use case and the proposed ways of handling errors in the function's signature contradictory?
After Kotlin 1.7 underscore operator for type arguments, @erikhuizinga solution is even more appealing because you can use it that way:
inline fun <reified T : Throwable, R> Result<R>.except(): Result<R> = onFailure { if (it is T) throw it }
// ...
.except<CancellationException, _>()
the type of the item inside the result will be automatically inferred by the underscore, without the need to use *.
We use runCatching for wrapping any exceptions into new exceptions which add context information. We need to catch all exception, because we call 3rd party function which do not specify which exceptions they can throw. The reason for doing this is to add debug information to the exception stacktrace, because often the call stack of a single exception is not complete and only contains the stack of the current coroutine. So we add runCatching before we do a suspend call and wrapp any exceptions. As a result we have the call stack from all coroutines. However if we wrap a CancellationException info a normal RuntimeException and throw the RuntimeException, the cancellation becomes an error.
The question is how can we add context information to the cancelation cause while the CancellationException is traveling down the call stack, so that if we finally log the exception we have all debug information we need?
We use runCatching for wrapping any exceptions into new exceptions which add context information
@Legion2 so how exactly do you wrap OutOfMemoryError, and why do you even want to wrap it at all? Using runCatching catches Throwable, not Exception. Plenty of which you shouldn't even try to catch (probably almost all Error descendants). It's not an API that is meant for business code, but rather something for framework code that needs to report exceptions in a different way, such as the coroutines library itself.
Yes we use it only for error reporting to have more glues where the exception originated from (the stack trace does not provide us with this information). We always rethrow them and now handle the cancelation exception specially and don't wrap it, because it is catched again by coroutine framework and converted into normal control flow again. So it does not make sense to alter cancelation exceptions.
Btw. The fact that withTimeout function throws a cancellation exception is very bad API design. it only makes sense in the block which is cancelled, but the caller should receive another type of exception. Because the caller is not canceled if the callee times out.
Any update on this ?
Being able a catch a CancellationException in a coroutine doesn't make any sense to me. In a perfect world, it should be impossible. It only happens because coroutines are based on Exceptions to propagate state. To me, it's an anti-pattern for a coroutine to be able to prevent the termination of its scope. A coroutine can specify it needs to "terminate gracefully" with a finally and / or with NonCancellable, but that should be it.
Why not instruct the compiler to automatically re-throw the CancellationException in a catch block surrounding a suspending function ? I can't see a decent usecase where this wouldn't be beneficial.
To be honnest, I expected this behavior to be automatic since there's so many bytecode generated for coroutines. I expected the "magic CancellationException propagation" would be somewhat "protected" at the compilation level when generating the code behind coroutines.
Any update on this ?
Unfortunately, no. There are two conceptual problems we see here:
- The name should be concise enough to be usable yet clearly reflect the intent. We do not have such a name yet
- We are not sure that it is completely safe to catch and ignore arbitrary cancellation exception, yet we do not have a mechanism to distinguish e.g. Java-specific CE from coroutines one.
It also would be nice to see more real-world use cases for such API to understand what kind of problems are being solved.
One can argue that "it's obvious that we don't want to catch CE", but it does not help to shape API.
For example, if the majority of cases operates with Result in a declarative manner, then except<> extension might do the trick. For others, runSuspendCatching might be preferable and it's crucial for us to understand what exactly is being solved.
Why not instruct the compiler to automatically re-throw the CancellationException in a catch block surrounding a suspending function ? ... at the compilation level when generating the code behind coroutines.
The compiler knows nothing about kotlinx coroutines, neither it knows about CancellationException and we would like to avoid pushing particular library concept into the core of the language
It also would be nice to see more real-world use cases for such API to understand what kind of problems are being solved.
This is a perfect example: https://github.com/Kotlin/kotlinx.coroutines/issues/1814#issuecomment-588317835. Why isn't it more considered ?
Even if @elizarov thinks this is not a realistic use case, this is exactly the kind of problem I'd meet in Android if I tried to protect my project from random Exceptions being thrown from a library I can't control (or ditch :D ). runCatching (or just plain try / catch Exception) is exactly this kind of use-case, to me. Transforming a world of obscure / goto Exceptions to a world of exhaustive sealed classes.
This is a perfect example: https://github.com/Kotlin/kotlinx.coroutines/issues/1814#issuecomment-588317835. Why isn't it more considered ?
@NinoDLC It's not an example use case, it's a made-up experiment to demonstrate a behaviour. The runCatching in this example wraps nothing: a delay and a plain int expression. The point here is to gather real use cases where you actually need to "catch everything" and you really feel like you need runCatching.
runCatching (or just plain try / catch Exception)
runCatching catches more than Exception, it catches Throwable - it's actually the main reason why I don't find it suitable at all for application code in the first place, and why the problem mentioned here is moot IMO (because if there is no need for runCatching in application code, there is no need for runSuspendCatching either).
My 2 cents about using runCatching and Result in general are expressed in more details here by the way:
https://stackoverflow.com/a/70847760/1540818
To me, it's an anti-pattern for a coroutine to be able to prevent the termination of its scope
@NinoDLC coroutine cancellation is cooperative, so in the general case there is no guarantee of prompt cancellation. It's always controlled by the coroutines. So no it's not an anti-pattern, it's rather one of the premises of the design
Sorry yes it's not an anti-pattern, more so a bad practice indeed. I wanted to mention that not checking periodically that your CoroutineContext is active in a lengthty function is possible, but not desired in the world of coroutines. Preventing the propagation of the CancellationException is the same idea. It prevents the coroutines from working.
The point here is to gather real use cases where you actually need to "catch everything" and you really feel like you need runCatching.
Sorry, english is not my native language so I try my best.
Basically, my usecase for using runCatching is like I said : I want the less verbose Kotlin way to protect my Kotlin code from obscure/countless Exceptions being thrown when calling a function from a library I can't control, and of course this is inside a coroutine. I don't care at all about the different types of Exceptions. Maybe tomorrow a new type of Exception will be added in the library. I don't care. Either the function fails, either I get a result. Perfect.
Now I want this runCatching (or any other way) to respect one of the "coroutines internal mechanism" which is the correct propagation of the CancellationException.
I don't find [
runCatching] suitable at all for application code in the first place
To you, it's bad. To me, it's acceptable. In Android, if I get an OOMError, ~my process will be terminated anyway, my application will crash and the application may be relaunched afterwards if the Android OS deemed it OK. The OS handles these errors for me. I don't care if I catch them or not, my process will die after anyway. If I get less verbosity out of using runCatching, why not ?~, it's swallowed (my bad on this !)
And if suspendRunCatching is bad, let's go with a suspendRunCatchingException then so everyone is happy ? :D
I see.
Then I got quite a simple question [to everyone interested in this API]: why do you prefer runCatching that ignores CancalletionException instead of runCatching that catches all, but invokes ensureActive right after? If there is any difference, why it does matter for you?