Should we merge Error.Message and Error.Exception?

[eyalgu]

We've received feedback from alpha users that StoreResponse has too many subclasses which make it annoying to handle.

I'm not sure if there's a clean way to merge them together if we want to support non exception error handling as a first class citizen.

The "easy" way to do it would be to make StoreResponse generic on the error type as well:

sealed class StoreResponse<out DATA, out ERROR> {
  class Data<out DATA> : StoreResponse<DATA, Nothing>()
  class Error<out ERROR> : StoreResponse<Nothing, ERROR>()
}

[ferinagy]

I must say, that I am not a fan of this change. Especially StoreBuilder.from() now with 4 parameters...

I would still vote for just removing a StoreResponse.Error.Message. After first a deeper look, I see that if there is a fetcher that returns FetcherResult.Message, it will be mapped to the corresponding StoreResponse.Error.Message state. But if someone wants to use some network call that returns some NetworkResult, it still needs to be mapped to a FetcherResult. Then it is still possible to map an error with a message to a StoreResponse.Error(Throwable(message)).

If that is a no go, then I would rather keep current state than adding more generic parameters.

[eyalgu]

Thanks for the feedback and for helping get the best API we can!

The reason I don't want to limit error type to be only exception is that there's a general move away from using exception to report errors from things like network etc.

as for the concern re StoreBuilder.from - Throwable should be inferred for users who use exceptions because Fetcher#of hard codes Error = Throwable

val store1 = StoreBuilder.from(Fetcher.of {key : Int -> key.toString() }).build()

val sot = SourceOfTruth.of(
    nonFlowReader = { k: String -> k },
    writer = {_: String, _: String -> Unit }
)
val store2 = StoreBuilder.from(Fetcher.of {key : String -> key }, sot).build()

[eyalgu]

but yeah, the extra generic is definitely a downside of this change that I want to have a bigger discussion on.

[digitalbuddha]

I'm with @ghus-raba StoreResponse.Error(Throwable(message)) seems reasonable enough to handle messages as StoreResponse.Error. 1-2 generics was ok 4 is a bit more than what I'd want from a user experience

[yigit]

+1.
Also, what is the problem w/ having their own exception classes?
They can even write extension fields to StoreResponse that will convert the error.

My vote is to keep it as it, making error generic complicates the APIs even further and I don't think it is worth it. At least not in this version.

[JavierSegoviaCordoba]

I like the OP sealed class which is similar to Either.

I would like something similar to:

sealed class StoreResponse<out D, out E> {
  data class Data<out D>(val data: D, val isLoading:  Boolean) : StoreResponse<D, Nothing>()
  data class Error<out D, out E>(val error: E, val fallbackData: D?, val isLoading: Boolean) : StoreResponse<D, E>()
}

Maybe isLoading property could be added to StoreResponse. If not, a val StoreResponse.isLoading: Boolean extension function should be great.

With that sealed class you could do all possible escenarios:

• success with no data
• success with no data + loading
• success with data + loading
• success with data
• error
• error + loading
• error + loading + data
• error + data

I like the generic error approach but if it can be a problem, it could be:

sealed class StoreResponse<out D> {
  data class Data<out D>(val data: D, val isLoading:  Boolean) : StoreResponse<D>()
  data class Error<out D>(val error: Throwable, val fallbackData: D?, val isLoading: Boolean) : StoreResponse<D>()
}

In my case I would like to abstract all datasources using domain models (which implies using domain error models too instead of propagate exceptions). If Store could hang error models instead of only exceptions should be great because probably I could avoid mapping the Throwable to some generic error later instead of using directly the abstractions.

[digitalbuddha]

closing for now, might approach again in next version of store. StoreResponse.Error(Throwable(message)) gives users the ability to map any throwable/response type toa StoreResponse.Error