Ezike / Baking-App-Kotlin

Android architecture sample with dynamic feature modularisation, clean architecture with MVI (Uni-directional data flow), dagger hilt, DFM Navigation, kotlin coroutines with StateFlow and Exo player.
Apache License 2.0
464 stars 85 forks source link

Retrofit Coroutine Exception Handling #46

Closed mochadwi closed 4 years ago

mochadwi commented 4 years ago

Does the current view state handle retrofit exception? E.g: 4xx or 5xx Retrofit error response/body

CMIIW, I cannot find try-catch blocks in your ViewModel, Processor, Remote & Repository to handle that

mochadwi commented 4 years ago

Our backend usecase has a custom body response using http status code for 4xx & 5xx

E.g image:

Screen Shot 2020-08-08 at 8 16 24 PM
Ezike commented 4 years ago

@mochadwi Yes it does handle exceptions. When using flow, you handle exceptions by using the catch operator which will catch exceptions that occur upstream. The catch operator doesn't handle exceptions for any operations added after it. For example, in the RecipeActionProcessor, you'll find this code:

private val refreshData: Flow<RecipeViewResult>
        get() = recipes.map { recipes ->
           ...
        }.onStart {
            emit(RefreshRecipesResult.Refreshing)
        }.catch { cause: Throwable ->
            emit(RefreshRecipesResult.Error(cause))
        }

Check out this article too for more context. https://medium.com/@elizarov/exceptions-in-kotlin-flows-b59643c940fb

mochadwi commented 4 years ago

Ah, I see. So the error catching was built-in when using Flow.

After reading your code, the .catch lambda exist on *ActionProcessor and returned with data class Error(val cause: Throwable) that explains everything!

mochadwi commented 4 years ago

In result, our code is much cleaner now without try-catch blocks!

Ezike commented 4 years ago

Ah, I see. So the error catching was built-in when using Flow.

After reading your code, the .catch lambda exist on *ActionProcessor combined with data class Error(val cause: Throwable) that explains everything!

yea exactly

mochadwi commented 4 years ago

Thanks and really appreciate it!

Ezike commented 4 years ago

In result, our code is much cleaner now without try-catch blocks!

Yea with flow you can use the catch operator, but with regular suspending functions, you still need to use a try / catch

mochadwi commented 4 years ago

In result, our code is much cleaner now without try-catch blocks!

Yea with flow you can use the catch operator, but with regular suspending functions, you still need to use a try / catch

Ah, I see. It's better to return a Flow (when using catch operator) instead of using suspend modifiers?

mochadwi commented 4 years ago

I've read in elizarov medium article and article that compare rx & coroutine here: http://disq.us/p/259lped

And my PoV was, like this:

fun returnListUseFlow(): Flow<List<Abc>>>

suspend fun returnSinglePojo(): Abc

@Ezike after you mention about suspend modifiers does that means, we have to use like this as well:

fun returnSinglePojo(): Flow<Abc>

CMIIW

Ezike commented 4 years ago

In result, our code is much cleaner now without try-catch blocks!

Yea with flow you can use the catch operator, but with regular suspending functions, you still need to use a try / catch

Ah, I see. It's better to return a Flow (when using catch operator) instead of using suspend modifiers?

You can only use the catch operator with flow. I guess if you're using mvi architecture you do need to wrap your suspending retrofit calls with flow

Ezike commented 4 years ago

I've read in elizarov medium article and article that compare rx & coroutine here: http://disq.us/p/259lped

And my PoV was, like this:

fun returnListUseFlow(): Flow<List<Abc>>>

suspend fun returnSinglePojo(): Abc

@Ezike after you mention about suspend modifiers does that means, we have to use like this as well:

fun returnSinglePojo(): Flow<Abc>

CMIIW

If you're not wrapping your network call in a flow, you need to use try catch to handle exceptions. There's also a runCatching function that returns a result type that could either be a success or failure if you don't want to write try/catch blocks

mochadwi commented 4 years ago

There's also a runCatching function that returns a result type that could either be a success or failure if you don't want to write try/catch blocks

Awesome, noted will using runCatching for this usecases.

I guess if you're using mvi architecture you do need to wrap your suspending retrofit calls with flow Ah, nice. We're using this extensions:

// extensions
fun <R> suspendAsFlow(block: suspend () -> R): Flow<R> = block.asFlow()

// repository
suspend fun doAbc(): Abc

// usecase
override fun doSomething(): Flow<Abc> = suspendAsFlow { repository.doAbc() } 

Or does flow already has extensions for transforming from suspend into Flow? @Ezike

Ezike commented 4 years ago

Ah, nice. We're using this extensions:

// extensions
fun <R> suspendAsFlow(block: suspend () -> R): Flow<R> = block.asFlow()

// repository
suspend fun doAbc(): Abc

// usecase
override fun doSomething(): Flow<Abc> = suspendAsFlow { repository.doAbc() } 

Or does flow already has extensions for transforming from suspend into Flow? @Ezike

This is nice. You can also just use the flow builder and call your suspending function inside...

override fun doSomething(): Flow<Abc> = flow { emit(repository.doAbc()) }
mochadwi commented 4 years ago

Agreed!

Previously we're using flow builder and after a while searching on the internet, then we're using the extension to reduce boilerplate, as stated in Kotlin Flow code it has same purpose:

Screen Shot 2020-08-08 at 8 53 50 PM

Or using method references approach:

override fun doSomething(): Flow<Abc> = repository::doAbc.asFlow()
mochadwi commented 4 years ago

Closing this issue, because it already answered our usecase. Thanks a lot & much appreciate it once again @Ezike

Ezike commented 4 years ago

Closing this issue, because it already answered our usecase. Thanks a lot & much appreciate it once again @Ezike

You're welcome