MvRx (Mavericks?) 2.0 Fail states from execute()

[Minirogue]

https://github.com/airbnb/MvRx/blob/609a797954863ff7815cb697a8964f96db1d867e/mvrx/src/main/kotlin/com/airbnb/mvrx/MavericksViewModel.kt#L211

I was reviewing a PR in our codebase that uses setState to manually set the values of Async objects when using coroutines with the current version of MvRx. With 2.0, we'll be able to use execute() to generate the Async objects for us. However, I realized that error states still depend on an Exception being thrown. This is a little out of line with Kotlin's idioms for error handling: https://medium.com/@elizarov/kotlin-and-exceptions-8062f589d07.

I've been catching API exceptions at point of contact and either converting them to null or to my own wrapping class. This means that I'd get a Success out of execute at the end that I'd have to parse and turn into a Fail.

I'll probably end up writing an extension function to handle values of my result/error wrapping class, but it seems like there should be an executeNullIsFailure() (but with a better name of course) or an option to treat null responses as failures directly in an execute boolean argument.

A more general solution might be to have some way to pass in a (T) -> Boolean to determine what values should be marked as a Fail.

[gpeal]

@Minirogue You would lose the underlying cause of the error which is often useful both for logging and for displaying more specific errors to the user.

[Minirogue]

Then how about (T) -> Throwable? where null means no error: https://github.com/Minirogue/MvRx/commit/2f0dbcb37f8224bbfddfa9972ad27c4ee84ed0a2

[gpeal]

@Minirogue At that point, you might as well do the unwrapping in your suspend function.

[elihart]

I don't know that the article you linked to shows that the Mavericks approach is out of line with Kotlin best practices. If anything it might agree with our approach What it has to say about coroutines specifically is at the bottom:

All exceptions should automatically percolate to the top-level of the application to get centrally handled. The key design principle is that no exception should be lost, all bugs should get reported to developers.

Thus, the same general advice applies to exceptions and coroutines: don’t use exceptions if you need local handling of certain failure scenarios in your code, don’t use exceptions to return a result value, avoid try/catch in general application code, implement centralized exception-handling logic

execute acts as a central, top level boundary to catch exceptions, and prevents general application code from needing to catch them (like you say you are doing now).

I don't think new kotlin code should be designed to throw exceptions necessarily, but the current approach seems in line with how to interop with existing code that throws exceptions.

To handle error results with execute in a more kotlin standard way I think we would need a standard monad wrapper type, like Try or Result. Kotlin does have a Result class for coroutines results but it is still experimental.

Maybe we can support it as experimental to have an execute function where the receiver is a Result<T> class, or you can make your own extensions for it. Many people will have their own unique wrapper classes for results so it is hard for Mavericks to standardize on right now.

[Minirogue]

Result is actually in the standard library (although it looks like it was originally only in the coroutines library). It looks like creating an execute function for Result would be the ideal solution, but it's forcibly discouraged as a return type. Technically it's not "experimental", which Kotlin uses to mean it's available for use, but the API might change. If it were, I would ask to add the execute function for it.

I guess a custom wrapper class and an extension execute function is going to be the solution for me. Thanks for your time.

[langsmith]

Maybe we can support it as experimental to have an execute function where the receiver is a Result class

@elihart , this would really be great to add to the Mavericks library