icerockdev / moko-mvvm

Model-View-ViewModel architecture components for mobile (android & ios) Kotlin Multiplatform development
https://moko.icerock.dev/
Apache License 2.0
994 stars 95 forks source link

viewModelScope.launch {...} not executed again when exception occurred #249

Open PhilippNowak96 opened 10 months ago

PhilippNowak96 commented 10 months ago

If you use viewModelScope.launch {} with a CoroutineExceptionHandler and an exception occurs, the same function does not run anymore when triggered again.

Repro

class AppViewModel : ViewModel() {
    private val exceptionHandler = CoroutineExceptionHandler { _, throwable ->
        Logger.e("Exception caught", throwable)
    }
    fun doSomething() {
        Logger.i("Trying to do something...")

        viewModelScope.launch(exceptionHandler) {
            Logger.i("I did it!")

            throw Exception()
        }
    }
}

(Logger is from implementation("co.touchlab:kermit:2.0.0-RC5"))

The doSomething function is triggered on compose button click. First run outputs as expected:

Trying to do something...
I did it!
Exception caught
java.lang.Exception
    at AppViewModel$doSomething$1.invokeSuspend(AppViewModel.kt:16)
    at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
        [...]

If you then click the button again, only Trying to do something... appears, so the viewModelScope.launch is not executed anymore.

If you change the doSomething() function to use CoroutineScope directly, everything works as expected:

fun doSomething() {
    Logger.i("Trying to do something...")
    CoroutineScope(Dispatchers.Main).launch(exceptionHandler) {
        Logger.i("I did it!")
        throw Exception()
    }
}
msomu commented 9 months ago

This is happening to me as well in a similar way, did you manage to find a solution around this?

PhilippNowak96 commented 9 months ago

The only way I found was the one I mentioned above. Use CoroutineScope(Dispatchers.Main).launch(exceptionHandler) {...} directly. In addition you can take the Job it returns and make sure to cancel it by yourself in the ViewModel's onCleared method. That should put you on the safe side.

shlee85r commented 8 months ago

Cause

The mvvm.viewmodel viewModelScope context does not include a SupervisorJob() which means if the job executed through viewModelScope is canceled, its child jobs will also be canceled.

Solution

private val ioDispatchers = SupervisorJob() + Dispatchers.IO

viewModelScope.launch(ioDispatchers) {
            block() // Even if an exception occurs, viewModelScope will not be canceled.
        }
DjuroRad commented 6 months ago

I have the similar issue. Using runcatching{}.onfailure{} can help in this case but this is not idea as it breaks structured concurrency since Cancellation exception is captured. You can always rethrow it but then why have structured concurrency 🤷 Is someone planning to fix this?

DjuroRad commented 6 months ago

Cause

The mvvm.viewmodel viewModelScope context does not include a SupervisorJob() which means if the job executed through viewModelScope is canceled, its child jobs will also be canceled.

Solution

private val ioDispatchers = SupervisorJob() + Dispatchers.IO

viewModelScope.launch(ioDispatchers) {
            block() // Even if an exception occurs, viewModelScope will not be canceled.
        }

This is not the solution. It is highly discouraged to use custom Job instances when launching new coroutines. Passing a job instance to launch as a coroutine context will override its parent job, not the newly created coroutine's job making it an entry point to unstructured concurrency. These top-level coroutines should be able to be managed through CoroutineExceptionHandler instance.

Here Roman Elizarov's article about it in more depth