Hey I want to ask people around if `runCatching` is in use i

Question

Hey I want to ask people around if `runCatching` is in use in any of your Kotlin or Android projects I feel my team is abusing it and I m trying to understand if there s any pros about using it in any coroutine based codebase My team just adopted this convention and I m at loss of words Examples in thread for anyone interested

Nino · Answer

Basically new suspending code would look like it ```class FetchFooUseCase Inject constructor private val synchronizeFooUseCase SynchronizeFooUseCase private val fooRepository FooRepository suspend fun invoke fooId String Result lt ShowEntity gt = runCatching runCatching fooRepository getFooById fooId getOrNull synchronizeFooUseCase invoke fooId as RefreshFooFromServerResult Success foo ``` which is used by ```class GetBarsByFooIdUseCase Inject constructor private val fetchFooUseCase FetchFooUseCase suspend fun invoke fooId String Result lt List lt BarAggregate gt gt = runCatching fetchFooUseCase invoke fooId getOrThrow mapNotNull runCatching createBarAggregate it getOrNull ``` Another example with Flows ```class GetFooUpdatesUseCase Inject constructor private val fooDownloadStatusRepository FooDownloadStatusRepository private val fooFavoriteRepository FooFavoriteRepository private val fooProgressRepository FooProgressRepository fun invoke Flow lt FooUpdate gt = merge runCatching fooFavoriteRepository getFavoriteFooUpdates getOrDefault emptyFlow runCatching fooProgressRepository getProgressUpdates getOrDefault emptyFlow runCatching fooDownloadStatusRepository getDownloadStatusUpdates getOrDefault emptyFlow ```

Joffrey · Answer

Here is what I think <https stackoverflow com a 70847760 1540818> IMO there is only a tiny fraction of use cases where `runCatching` could be useful and no business code should use it

Joffrey · Answer

OMG this code is horrendous to read to be frank I would love to see arguments in favor of this It s also wrong because in suspending contexts this will catch `CancellationException` which would be incorrect 99 of the time

streetsofboston · Answer

Yup it s a bit abused and you run the risk of catching and not re throwing CancellationExceptions breaking the structured concurrency model of coroutines

Nino · Answer

Yes I shared this very interesting and thoughtful answer with my team along with the excellent article by Roman but they just dismissed it because Roman has a background of backend developper so his thoughts won t apply to Android sic

streetsofboston · Answer

If they want to use railway oriented programming I suggest using Arrow s `Either` class instead where this is made easier and works well within the structured concurrency model of coroutines

Nino · Answer

Yes that was my point too either with sealed classes which we used before or `Either` obviously

streetsofboston · Answer

Still just ignoring exceptions by returning null or empty lists etc not properly modeling your errors is not a good way to go

Nino · Answer

But their counter argument was Domain should be safe sic

Joffrey · Answer

Just because Android has a UI doesn t mean all exceptions should be caught at the lowest level The mistake most people make here is that they confuse use exceptions bubbling with don t handle exceptions The point is that exceptions should be handled at a high enough level so user actions can be met with user level errors

streetsofboston · Answer

Safe as in there won t be any exceptions

Nino · Answer

I don t know to be honest at that point I didn t even know what to say anymore joy

Joffrey · Answer

Yeah I d be eager to know how much safer they think the nested `runCatching` makes things in the first example

streetsofboston · Answer

Oh you won t get any crashes from exceptions thrown from your domain but you ll be in for for lot of surprises higher up the domain chain slightly smiling face

Joffrey · Answer

Yeah this doesn t make any sense You can prevent hard crashes at the highest level if you don t want that and that makes sense but turning every error into a null or something like that is just begging for subtle invisible bugs Good luck tracking the source of the problems

Nino · Answer

Wrapping everything in `runCatching` makes me afraid of blank pages where if you have any issue instead of crashing which is automatically reported back so we have metrics you just have a non functional UI which is wayyy harder to detect

streetsofboston · Answer

< Nino> I prefer sum types separating good results and errors composition using sum types like Either for example over combining them into one hierarchy unless it s the UI state you re modelling where you fold both errors and good data into one UI

CLOVIS · Answer

Same message as always when someone mentions `runCatching` it is NOT safe to use with coroutines runCatching catches Throwable which includes CancellationException This stops coroutines from dying correctly if a coroutine is cancelled it will still be considered alive by its parent leading to memory exhaustion and an eventuaal live lock

Joffrey · Answer

It is not safe to use without coroutines either laughing

CLOVIS · Answer

