https://kotlinlang.org logo
b

Bhumil

07/14/2023, 1:58 AM
Hi all, We're currently using Kotlin's Result type for exception handling across different layers, propagating errors from the source to the ViewModel. Here's an example of our approach:
Copy code
// ViewModel
fun doSome() { 
    repo.doSome().onSuccess { // handle success }.onFailure { // handle error }
}

// Repository
fun doSome(): Result<Some> { 
    return runCatching { 
        // do something that might fail 
    }
}
We're planning for a change where exceptions are directly thrown and caught in the ViewModel instead, removing the Result type. What are your thoughts on this? How do you handle exceptions across layers in your projects?
Looking at NowInAndroid, it doesn’t have good exception handling examples. It doesn’t deal with exception handling in VM at all (try/catch or Result or anything else) or in repos. But it does seem to hand the datastore layer responsibility to handle the exceptions associated with it, but it also means VM doesn't know about the error so it can't handle it to present something to the user.
p

Pablichjenkov

07/14/2023, 2:51 AM
I prefer to have my own Result class or actually sealed class.
Copy code
sealed class Result<out T> {
    class Error(val error: BaseError) : CallResult<Nothing>()
    class Success<T>(val value: T) : CallResult<T>()
}
May look a bit verbose because you will have to map from result to result per each layer but is very robust. Let say you have
ViewModel/UseCase/Service/Api
Then the mapping will go something like:
Result<Api> -> Result<Service> -> Result<UseCase> -> VM
Throwing and catching is more of a Java old design thing. If you forget to catch it will be a bad experience for an end user. My rule is leave exceptions to the system or the language not your business logic.
b

Bhumil

07/14/2023, 5:30 AM
Just curious why do you prefer to create your own Result class rather than using built-in Result class given it allows lot more additional features?
p

Pablichjenkov

07/14/2023, 5:57 AM
I like the
sealed
class one because the way you do pattern matching with the
when
clause/statement. The built-in one is not a sealed class so the way you parse the encapsulated value is a bit different. It has some extensions to pull the value out, or to do an action depending on value/exception. It kind of forces you to work with exceptions when failure and that is what I don’t want. With the sealed one I model my failure to different errors related to my business domain.
👍 1
j

Jacob Ras

07/14/2023, 8:32 AM
@Bhumil interesting question. Personally, I never use
Result
in my repositories. If my repository thinks something failed in such a manner that an exception should be thrown (e.g. a network call failed): great. I call the repository from a
UseCase
class, which returns the
Result
. The advantage of this is that your repository layer stays "clean". Just a method with a result or an exception if things go wrong. Another is that combining several repository calls, even though
Result
has nice chaining methods, are still easier just called in a row from a
UseCase
, while having the big result wrapper around it. But, at my company we're looking into the
Either
setup, specifically the one from the ArrowKt library. One big advantage is, like @Pablichjenkov also mentions about his setup, that you can do sealed matching. If you get an exception, it can be anything and you're still left to communicate from your repository docs to your use case or vm docs what exceptions that might be. With a unique error type, though, you can make use of sealed matching and you'll never miss any domain error. I like both setups and will see how both work out.
p

Pablichjenkov

07/14/2023, 2:02 PM
Either is good. But not sure if you can only import just the Either stuff or you have to bring the full ArrowKt library. In which case will be an overkill to just use Either.
j

Jacob Ras

07/14/2023, 2:04 PM
True. We decided to import the full library to allow devs to use the extensions on Either. We don't worry too much about the size as most of it will be stripped away by R8.
👍 1
c

CLOVIS

07/19/2023, 1:04 PM
Do not use kotlin.Result for exception handling. Result is meant for exception transmission where all exceptions are rethrown somewhere else. If you don't rethrow them, you will have issues. For example, if you use coroutines,
runCatching
creates zombie coroutines which continue running even after being cancelled (good luck trying to debug that). This is a topic that comes back every two or three weeks in this slack. Use normal exceptions, use custom sealed classes, use Arrow's Either, but do not use
runCatching
.
👍 1
you have to bring the full ArrowKt library
The Arrow team has released Arrow 1.2, in which they deprecated a lot of stuff. Once 2.0 arrives (hopefully in less than a year), all deprecated functions will be removed. This will reduce the size by a factor of 2–3. See their talk at KotlinConf for more details.
j

Jacob Ras

07/19/2023, 1:06 PM
Absolutely right! I forgot to mention in my message above that we use our own wrappers to create the Result classes, which re-throw
CancellationException
, to not interfere with coroutines (I wrote details about that here: https://medium.com/proandroiddev/resilient-use-cases-with-kotlin-result-coroutines-and-annotations-511df10e2e16)
c

CLOVIS

07/19/2023, 1:56 PM
@Jacob Ras Note that rethrowing
CancellationException
is not right in all cases either, see https://betterprogramming.pub/the-silent-killer-thats-crashing-your-coroutines-9171d1e8f79b
The point is, never do anything with CancellationException or any of its supertypes, because it's hard and tricky.
j

Jacob Ras

07/19/2023, 2:16 PM
Article sounds interesting, but I'm not seeing the issue. Wait, trying a bit more.
a “rogue cancellation exception.” The cancellation exception thrown when you call
await
on a cancelled task might be the most common example
That is how the cancellation mechanism works, why is it considered "rogue"? I see the example code (with
doStuff1()
and
2()
), but if I change that to re-throw the
CancellationException
, it still works as expected: both issues propagate the cancellation all the way up.
Edit: I'm not following that example at all. It catches the cancellation exception, and suppresses it by not re-throwing it. Instead, it calls
ensureActive()
to only cancel if the outer coroutine is cancelled. I see that it "fixes" how cancelled nested coroutines propagate upwards, but in my opinion this deliberately breaks structured concurrency. I can proof that, by removing the try/catch and letting the code run normally (i.e. printing a statement after calling a cancelled or non-cancelled sub-coroutine). Inner coroutine cancellation = outer coroutine cancellation. The suggested workaround changes that behaviour. 🤷‍♂️ I don't know man. Interesting article regardless, love reading about this 🙂
3 Views