Hi guys, what's the best practice for handling err...

# coroutines

[Djuro]

Hi guys, what's the best practice for handling errors in child coroutines or arbitrary places in suspend functions?
I know that try/catch and runCatching swallow the CancellationException .
Do you recommend wrapping the exception within coroutineScope which rethrows the exceptions at the call site like this

runCatching {
    coroutineScope {/*code*/}
}.OnFailure {
    // handle
}

I also use runSuspendCatching in some projects, what do you think about that approach?

[Dmitry Khalanskiy [JB]]

try/catch doesn't swallow exceptions if you only catch the exceptions that you need to handle.

[Joffrey]

@Dmitry Khalanskiy [JB] is there already an open issue for marking runCatching as delicate? More and more people seem to try to use it for railway programming, and don't realize they are catching too much with it. It seems we made it too easy to do the wrong thing with this function.

[Djuro]

@Dmitry Khalanskiy [JB] so you say that I should always be aware of which exceptions I am handling? What about cases when working with some old api that's not documented well, or cases when I just don't care what type of exception it is, does that just mean I am doing it wrong

[Dmitry Khalanskiy [JB]]

I should always be aware of which exceptions I am handling?

Yes, sure. When you are asking the program to do something, why could you not care about why it failed to do so? There are some rare situations like "I'm asking the program to do something, and it would be nice if it did that, but I don't actually care," but they are rare.

when I just don't care what type of exception it is

Tough to imagine this situation. Do you really not care if you get an OutOfMemoryError? StackOverflowError? ThreadDeath? runCatching will catch all of them, even though the only option when you receive them is usually to crash the program.

What about cases when working with some old api that's not documented well

This question is too abstract, tough to say anything definite. However, it's tough for me to imagine why you wouldn't care about why some API failed. Let's say that, during testing, you found out that the undocumented function startServer can fail with IllegalArgumentException and IllegalStateException. You add try-catch blocks for these scenarios. Then, in a month, suddenly, this API crashes with UnsupportedOperationException each time it's called on some device. Do you really not care that this function failed with something you didn't anticipate?

[Dmitry Khalanskiy [JB]]

In any case, all rules can be bent. We have a hundred instances of catch (e: Throwable) and a dozen calls to runCatching in kotlinx-coroutines. There are some cases where this is the best or even the only choice. But using runCatching should be a deliberate choice, not the default option.

[Dmitry Khalanskiy [JB]]

@Joffrey, we do intend to do something soon, but we didn't yet have a design meeting to discuss it. Nothing is yet decided. I shared your question with the team.

[Pablichjenkov]

Is there anything wrong with this:

public inline fun <T, R> T.cooperativeCatch(block: T.() -> R): Result<R> {
    return try {
        Result.success(block())
    } catch (e: CancellationException) {
        throw e
    } catch (e: Throwable) {
        Result.failure(e)
    }
}

[Dmitry Khalanskiy [JB]]

Yes, it doesn't always work with the cooperativeCatch { deferred.await() } pattern (where deferred is a Deferred<T>): https://github.com/Kotlin/kotlinx.coroutines/issues/1814#issuecomment-1943224856

[Pablichjenkov]

😥 I see.

[CLOVIS]

@Joffrey I created this issue quite some time ago due the number of people asking similar questions in this slack: https://youtrack.jetbrains.com/issue/KT-55480/Document-that-runCatching-breaks-structured-concurrency

It seems there is an open issue to add a warning in IDEA

[Joffrey]

@CLOVIS thanks, but my point is unrelated to CancellationException and coroutines. runCatching is mostly an anti-pattern in general, and coroutine exceptions are just one obvious example. People start to ask about band-aids for `runCatching`+coroutines but the problem is more general.

[Djuro]

Thanks for the insight. All makes more sense now tbh. I was just looking for best practices regarding this

[Djuro]

So conclusion would be to know what you are doing and dont catch exceptions you dont know how to manage 😄
@Joffrey if runCatching is an anti pattern in general, how isnt try/catch also an anti pattern?

[CLOVIS]

catch(e: Throwable) is the exact same anti-pattern as runCatching. catch(e: TheSpecificExceptionIKnowThisCodeMayThrow) is fine.

The anti-pattern isn't catch itself, it's “catching absolutely anything that could happen indescriminately”. Much like having variables of type Any is an anti-pattern but any other type is fine. Even non-concrete types like `List`are fine because they still communicate intent.

[Djuro]

Ah thats what you ment, got it. Thank you

[juliocbcotta]

Jet brains folks, my experience is the opposite of what you folks say. As I said in the issue on GitHub, I don't care about any exceptions in particular, network error, failed json parser or out of memory. There is nothing devs can do with that in an android app to remedy the situation without monitoring and a new release. We just need to detect that it happened and show an error state, many times which particular error happened is irrelevant to the user, the operation failed and that is it. I included OOM in the list because the cause of the OOM may be the coroutine code and for sure any company would rather have their app crash later than sooner, also, by catching it I can log it as non fatal, it is way more graceful to have the app handling it like that. For instance, I may be "downloading a file that is too big to fit in memory", should my app crash or should I just present an error to the user? No sane product person would say that is better to crash. "My recursion call went too deep and I got a Stack overflow" , same case. "The process is dead?" Great, I don't care.

runcatching is not an ant pattern, it does what we want, we now need a specialization of it to handle coroutines better.

[CLOVIS]

I may be "downloading a file that is too big to fit in memory", should my app crash or should I just present an error to the user?

Catching the OutOfMemoryException will not stop the app from crashing. When OOM is thrown, it's very likely that anything you do will still lead to the app crashing, unless you are able to free a lot of memory directly in the catch block (and even then, I'm unsure it's enough for the JVM to continue working). This is what the previous users were saying: you cannot realistically recover from most non-Exception throwables. You may say you want to, but runCatching doesn't let you do that.

runcatching is not an ant pattern, it does what we want

it doesn't.

we now need a specialization of it to handle coroutines better.

Use either.catch.

[Dmitry Khalanskiy [JB]]

@juliocbcotta, you're presenting this as if there were only two choices: either using runCatching is okay, or the app crashes. But that's a false dichotomy. The use case you are describing (having global catch (e: Throwable) around a huge block of logic) is only a tiny portion of error handling, and having a bit more ceremony around these parts (explicitly writing out try/catch blocks, with comments that this is intentional, maybe wrapped in some code to retry the operation a couple of times) shouldn't be a problem. The issue with runCatching is that it's a shorthand version of something that should not be taken lightly.

There are use cases for wiping out all user data on your PC. There should be a way to do so. It would still be surprising if there was a dedicated button without a "danger" label on the side of your PC that did so.

[juliocbcotta]

I have said anything about two options, I said that the current implementation does not help with real world scenarios and that devs would like to have something like suspendRunCatching without all the ceremony of handling individual exceptions.

[juliocbcotta]

Ivan, it does not matter, I am fine with the possibility of app crashing later. The operation running the coroutine can be the cause of the OOM so I would prefer to catch the error and hope that GC run and allow me to log it and show an error state in the UI. I am not using Arrow or intent to. Not to solve this issue.

[juliocbcotta]

I prefer the possibility of crashing over the for sure this will crash.