If you want railroad oriented programming use Arrow s Either catch which handles CancellationException correctly and also has an amazing DSL to make it nice to read

Nino · Answer

gt unless it s the UI state you re modelling where you fold both errors and good data into one UI Yes that was that strategy we used before either a nullable type is returned if we didn t care about the error type it just failed so `null` is returned so we would have a generic error message displayed in the view either a sealed class providing the result or the error to be able to differentiate between NotFound and NoInternet errors for example

Nino · Answer

They wanted to use a suspendRunCatching function re throwing the `CancellationException` but it still seemed like a bad idea Any thoughts

Joffrey · Answer

The guideline to use `runCatching` all over the place specifically using `runCatching` at different levels in the code effectively nesting them locally or less locally shows laziness on the part of the devs IMO It shows that instead of reflecting on the proper place to handle and report errors they want to feel safe and hide errors This doesn t work in the long run

CLOVIS · Answer

< Nino> it s very bad practice to catch all exceptions no matter how you do it

streetsofboston · Answer

That would be a bit better but still as bad as wrapping everything in try catch and then basically ignoring the catch by always returning null empty Errors should be modeled even if it is just a simple model

CLOVIS · Answer

Also you shouldn t always rethrow CancellationException There are some cases in which they are genuine errors

streetsofboston · Answer

^^^ Doesn t happen often but yes

Nino · Answer

Interesting I didn t know that Do you have any usecase in mind where rethrowing CancellationException is a mistake

streetsofboston · Answer

A Java lib could throw a CancellationException

streetsofboston · Answer

One that is an error and shouldn t really cancel the coroutines

streetsofboston · Answer

< Joffrey> Subclasses of CancellationExceptions are fine they are still CancellationExceptions

Nino · Answer

A java lib that knows and throws `kotlin coroutines cancellation CancellationException` That s weird

streetsofboston · Answer

There are java ones as well

Joffrey · Answer

Sorry I was too late to delete the message This example was not the correct one

Peter Farlow · Answer

```public actual typealias CancellationException = java util concurrent CancellationException```

Peter Farlow · Answer

it s just Java s under the hood in the JVM

streetsofboston · Answer

Hi Peter

CLOVIS · Answer

Well mistake depends on what your objective is Some channel functions return CancellationException when there are no elements If you rethrow them they ll invisibly cancel the calling coroutine and you ll never get an error message

Nino · Answer

Ok you just blew my mind Peter lol I would never expect this typealias to be done Is there any reason for that It seems dangerous for coroutines to rely on it Like any lower java api could unexpectedly cancel the coroutine open mouth

streetsofboston · Answer

Overall CancellationExceptions should be re thrown for coroutines Occasionally alas a method throws such an exception incorrectly they should ve used another type of exception Exceptions always have been implemented used terribly on Java confused

CLOVIS · Answer

Great blog on CancellationException <https betterprogramming pub the silent killer thats crashing your coroutines 9171d1e8f79b gi=b4555a1271e9|The Silent Killer That s Crashing Your Coroutines | by Sam Cooper | Feb 2023 | Better Programming>

Joffrey · Answer

So actually let me come back to the timeout example The reason why I thought of `TimeoutCancellationException` subclass of `CancellationException` is that in this case the exception is an actual error it expresses a timeout and it s not meant to cancel the current coroutine So if the purpose of the over confident `runCatching` is to catch errors this timeout should be one of them Just re throwing it kinda defeats the purpose NOT of `withTimeout` but of `runCatching` in this case

streetsofboston · Answer

I read that article and i m not sure I agree with it entirely slightly smiling face Wonder what Roman would think about this article slightly smiling face

Joffrey · Answer

Roman actually thinks this article is on point wink from what I could tell

streetsofboston · Answer

Nice Did he comment somewhere about it

Joffrey · Answer

Yes internally at JetBrains

Joffrey · Answer

But there are also some public issues about the fact that `withTimeout` and channel functions the one mentioned in the article you linked throw subclasses of `CancellationExceptions` in situations where the caller of the function is not cancelled

Joffrey · Answer

Those exceptions are not meant or should not be meant to cancel the coroutine of the caller They show an actual error in the call which is related in some way with the cancellation of some other coroutine In these cases I bet your colleagues pushing for `runCatching` would actually want to catch those

Joffrey · Answer

gt Roman actually thinks this article is on point wink from what I could tell Btw I don t want to put words in your mouth < elizarov> so please correct me if I m wrong in saying this

OG · Answer

