MobileNativeFoundation / Store

A Kotlin Multiplatform library for building network-resilient applications
https://mobilenativefoundation.github.io/Store/
Apache License 2.0
3.16k stars 197 forks source link

Good practices for error handling and propagation #610

Closed kakai248 closed 6 months ago

kakai248 commented 6 months ago

We've been trying to improve our error handling regarding Store. Use case is fairly straightforward, server may return an error response and we want to transport this data from the fetcher to other layers. We can have some specific handling for specific errors that would fit better in other places than inside fetcher.

Currently, StoreReadResponse has the following API:

sealed class StoreReadResponse<out Output> {
    ...
    sealed class Error : StoreReadResponse<Nothing>() {
        data class Exception(
            val error: Throwable,
            override val origin: StoreReadResponseOrigin
        ) : Error()

        data class Message(
            val message: String,
            override val origin: StoreReadResponseOrigin
        ) : Error()

        data class Custom<E : Any>(
            val error: E,
            override val origin: StoreReadResponseOrigin
        ) : Error()
    }
    ...
}

This separation makes it a bit awkward to handle because if we consider the Store to be sort of a blackbox, we'd have to handle all cases even knowing they won't really happen. I'd argue that Message is not really helpful and we could at least always fallback to Exception. (https://github.com/MobileNativeFoundation/Store/issues/231)

Exception has other issues though. Error handling through exceptions ends up in type checking/casts and considering the blackbox view again, we shouldn't really know what exceptions can be returned because they're not part of the API. If we want to pass additional data, for example, a specific error code that comes from the server, we'll have to create a custom exception and know that we have to cast to this specific type to retrieve the information. Much weirder than using the typical IOException. I'm not really a fan of using exceptions for domain error encoding.

I'd rather work with a custom error hierarchy, such as is provided by Custom<E : Any> (https://github.com/MobileNativeFoundation/Store/pull/583). However since this generic is not passed to the Store instance, we end up having to type check/cast in totally unrelated layers knowing implementation details. It's not very different from Exception.

Also related: https://github.com/MobileNativeFoundation/Store/issues/82

So what I would like to ask is what should be the pattern for handling and propagating errors when using Store? Are there any open source examples that we could look at? We're feeling that our current approach (using exceptions and type checking/cast in different layers) ends up sort of abusing implementation details and it does not really feel safe to use.

Thank you!

matt-ramotar commented 6 months ago

Hey @kakai248 - Sorry to be slow. This is on my radar, but I've been very busy. Will try to get you a sample this weekend