Hi all, We're currently using Kotlin's Result typ...

# android

[Bhumil]

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:

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

[Bhumil]

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.

[Pablichjenkov]

I prefer to have my own Result class or actually sealed class.

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.

[Bhumil]

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?

[Pablichjenkov]

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.

[Jacob Ras]

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

[Pablichjenkov]

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.

[Jacob Ras]

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.

[CLOVIS]

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 .

[CLOVIS]

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.

[Jacob Ras]

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

[CLOVIS]

@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

[CLOVIS]

The point is, never do anything with CancellationException or any of its supertypes, because it's hard and tricky.

[Jacob Ras]

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 🙂