gt OMG this code is horrendous to read to be frank I would love to see arguments in favor of this It s also wrong because in suspending contexts this will catch `CancellationException` which would be incorrect 99 of the time edited FWIW I have used these wrappers for properly handling CancellationExceptions ``` Executes block and returns a Result All exceptions thrown are returned as a Result Failure except a kotlinx coroutines CancellationException which will be rethrown See Throwable isCancellation for more info inline fun lt T gt runCatchingIgnoreCancelled block gt T Result lt T gt = runCatching block onFailure error gt if error isCancellation throw error ``` ``` Extension on a Throwable that returns true if it is of type CancellationException Typically this type of Exception should not be caught and should be bubbled up Coroutines internally uses CancellationException for cancelling of work and they are ignored by all handlers by default See cancellation and exceptions <https kotlinlang org docs exception handling html cancellation and exceptions> fun Throwable isCancellation = this is CancellationException amp amp this is TimeoutCancellationException``` This was mainly useful in UseCase domain layer where the consumer presentation layer didn t care about the lower level error or why something failed but only if there was a result or not

Joffrey · Answer

A note about this special case for `TimeoutCancellationException` Imagine your use case layer `await`s on an `Deferred` produced by some `async` in a different coroutine scope If that other coroutine scope is cancelled NOT the one of your use case doing the `await ` the `await` will throw `CancellationException` which is just another error in this case representing the absence of value In this case you will re throw and it will not go through your classic error handling that you intended to hook via `runCatching` Same goes for channels `send` `receive` Cancelling some other coroutine at the other end of the communication channel shouldn t really be considered like other cancellations by your code but rather like an error most of the time See <https betterprogramming pub the silent killer thats crashing your coroutines 9171d1e8f79b gi=b4555a1271e9> for more details about these weird `CancellationException`s

Joffrey · Answer

gt This was mainly useful in UseCase domain layer where the consumer presentation layer didn t care about the lower level error or why something failed but only if there was a result or not So you mean you decided on purpose that you don t want to have log reports of `ClassNotFoundException` `ThreadDeath` `NoSuchMethodError` `StackOverflowError` `ArithmeticException` `IllegalArgumentException` etc but you just want to swallow those errors and show an empty screen What do you do in the upper layer with the failure case of those results

OG · Answer

If there s specific errors that you want to handle map or do logging you still can with chaining `fold` or more logic in your `onFailure` hook It s up to you I am simplifying some stuff here just to not derail the convo from what I think was mainly the danger of not bubbling up `CancellationException` properly But quick short answer for you yes in these cases the UI required all data to be present or none and in the case of any error there was specific UI shown for retrying continuing and trying later or contact info for more help etc hope that is a bit more clear slightly smiling face

Thad · Answer

This can be misused just like try catch can be misused doesn t mean it s wrong to use it as it takes the boilerplate out of things like rethrowing cancellation exceptions which many people do from try catches anyway normal `runCatching` doesn t do anything that try catch doesn t do except provide some syntactic sugar to make things like exception recovery and chaining events easier

Thad · Answer

The way I see this best used is for cases where you want to put a general catch in to recover from an exception you didn t expect Generally I use this pattern for cases where hey I don t think I ll have an exception here but just in case I want to make sure it doesn t crash the app and gets logged Cancellation exception generally in those cases is the one exception bubble up past because it still fits the pattern of don t crash the app This is generally useful since just because you don t think an exception will happen doesn t mean you shouldn t prepare for one We generally never think crashes will happen if we didn t then we would have covered the case

CLOVIS · Answer

< Thad> It does one thing very differently `try catch` is bad if you write `catch e Throwable ` `runCatching` is always bad

CLOVIS · Answer

If you were forced to specify which exception you wanted to catch `runCatching` would be fine

Thad · Answer

My case above cites a very specific case where it is not bad so I can say without doubt that your statement is not correct it s not always bad

Thad · Answer

you are not forced to specify what kind of exception you want to catch with try catch you can just specify all of them

Joffrey · Answer

gt I am simplifying some stuff here just to not derail the convo from what I think was mainly the danger of not bubbling up CancellationException properly Actually the point about cancellation exception is the side track The main point of the question is whether using `runCatching` everywhere is a sane idea

CLOVIS · Answer

In your case it is indeed fine to catch `Throwable` In my opinion it is not a sufficient use case to add a standard library method that breaks coroutines without saying anything in its documentation tempting people to use it everywhere and this thread has shown that people do in fact use it everywhere

Joffrey · Answer

