Good practices for error handling and propagation

[kakai248]

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. (#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> (#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: #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]

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

[matt-ramotar]

Hey @kakai248 - Thanks for your detailed feedback. Totally understand the challenges you're facing with the current approach and the desire for a more robust and type-safe way to handle custom errors.

Here are my thoughts:

1. Define a hierarchy of domain-specific error types that align with your app's requirements. This opens up mapping network, fetcher, SOT errors to more meaningful and UI-friendly errors at the domain layer.
2. When creating custom errors, use a sealed class hierarchy to represent different error cases. This provides a structured and type-safe way to handle specific error scenarios.
3. In your Fetcher impl, catch exceptions and map them to the appropriate custom error types. This abstracts away the low-level details and provides a consistent error representation across your data pipeline.
4. When collecting the response from the Store stream, use the errorOrNull<CustomErrorType>() extension function. This makes it possible to handle specific custom error types in a type-safe manner.

Re StoreReadResponse.Error.Custom losing the specific error type due to type erasure - I agree it's not ideal, but we have to balance this with the goal of maintaining backwards compatibility and avoiding breaking changes.

That said, I'm 100% open to considering non-breaking changes to improve ergonomics of type checking and error handling. Please let us know if you have suggestions or ideas.

Here's a sample:

inline fun <reified E : Any> StoreReadResponse<*>.onError(action: (E) -> Unit): StoreReadResponse<*> {
    if (this is StoreReadResponse.Error) {
        when (this) {
            is StoreReadResponse.Error.Custom<*> -> {
                val error = errorOrNull<E>()
                if (error != null) {
                    action(error)
                }
            }
            is StoreReadResponse.Error.Exception -> {
                val exception = error as? E
                if (exception != null) {
                    action(exception)
                }
            }
            is StoreReadResponse.Error.Message -> {}
        }
    }
    return this
}

sealed class AppError {
    data class NetworkError(val code: Int, val message: String) : AppError()
    data class DatabaseError(val message: String) : AppError()
    data class ValidationError(val field: String, val message: String) : AppError()
    // Add other custom errors
}

fun Throwable.toAppError(): AppError {
    return when (this) {
        is IOException -> AppError.NetworkError(code = 500, message = "Network error")
        is SQLiteException -> AppError.DatabaseError(message = "Database error")
        // ...
        else -> AppError.NetworkError(code = 500, message = "Unknown error")
    }
}

val fetcher = Fetcher.of { key: Key ->
    try {
        // Fetch data from network
    } catch (error: Throwable) {
        FetcherResult.Error.Custom(error.toAppError())
    }
}

val store = StoreBuilder.from(
    fetcher = fetcher,
    sourceOfTruth = // SOT impl
).build()

store.stream(StoreReadRequest.fresh(key))
    .collect { response ->
        when (response) {
            is StoreReadResponse.Data -> { // ... }
            is StoreReadResponse.Loading -> { // ... }
            else -> {
                response
                    .onError<AppError.NetworkError> { networkError ->
                        // Handle network error
                    }
                    .onError<AppError.DatabaseError> { databaseError ->
                        // Handle database error
                    }
                    .onError<AppError.ValidationError> { validationError ->
                        // Handle validation error
                    }
                    // Handle other custom errors
            }
        }
    }

If that's not sufficient, let me know. I'd be open to discussing breaking changes, but it would mean either a new major version of Store or introduction of a new extension lib (such as org.mobilenativefoundation.storex), which we've considered but haven't decided to move forward with.

[kakai248]

That approach is fairly similar to what we've changed our approach to. But it has this implicit knowledge about what error types a Store can return, due to not being present on the API. Considering a Store can be used by several other classes, we end up spreading this knowledge through multiple places without having the compiler validate it for us.

But we can't really solve this without changing the API.

Having said that, I honestly think it would be a good addition in order to guarantee type safety as a sealed hierarchy to represent errors is becoming more common.

On Store4, we wanted some behavior that was not supported (fallback to SoT on fresh requests and cache only requests) and ended up writing a full wrapper on top of it. It mimicked the full API but did some internal adjustments to implement those two features. We intentionally avoided that on Store5, but if we didn't, we could hide the cast there and maintain the error generic ourselves. What were the ideas around an extension lib? A new API surface that hides away the error casting?