Provide a `runCatching` that does not handle a `CancellationException` but re-throws it instead.

[Anton-Spaans-Intrepid]

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 suspend keyword is added to prevent this new function from being called in regular non-suspending/blocking code.
• When code calls the StdLib's runCatching in a Coroutine/suspendable-context, a lint warning should warn the developer that handling cancellations may be compromised and runSuspendCatching should be used instead.

[qwwdfsad]

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?

[Anton-Spaans-Intrepid]

@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.

[fvasco]

@Anton-Spaans-Intrepid
Do you think that the current runCathing function should rethrow any InterruptedException?

[Anton-Spaans-Intrepid]

@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).

[Anton-Spaans-Intrepid]

(looping in my personal github account: @streetsofboston)

[mhernand40]

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.

[elizarov]

@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?

[recheej]

@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?

[recheej]

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")
            }
       }
   }

[erikhuizinga]

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.

[vogthenn]

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).

[erikhuizinga]

@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.

[ephemient]

@erikhuizinga https://github.com/Kotlin/KEEP/blob/master/proposals/stdlib/result.md Looks like intended use cases are postponing failure and internal within coroutines.

[erikhuizinga]

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?