gt hey I don t think I ll have an exception here but just in case I want to make sure it doesn t crash the app and gets logged Could you please elaborate on what you intend to do in the `catch` or `onFailure` in this case Crashing the app does get logged trying to recover from any `Throwable` is wrong

CLOVIS · Answer

IMO the rule should be never use `runCatching` If you have a legitimate use case to catch Throwable then do so explicitly

Thad · Answer

You can have that opinion I m going to continue using it when appropriate

Joffrey · Answer

gt runCatching doesn t do anything that try catch doesn t do except provide some syntactic sugar to make things like exception recovery and chaining events easier It hides a catch of `Throwable` which is almost never a good idea to catch unless you re writing a framework that runs user provided code It also adds boilerplate on the access of the successful value in the happy path

OG · Answer

gt Actually the point about cancellation exception is the side track The main point of the question is whether using runCatching everywhere is a sane idea gt Gotcha I guess my 2cents here is a definite no it s not a rule style convention that should be used entirely in a coroutine based codebase Only in places where it makes sense Like the above examples just shared potentially

Thad · Answer

gt Could you please elaborate on what you intend to do in the `catch` or `onFailure` in this case Crashing the app does get logged trying to recover from any `Throwable` is wrong I intend to show an error and log it instead of having the user force exit the app it s a pretty crappy user experience the other way

CLOVIS · Answer

The only place where it does make sense is at the very top of the code hierarchy e g the main method or in your exception handler if you do UI where any exception whatsoever means everything has gone extremely wrong It s a mistake to use anywhere else

CLOVIS · Answer

But even then your UI library probably already has it somewhere so it shouldn t even appear in your code

Joffrey · Answer

gt I intend to show an error and log it instead of having the user force exit the app it s a pretty crappy user experience the other way Then this should be at the top of the hierarchy not deep in the business code

Nino · Answer

Yes for example if you catch a Throwable in Android it could be for a OutOfMemoryError which mean you re silencing the root cause of the OOM and the next code that is fine but still need some memory would crash instead

Joffrey · Answer

Also your JVM is in an undefined state at this point So if you try to keep going without crashing the app some obscure things could happen

Thad · Answer

ok so agree it shouldn t catch `Throwable` preferably but instead `Exception` that s fair

CLOVIS · Answer

Nope same issue with `Exception`

CLOVIS · Answer

You should only catch specific subclasses when you know they can happen e g `IOException`

Joffrey · Answer

At least with `Exception` you don t have the hard JVM issues but still it s not advised to catch everything But in any case `runCatching` catches `Throwable` so it settles the question about using `runCatching` in many places in business code

CLOVIS · Answer

For example it s probably not a good idea to catch `IllegalArgumentException` If you ever get one it means your code is wrong There s no recovering from that Also `CancellationException` is an `Exception` so you re still breaking coroutines

Thad · Answer

How does that cover the case of I don t expect any errors By definition if I don t think any exceptions can happen I don t know what ones do

Joffrey · Answer

If you don t know what makes you think you know how to recover

CLOVIS · Answer

You should only have a catch everything code at a single place at the very start of the program ideally the `main` method or immediately after that Everywhere else in your codebase you should not catch stuff you re not aware of

Thad · Answer

Not if you do the `runCatchingIgnoreCanceled` trick < OG> posted earlier But it s up to the consumer to do the right things with the errors they are catching

Thad · Answer

Ok I still prefer to not crash the app when I could just post an error but honestly this I think has devolved to an opinion argument and you are entitled to your opinion It is fine for it to be in the standard lib and you just don t have to use is ` ` is a kotlin feature that really should never or rarely be used but I don t think I ve ever heard someone argue it shouldn t exist just because they don t think it s used often If you don t like it you don t have to use it I ll continue using it safely though I might make a new version of it to cover the Throwable case you accurately pointed out I forgot it caught more than exception

Nino · Answer

` ` helps migrating code from Java smile

Joffrey · Answer

gt is a kotlin feature that really should never or rarely be used but I don t think I ve ever heard someone argue it shouldn t exist just because they don t think it s used often I think this was added for pragmatism but I also believe no reasonable application should use it It s always superior to use ` error reason why you believe it s never null ` Except in code golf

CLOVIS · Answer

` ` is an explicit mark for the reviewer that they should pay attention to that piece of the code Most of use are taught in school to be very careful around `catch e Throwable ` for good reason Most of us don t realize how dangerous `runCatching` is which is its main issue

Joffrey · Answer

