MobileNativeFoundation / Store

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

Support custom error types #583

Closed matt-ramotar closed 10 months ago

matt-ramotar commented 12 months ago

Hello :spock-hand: ! Does someone have any guidance/sample project on how to handle network/http errors with Store? Currently the network layer in our Android app takes care of catching exceptions, parsing error payloads and returning a custom monad class like Result<NetworkError, T>> . Looking at StoreReadResponse it seems that using Throwable or String is the only way to go. Thanks for your answers!

https://kotlinlang.slack.com/archives/C06007Z01HU/p1699457424194509

matt-ramotar commented 12 months ago

@digitalbuddha We would need to fix tests obviously but I wanted to get something up first for discussion. Adding a generic to StoreReadResponse and FetcherResult would introduce breaking changes I know we don't want. Thoughts?

borsini commented 12 months ago

@matt-ramotar thanks for bringing up implementation ideas so quickly. Would it be possible for the Custom error to accept any type instead of only Throwable? As an example, our network layer uses a sealed interface with no Throwable inheritance. Something like this

sealed interface NetworkError { 
    data class StatusCodeError(val statusCode: Int, val error: CustomApiError?) : NetworkError
    data class UnknownError(val throwable: Throwable) : NetworkError
    data object NetworkUnavailableError : NetworkError
}
matt-ramotar commented 12 months ago

@borsini Seems fine to me 👍🏽

borsini commented 10 months ago

Hello @matt-ramotar, I checked how this PR was doing and came up with an idea. I created this little draft PR to illustrate my thinking. What if, instead of having a sealed hierarchy, Store use only one generic Error type.

Let me know what you think 😄

matt-ramotar commented 10 months ago

Sorry to be slow @borsini, will try to get this merged by end of week

borsini commented 10 months ago

No worries @matt-ramotar this is open source 🙂