Store: Handle fetchers that don't throw an exception

Exceptions aren’t good. I think the Kotlin community is coalescing around returning a sealed class with error information instead of throwing an exception to indicate that a function has an error. That’s what Store does with the StoreResponse class. However, a StoreResponse.Error object will only be returned if the fetcher throws an exception. Now the libraries commonly used by a fetcher, such as Retrofit or Ktor, throw exceptions on error (bad Ktor, no biscuit). But what if someone wants to use a library that returns a sealed class to indicate success or failure?

For example, if MyLibrary returns a MyLibraryResult.Success or a MyLibraryResult.Error, how should the fetcher or Store handle it?

The app could define a custom exception which the fetcher throws when it receives an error, but we’re trying to get away from exceptions. Introducing an exception between two libraries that don’t use exceptions doesn’t sound right.
The fetcher could return the MyLibraryResult object (whether or not there was an error), but then Store doesn’t know there was a failure. Also the persister has to deal with unwrapping the data while storing it, and handling any errors.
There could be some direct way for the fetcher to tell Store there was a failure. The fetcher lambda could be an extension function on an interface, perhaps with a reportError() method. If this is called then the fetcher’s return value is ignored and it is treated as an error.
The fetcher returns a StoreResponse.Error object, which Store detects and treats the same as if an exception was thrown.
The StoreBuilder functions, in addition to the key and output types, has a third type for errors. If the fetcher returns an object of this class, it treats it as an error. For example: StoreBuilder.fromNotFlow<String, Data, MyLibraryResult.Error>(). If the fetcher succeeds, it unwraps the data from the result class and returns it. If it fails, it returns an error object.

The StoreResponse class would need to be modified to handle this new kind of error. Maybe there are two error subclasses, one for exceptions and one for non-exceptions. Maybe StoreResponse.Error can contain either a Throwable or some other error class.

About this issue

Original URL
State: closed
Created 4 years ago
Reactions: 3
Comments: 19 (9 by maintainers)

Most upvoted comments

I don’t think in the world of Kotlin Flow, exceptions are “exceptional”. In fact, the whole flow works on exceptions as a cancellation mechanism. Exceptions are the only way to stop a collections, e.g. this is the implementation of flow.first(): https://github.com/Kotlin/kotlinx.coroutines/blob/master/kotlinx-coroutines-core/common/src/flow/terminal/Reduce.kt#L88

public suspend fun <T> Flow<T>.first(): T {
    var result: Any? = NULL
    try {
        collect { value ->
            result = value
            throw AbortFlowException(NopCollector)
        }
    } catch (e: AbortFlowException) {
        // Do nothing
    }

    if (result === NULL) throw NoSuchElementException("Expected at least one element")
    return result as T
}

As for why Store does not throw when an error happens is because we do not want to stop the stream when an error happens. The practical use case is data driven UIs. When you have some code like this:

store.stream(StoreRequest.cached(userId, refresh=true)).collect {
  // update UI
}

you don’t want stream to be cancelled just because network request failed. It should still send new values if data is fetched for another request or simply because data changed on disk. So from Store’s perspective, the fact that fetch failed does not mean an error to cancel the stream, it only is an event that denotes the attempt to refresh has failed. Of course, if the desired behavior is to cancel it, you can always apply a transformation or use one of the methods that flow data instead of events.

yigit on Feb 3, 2020

One additional alternative is that the builder can accept an additional parameter: fetchResultDecoder: (Input) -> StoreResult<UnwrappedInput> (similar to adding a source of truth, this will change the Value type of the resulting store).

The benefits of this approach is:

It’s more discoverable than updating the documentation of fetcher to say that, if it returns StoreResult.Error it will be treated differently.
It doesn’t force the user to use StoreResult as Input for their sourceOfTruth, or worse yet, as the Value of the resulting store if there’s no source of truth.

The main downside is that we’re adding another concept to the API, but given that it’s an optional parameter in the builder that might not be too bad.

Thoughts?

eyalgu on Jan 30, 2020