gt Ok I still prefer to not crash the app when I could just post an error We don t disagree on this What we re saying is that you should prevent the app from crashing in a central place show the error popup and then still exit For this you don t need `runCatching` nor `catch Throwable ` anywhere in the business code just in one place close to the entrypoint of your program

OG · Answer

Whoo this thread is moving miles a minute from when I last checked If we put aside the runCatching vs traditional try catch for a moment In a world where you re using just try catch to catch specific errors when calling suspend functions never catching Throwable or Exception should you then always explicitly check and catch CancellationException Since CancellationException does have special meaning in coroutines world and handle them depending on your case rethrow or possibly swallow

OG · Answer

Or just never catch it and let it always bubble up

Nino · Answer

The other way around I d say Catch the kind of Exceptions you want to catch not just `Exception`

OG · Answer

Reworded my message just to make sure it s a bit more clear what I mean

Joffrey · Answer

I just leave `CancellationException` be in general If I m catching specific exceptions 99 9 of the time this doesn t include `CancellationException` so it s fine to not care about it If you do write a catch all block somewhere it s usually high enough in the hierarchy and everything will be rethrown anyway in the `catch`

CLOVIS · Answer

W r t CancellationException never catch it and let it bubble up except when you re calling functions which can throw CancellationException in another case than when you re cancelled e g withTimeout in which case it s fine to catch it but you MUST check if you re cancelled or not immediately using `ensureActive`

CLOVIS · Answer

E g this is ok ```try withTimeout 5000 someNetworkRequest toList catch e TimeoutCancellationException I m catching a subclass so if I m cancelled it will bubble up without creating zombies Here I know that the timeout failed and it s safe to recover however I want return emptyList ```

OG · Answer

Yup I figured TimeoutCancellationException would be different here yeah

Joffrey · Answer

I would say in the specific case of `withTimeout` I would just use `withTimeoutOrNull` and check for null It is simply safer because you can be sure that it s your own timeout in that case Otherwise you can have problems with nested `withTimeout`

Nino · Answer

Yes ```withTimeoutOrNull 5000 someNetworkRequest toList emptyList ```

CLOVIS · Answer

This creates zombies ```try catch e CancellationException ``` This is fine but less readable and a bit slower than catching TimeoutCancellationException ```try catch e CancellationException ensureActive your stuff ```

Nino · Answer

`null` was a nightmare in Java `null` a strength in Kotlin

CLOVIS · Answer

Even in the case of `withTimeout` where it would be safe to catch `CancellationException` there s a nicer way to do things sweat smile So just always find a way never to touch `CancellationException` everything will be easier this way

OG · Answer

Tldr runCatching is equivalent to try catch with Throwable dangerous Better to use try catch and handle specific errors you care about Typically you don t need to handle catching CancellationException the exception is TimeoutCancellationException but using withTimeoutOrNull is a better option

OG · Answer

Did I get that right

Joffrey · Answer

I think the part about timeout is a bit unclear I would rephrase to `runCatching` is equivalent to try catch with Throwable dangerous Better to use `try` `catch` and handle specific errors you care about Typically you don t need to handle catching `CancellationException` if you follow the previous point An exception to the previous point is catching `TimeoutCancellationException` when using `withTimeout` but in that case using `withTimeoutOrNull` is a better option And I would add that catching `Exception` or even `Throwable` can be useful in a very top level place in order to avoid crashes and allow to terminate the application more gracefully Catching `Exception` can also be useful if you want to rethrow a more high level exception with the original one as cause

OG · Answer

I like the rewording there on TimeoutCancellationException Thanks

ayodele · Answer

Very informative thread If I get the discussion right exceptions should bubble up to the top e g UI Meaning your `data` or `domain` layer should not contain any error catching let them throw and you handle them in the top most level e g the `ViewModel`

CLOVIS · Answer

And general error catching If you call a function that can throw exceptions it s fine to catch that specific kind of exceptions there E g if you handle files you can catch IOException if you call Ktor functions you can catch NetworkException etc But only the very top of the app is allowed to catch everything

CLOVIS · Answer

Your UI framework probably lets you register a global event error handler that does this for you though

CLOVIS · Answer

As an aside for the real reason runCatching exists my understanding is it s useful to temporarily represent exceptions as data in order to rethrow them somewhere else for example when converting synchronous functions into asynchronous ones It s meant to encapsulate errors temporarily so you can rethrow them somewhere else not to absorb them That s why I think it should have stayed an internal detail of coroutines and not public API