From 70da1dc4c9593b4087edf944c210ba736a7c7f20 Mon Sep 17 00:00:00 2001 From: Nek-12 Date: Thu, 21 Dec 2023 22:52:54 +0300 Subject: [PATCH 1/4] minor refactoring --- .../android/compose/preview/EmptyReceiver.kt | 2 ++ .../respawn/flowmvi/sample/CounterModels.kt | 2 +- .../flowmvi/sample/compose/ComposeScreen.kt | 4 ++-- .../flowmvi/sample/compose/CounterContainer.kt | 10 ++++------ .../flowmvi/sample/view/CounterActivity.kt | 6 +++++- app/src/main/res/values/strings.xml | 2 +- .../respawn/flowmvi/modules/ActionModule.kt | 18 +++++++++--------- .../respawn/flowmvi/modules/IntentModule.kt | 10 +++++----- .../respawn/flowmvi/modules/PluginModule.kt | 10 +++++----- .../pro/respawn/flowmvi/modules/StateModule.kt | 4 ++-- .../kotlin/pro/respawn/flowmvi/test/TestDsl.kt | 15 +++++++-------- 11 files changed, 43 insertions(+), 40 deletions(-) diff --git a/android-compose/src/main/kotlin/pro/respawn/flowmvi/android/compose/preview/EmptyReceiver.kt b/android-compose/src/main/kotlin/pro/respawn/flowmvi/android/compose/preview/EmptyReceiver.kt index 6b3a31ce..5da066a4 100644 --- a/android-compose/src/main/kotlin/pro/respawn/flowmvi/android/compose/preview/EmptyReceiver.kt +++ b/android-compose/src/main/kotlin/pro/respawn/flowmvi/android/compose/preview/EmptyReceiver.kt @@ -1,3 +1,5 @@ +@file:Suppress("DEPRECATION") + package pro.respawn.flowmvi.android.compose.preview import androidx.compose.runtime.Composable diff --git a/app/src/main/kotlin/pro/respawn/flowmvi/sample/CounterModels.kt b/app/src/main/kotlin/pro/respawn/flowmvi/sample/CounterModels.kt index c1e615d3..8c0b8eea 100644 --- a/app/src/main/kotlin/pro/respawn/flowmvi/sample/CounterModels.kt +++ b/app/src/main/kotlin/pro/respawn/flowmvi/sample/CounterModels.kt @@ -34,7 +34,7 @@ sealed interface CounterIntent : MVIIntent { sealed interface CounterAction : MVIAction { - data object ShowErrorMessage : CounterAction + data class ShowErrorMessage(val message: String?) : CounterAction data object ShowLambdaMessage : CounterAction data object GoBack : CounterAction } diff --git a/app/src/main/kotlin/pro/respawn/flowmvi/sample/compose/ComposeScreen.kt b/app/src/main/kotlin/pro/respawn/flowmvi/sample/compose/ComposeScreen.kt index 485803e6..0ae6afee 100644 --- a/app/src/main/kotlin/pro/respawn/flowmvi/sample/compose/ComposeScreen.kt +++ b/app/src/main/kotlin/pro/respawn/flowmvi/sample/compose/ComposeScreen.kt @@ -24,9 +24,9 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch import org.koin.core.parameter.parametersOf import pro.respawn.flowmvi.api.IntentReceiver -import pro.respawn.flowmvi.compose.preview.StateProvider import pro.respawn.flowmvi.compose.dsl.subscribe import pro.respawn.flowmvi.compose.preview.EmptyReceiver +import pro.respawn.flowmvi.compose.preview.StateProvider import pro.respawn.flowmvi.sample.CounterAction.GoBack import pro.respawn.flowmvi.sample.CounterAction.ShowErrorMessage import pro.respawn.flowmvi.sample.CounterAction.ShowLambdaMessage @@ -57,7 +57,7 @@ fun ComposeScreen(onBack: () -> Unit) { // consume() block will only be called when a new action is emitted (independent of recompositions) when (action) { is ShowLambdaMessage -> scaffoldState.snackbar(context.getString(R.string.lambda_message)) - is ShowErrorMessage -> scaffoldState.snackbar(context.getString(R.string.error_message)) + is ShowErrorMessage -> scaffoldState.snackbar(context.getString(R.string.error_message, action.message)) is GoBack -> onBack() } } diff --git a/app/src/main/kotlin/pro/respawn/flowmvi/sample/compose/CounterContainer.kt b/app/src/main/kotlin/pro/respawn/flowmvi/sample/compose/CounterContainer.kt index b5aa05ad..b39c4b85 100644 --- a/app/src/main/kotlin/pro/respawn/flowmvi/sample/compose/CounterContainer.kt +++ b/app/src/main/kotlin/pro/respawn/flowmvi/sample/compose/CounterContainer.kt @@ -41,12 +41,10 @@ class CounterContainer( ) val undoRedo = undoRedo(10) recover { - launch { - if (it is IllegalArgumentException) - action(ShowErrorMessage) - else updateState { - CounterState.Error(it) - } + if (it is IllegalArgumentException) + action(ShowErrorMessage(it.message)) + else updateState { + CounterState.Error(it) } null } diff --git a/app/src/main/kotlin/pro/respawn/flowmvi/sample/view/CounterActivity.kt b/app/src/main/kotlin/pro/respawn/flowmvi/sample/view/CounterActivity.kt index 72a9bf74..007d2a6d 100644 --- a/app/src/main/kotlin/pro/respawn/flowmvi/sample/view/CounterActivity.kt +++ b/app/src/main/kotlin/pro/respawn/flowmvi/sample/view/CounterActivity.kt @@ -48,7 +48,11 @@ class CounterActivity : // Handle any side effects of the UI layer here override fun consume(action: CounterAction) { when (action) { - is ShowErrorMessage -> Snackbar.make(binding.root, R.string.error_message, Snackbar.LENGTH_SHORT).show() + is ShowErrorMessage -> Snackbar.make( + binding.root, + getString(R.string.error_message, action.message), + Snackbar.LENGTH_SHORT + ).show() is ShowLambdaMessage -> Snackbar.make(binding.root, R.string.lambda_message, Snackbar.LENGTH_SHORT).show() is CounterAction.GoBack -> finish() } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 19d3081a..a0655d51 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -1,6 +1,6 @@ FlowMVI Sample - onException invoked + recovered from error: %1$s Increment counter Counter: %d Started processing your intent diff --git a/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/ActionModule.kt b/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/ActionModule.kt index c700e989..4596e14e 100644 --- a/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/ActionModule.kt +++ b/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/ActionModule.kt @@ -12,6 +12,15 @@ import pro.respawn.flowmvi.api.ActionShareBehavior import pro.respawn.flowmvi.api.DelicateStoreApi import pro.respawn.flowmvi.api.MVIAction +internal fun actionModule( + behavior: ActionShareBehavior, +): ActionModule = when (behavior) { + is ActionShareBehavior.Distribute -> DistributingModule(behavior.buffer, behavior.overflow) + is ActionShareBehavior.Restrict -> ConsumingModule(behavior.buffer, behavior.overflow) + is ActionShareBehavior.Share -> SharedModule(behavior.replay, behavior.buffer, behavior.overflow) + is ActionShareBehavior.Disabled -> ThrowingModule() +} + internal interface ActionModule : ActionProvider, ActionReceiver internal abstract class ChannelActionModule( @@ -80,12 +89,3 @@ internal class ThrowingModule : ActionModule { private const val ActionsDisabledMessage = "Actions are disabled for this store" } } - -internal fun actionModule( - behavior: ActionShareBehavior, -): ActionModule = when (behavior) { - is ActionShareBehavior.Distribute -> DistributingModule(behavior.buffer, behavior.overflow) - is ActionShareBehavior.Restrict -> ConsumingModule(behavior.buffer, behavior.overflow) - is ActionShareBehavior.Share -> SharedModule(behavior.replay, behavior.buffer, behavior.overflow) - is ActionShareBehavior.Disabled -> ThrowingModule() -} diff --git a/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/IntentModule.kt b/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/IntentModule.kt index db5649b5..1b5d9b84 100644 --- a/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/IntentModule.kt +++ b/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/IntentModule.kt @@ -8,17 +8,17 @@ import kotlinx.coroutines.yield import pro.respawn.flowmvi.api.IntentReceiver import pro.respawn.flowmvi.api.MVIIntent -internal interface IntentModule : IntentReceiver { - - suspend fun awaitIntents(onIntent: suspend (intent: I) -> Unit) -} - internal fun intentModule( parallel: Boolean, capacity: Int, overflow: BufferOverflow, ): IntentModule = IntentModuleImpl(parallel, capacity, overflow) +internal interface IntentModule : IntentReceiver { + + suspend fun awaitIntents(onIntent: suspend (intent: I) -> Unit) +} + private class IntentModuleImpl( private val parallel: Boolean, capacity: Int, diff --git a/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/PluginModule.kt b/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/PluginModule.kt index e17c36ca..e5da0831 100644 --- a/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/PluginModule.kt +++ b/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/PluginModule.kt @@ -7,7 +7,11 @@ import pro.respawn.flowmvi.api.PipelineContext import pro.respawn.flowmvi.api.StorePlugin import pro.respawn.flowmvi.plugins.AbstractStorePlugin -internal class PluginModule( +internal fun pluginModule( + plugins: Set> +): StorePlugin = PluginModule(plugins) + +private class PluginModule( private val plugins: Set>, ) : AbstractStorePlugin(null) { @@ -30,7 +34,3 @@ internal class PluginModule( block: StorePlugin.(R) -> R? ) = plugins.fold<_, R?>(initial) { acc, it -> it.block(acc ?: return@plugins acc) } } - -internal fun pluginModule( - plugins: Set> -): StorePlugin = PluginModule(plugins) diff --git a/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/StateModule.kt b/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/StateModule.kt index 01b4f162..243b3ca6 100644 --- a/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/StateModule.kt +++ b/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/StateModule.kt @@ -11,10 +11,10 @@ import pro.respawn.flowmvi.api.StateProvider import pro.respawn.flowmvi.api.StateReceiver import pro.respawn.flowmvi.util.withReentrantLock -internal interface StateModule : StateReceiver, StateProvider - internal fun stateModule(initial: S): StateModule = StateModuleImpl(initial) +internal interface StateModule : StateReceiver, StateProvider + private class StateModuleImpl(initial: S) : StateModule { private val _states = MutableStateFlow(initial) diff --git a/test/src/commonMain/kotlin/pro/respawn/flowmvi/test/TestDsl.kt b/test/src/commonMain/kotlin/pro/respawn/flowmvi/test/TestDsl.kt index ede7f8a7..17d5b7f4 100644 --- a/test/src/commonMain/kotlin/pro/respawn/flowmvi/test/TestDsl.kt +++ b/test/src/commonMain/kotlin/pro/respawn/flowmvi/test/TestDsl.kt @@ -1,6 +1,7 @@ package pro.respawn.flowmvi.test import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.Job import kotlinx.coroutines.cancelAndJoin import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.flow.firstOrNull @@ -70,11 +71,11 @@ public class StoreTestScope @Publish * Call [Store.start] and then execute [block], cancelling the store afterwards */ public suspend inline fun Store.test( - crossinline block: suspend Store.() -> Unit -): Unit = coroutineScope { + crossinline block: suspend Store.(job: Job) -> Unit +): Job = coroutineScope { val job = start(this) - block() - job.cancelAndJoin() + block(job) + job.apply { cancelAndJoin() } } /** @@ -83,12 +84,10 @@ public suspend inline fun Store Store.subscribeAndTest( crossinline block: suspend StoreTestScope.() -> Unit, -): Unit = test { +): Job = test { coroutineScope { subscribe { - StoreTestScope(this, this@subscribeAndTest).run { - block() - } + StoreTestScope(this, this@subscribeAndTest).run { block() } } } } From 80c42613e94568b593de1968bdc014d4631b8c35 Mon Sep 17 00:00:00 2001 From: Nek-12 Date: Thu, 21 Dec 2023 22:53:18 +0300 Subject: [PATCH 2/4] add unrecoverable exceptions and improve store exception handling --- .../pro/respawn/flowmvi/api/Recoverable.kt | 36 -------- .../flowmvi/api/UnrecoverableException.kt | 12 +++ .../flowmvi/exceptions/StoreExceptions.kt | 41 +++++++++ .../respawn/flowmvi/modules/PipelineModule.kt | 8 +- .../respawn/flowmvi/modules/RecoverModule.kt | 72 +++++++++++++++ .../respawn/flowmvi/modules/Recoverable.kt | 24 ----- .../pro/respawn/flowmvi/store/StoreImpl.kt | 39 +++----- .../flowmvi/test/store/StoreExceptionsText.kt | 90 +++++++++++-------- .../pro/respawn/flowmvi/util/TestStore.kt | 4 +- gradle/libs.versions.toml | 2 +- 10 files changed, 196 insertions(+), 132 deletions(-) delete mode 100644 core/src/commonMain/kotlin/pro/respawn/flowmvi/api/Recoverable.kt create mode 100644 core/src/commonMain/kotlin/pro/respawn/flowmvi/api/UnrecoverableException.kt create mode 100644 core/src/commonMain/kotlin/pro/respawn/flowmvi/exceptions/StoreExceptions.kt create mode 100644 core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/RecoverModule.kt delete mode 100644 core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/Recoverable.kt diff --git a/core/src/commonMain/kotlin/pro/respawn/flowmvi/api/Recoverable.kt b/core/src/commonMain/kotlin/pro/respawn/flowmvi/api/Recoverable.kt deleted file mode 100644 index 74eb4b76..00000000 --- a/core/src/commonMain/kotlin/pro/respawn/flowmvi/api/Recoverable.kt +++ /dev/null @@ -1,36 +0,0 @@ -package pro.respawn.flowmvi.api - -import kotlin.coroutines.CoroutineContext -import kotlin.coroutines.cancellation.CancellationException - -/** - * An entity that can [recover] from exceptions happening during its lifecycle. Most often, a [Store] - */ -@Suppress("FUN_INTERFACE_WITH_SUSPEND_FUNCTION") // https://youtrack.jetbrains.com/issue/KTIJ-7642 -public fun interface Recoverable : CoroutineContext.Element { - - @OptIn(DelicateStoreApi::class) - override val key: CoroutineContext.Key<*> get() = Recoverable - - /** - * Recover from an exception in the given context. - */ - public suspend fun PipelineContext.recover(e: Exception) - - /** - * Run [block] catching any exceptions and invoking [recover]. This will add this [Recoverable] key to the coroutine - * context of the [recover] block. - */ - @OptIn(DelicateStoreApi::class) - public suspend fun PipelineContext.catch(block: suspend () -> Unit): Unit = try { - block() - } catch (e: CancellationException) { - throw e - } catch (expected: Exception) { - if (coroutineContext[Recoverable] != null) throw expected - recover(expected) - } - - @DelicateStoreApi - public companion object : CoroutineContext.Key> -} diff --git a/core/src/commonMain/kotlin/pro/respawn/flowmvi/api/UnrecoverableException.kt b/core/src/commonMain/kotlin/pro/respawn/flowmvi/api/UnrecoverableException.kt new file mode 100644 index 00000000..4a1d0c34 --- /dev/null +++ b/core/src/commonMain/kotlin/pro/respawn/flowmvi/api/UnrecoverableException.kt @@ -0,0 +1,12 @@ +package pro.respawn.flowmvi.api + +/** + * An exception that has happened in the [Store] that cannot be recovered from. + * This is either an exception resulting from developer errors (such as unhandled intents), + * or an exception while trying to recover from another exception (which is prohibited). + * You may also use this to bypass store plugins handling this particular exception. + */ +public class UnrecoverableException( + override val cause: Exception? = null, + override val message: String? = null, +) : IllegalStateException(message, cause) diff --git a/core/src/commonMain/kotlin/pro/respawn/flowmvi/exceptions/StoreExceptions.kt b/core/src/commonMain/kotlin/pro/respawn/flowmvi/exceptions/StoreExceptions.kt new file mode 100644 index 00000000..0bc5caa3 --- /dev/null +++ b/core/src/commonMain/kotlin/pro/respawn/flowmvi/exceptions/StoreExceptions.kt @@ -0,0 +1,41 @@ +@file:Suppress("FunctionName") + +package pro.respawn.flowmvi.exceptions + +import pro.respawn.flowmvi.api.UnrecoverableException + +internal fun NonSuspendingSubscriberException() = UnrecoverableException( + message = """ + You have subscribed to the store, but your subscribe() block has returned early (without throwing a + CancellationException). When you subscribe, make sure to continue collecting values from the store until the Job + Returned from the subscribe() is cancelled as you likely don't want to stop being subscribed to the store + (i.e. complete the subscription job on your own). + """ + .trimIndent(), + cause = null, +) + +internal fun UnhandledIntentException() = UnrecoverableException( + message = """ + An intent has not been handled after calling all plugins. + You likely don't want this to happen because intents are supposed to be acted upon. + Make sure you have at least one plugin that handles intents, such as reducePlugin(). + """ + .trimIndent(), + cause = null, +) + +internal fun RecursiveRecoverException(cause: Exception) = UnrecoverableException( + message = """ + Recursive recover detected, which means you have thrown in a recover plugin or in the `onException` block. + Please never throw while recovering from exceptions, or that will result in an infinite loop. + """.trimIndent(), + cause = cause +) + +internal fun UnhandledStoreException(cause: Exception) = UnrecoverableException( + message = """ + Store has run all its plugins (exception handlers) but the exception was not handled by any of them. + """.trimIndent(), + cause = cause, +) diff --git a/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/PipelineModule.kt b/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/PipelineModule.kt index 440762cc..601b0f12 100644 --- a/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/PipelineModule.kt +++ b/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/PipelineModule.kt @@ -16,7 +16,6 @@ import pro.respawn.flowmvi.api.MVIAction import pro.respawn.flowmvi.api.MVIIntent import pro.respawn.flowmvi.api.MVIState import pro.respawn.flowmvi.api.PipelineContext -import pro.respawn.flowmvi.api.Recoverable import pro.respawn.flowmvi.api.StateReceiver import kotlin.coroutines.CoroutineContext @@ -39,12 +38,13 @@ internal inline fun T.launchPipe crossinline onAction: suspend PipelineContext.(action: A) -> Unit, crossinline onTransformState: suspend PipelineContext.(transform: suspend S.() -> S) -> Unit, onStart: PipelineContext.() -> Unit, -): Job where T : IntentReceiver, T : StateReceiver, T : Recoverable = object : +): Job where T : IntentReceiver, T : StateReceiver, T : RecoverModule = object : IntentReceiver by this, StateReceiver by this, PipelineContext, ActionReceiver { + override fun toString(): String = "${name.orEmpty()}PipelineContext" override val key = PipelineContext // recoverable should be separate. private val job = SupervisorJob(parent.coroutineContext[Job]).apply { invokeOnCompletion { @@ -55,8 +55,8 @@ internal inline fun T.launchPipe } } } - private val handler = PipelineExceptionHandler(this) - private val pipelineName = CoroutineName("${name.orEmpty()}PipelineContext") + private val handler = PipelineExceptionHandler() + private val pipelineName = CoroutineName(toString()) override val coroutineContext: CoroutineContext = parent.coroutineContext + pipelineName + job + handler + this override suspend fun updateState(transform: suspend S.() -> S) = onTransformState(transform) override suspend fun action(action: A) = onAction(action) diff --git a/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/RecoverModule.kt b/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/RecoverModule.kt new file mode 100644 index 00000000..3c3c98a2 --- /dev/null +++ b/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/RecoverModule.kt @@ -0,0 +1,72 @@ +package pro.respawn.flowmvi.modules + +import kotlinx.coroutines.CoroutineExceptionHandler +import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext +import pro.respawn.flowmvi.api.MVIAction +import pro.respawn.flowmvi.api.MVIIntent +import pro.respawn.flowmvi.api.MVIState +import pro.respawn.flowmvi.api.PipelineContext +import pro.respawn.flowmvi.api.Store +import pro.respawn.flowmvi.api.UnrecoverableException +import pro.respawn.flowmvi.exceptions.RecursiveRecoverException +import kotlin.coroutines.CoroutineContext +import kotlin.coroutines.cancellation.CancellationException +import kotlin.coroutines.coroutineContext + +/** + * An entity that can [recover] from exceptions happening during its lifecycle. Most often, a [Store] + */ +@Suppress("FUN_INTERFACE_WITH_SUSPEND_FUNCTION") // https://youtrack.jetbrains.com/issue/KTIJ-7642 +internal fun interface RecoverModule : CoroutineContext.Element { + + override val key: CoroutineContext.Key<*> get() = RecoverModule + + /** + * Recover from an exception in the given context. + */ + suspend fun PipelineContext.recover(e: Exception) + + /** + * Run [block] catching any exceptions and invoking [recover]. This will add this [RecoverModule] key to the coroutine + * context of the [recover] block. + */ + suspend fun PipelineContext.catch(block: suspend () -> Unit): Unit = try { + withContext(this@RecoverModule) { block() } + } catch (expected: Exception) { + when { + alreadyRecovered() -> throw RecursiveRecoverException(expected) + expected is CancellationException || expected is UnrecoverableException -> throw expected + else -> recover(expected) + } + } + + @Suppress("FunctionName") + fun PipelineContext.PipelineExceptionHandler() = CoroutineExceptionHandler { ctx, e -> + when { + e !is Exception || e is CancellationException -> throw e + e is UnrecoverableException -> throw e.unwrapRecursion() + ctx.alreadyRecovered -> throw e + // add Recoverable to the coroutine context + // and handle the exception asynchronously to allow suspending inside recover + // Do NOT use the "ctx" parameter here, as that coroutine context is already invalid and will not launch + else -> launch(this@RecoverModule) { recover(e) }.invokeOnCompletion { cause -> + if (cause != null && cause !is CancellationException) throw cause + } + } + } + + companion object : CoroutineContext.Key> +} + +private tailrec fun UnrecoverableException.unwrapRecursion(): Exception = when (val cause = cause) { + null -> this + this -> this // cause is the same exception + is UnrecoverableException -> cause.unwrapRecursion() + is CancellationException -> throw cause + else -> cause +} + +internal suspend fun alreadyRecovered() = coroutineContext.alreadyRecovered + +private val CoroutineContext.alreadyRecovered get() = this[RecoverModule] != null diff --git a/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/Recoverable.kt b/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/Recoverable.kt deleted file mode 100644 index ab0c51c6..00000000 --- a/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/Recoverable.kt +++ /dev/null @@ -1,24 +0,0 @@ -package pro.respawn.flowmvi.modules - -import kotlinx.coroutines.CoroutineExceptionHandler -import kotlinx.coroutines.launch -import pro.respawn.flowmvi.api.DelicateStoreApi -import pro.respawn.flowmvi.api.MVIAction -import pro.respawn.flowmvi.api.MVIIntent -import pro.respawn.flowmvi.api.MVIState -import pro.respawn.flowmvi.api.PipelineContext -import pro.respawn.flowmvi.api.Recoverable - -@OptIn(DelicateStoreApi::class) -@Suppress("FunctionName") -internal fun Recoverable.PipelineExceptionHandler( - pipeline: PipelineContext, -) = CoroutineExceptionHandler { ctx, e -> - when { - e !is Exception -> throw e - ctx[Recoverable] != null -> throw e - // add Recoverable to the coroutine context - // and handle the exception asynchronously to allow suspending inside recover - else -> with(pipeline) { launch(this@PipelineExceptionHandler) { recover(e) } } - } -} diff --git a/core/src/commonMain/kotlin/pro/respawn/flowmvi/store/StoreImpl.kt b/core/src/commonMain/kotlin/pro/respawn/flowmvi/store/StoreImpl.kt index af8a5f9e..86e450c1 100644 --- a/core/src/commonMain/kotlin/pro/respawn/flowmvi/store/StoreImpl.kt +++ b/core/src/commonMain/kotlin/pro/respawn/flowmvi/store/StoreImpl.kt @@ -8,20 +8,23 @@ import kotlinx.coroutines.job import kotlinx.coroutines.launch import kotlinx.coroutines.plus import kotlinx.coroutines.withContext -import pro.respawn.flowmvi.api.DelicateStoreApi import pro.respawn.flowmvi.api.MVIAction import pro.respawn.flowmvi.api.MVIIntent import pro.respawn.flowmvi.api.MVIState import pro.respawn.flowmvi.api.PipelineContext import pro.respawn.flowmvi.api.Provider -import pro.respawn.flowmvi.api.Recoverable import pro.respawn.flowmvi.api.Store import pro.respawn.flowmvi.api.StorePlugin +import pro.respawn.flowmvi.exceptions.NonSuspendingSubscriberException +import pro.respawn.flowmvi.exceptions.UnhandledIntentException +import pro.respawn.flowmvi.exceptions.UnhandledStoreException import pro.respawn.flowmvi.modules.ActionModule import pro.respawn.flowmvi.modules.IntentModule +import pro.respawn.flowmvi.modules.RecoverModule import pro.respawn.flowmvi.modules.StateModule import pro.respawn.flowmvi.modules.SubscribersModule import pro.respawn.flowmvi.modules.actionModule +import pro.respawn.flowmvi.modules.alreadyRecovered import pro.respawn.flowmvi.modules.intentModule import pro.respawn.flowmvi.modules.launchPipeline import pro.respawn.flowmvi.modules.observeSubscribers @@ -33,8 +36,8 @@ internal class StoreImpl( private val config: StoreConfiguration, ) : Store, Provider, - Recoverable, - StorePlugin by pluginModule(config.plugins), // store is a plugin to itself that manages other plugins + RecoverModule, + StorePlugin by pluginModule(config.plugins), SubscribersModule by subscribersModule(), StateModule by stateModule(config.initial), IntentModule by intentModule(config.parallelIntents, config.intentCapacity, config.onOverflow), @@ -72,18 +75,17 @@ internal class StoreImpl( } launch { awaitIntents { - catch { check(onIntent(it) == null || !config.debuggable) { UnhandledIntentMessage } } + catch { if (onIntent(it) != null && config.debuggable) throw UnhandledIntentException() } } } } } } - @OptIn(DelicateStoreApi::class) override suspend fun PipelineContext.recover(e: Exception) { - if (coroutineContext[Recoverable] != null) throw e - withContext(this@StoreImpl) { // add recoverable to the context - onException(e)?.let { throw it } + if (alreadyRecovered()) throw e + withContext(this@StoreImpl) { + onException(e)?.let { throw UnhandledStoreException(it) } } } @@ -92,7 +94,7 @@ internal class StoreImpl( ): Job = launch { newSubscriber() block(this@StoreImpl) - check(!config.debuggable) { NonSuspendingSubscriberMessage } + if (config.debuggable) throw NonSuspendingSubscriberException() }.apply { invokeOnCompletion { removeSubscriber() } } @@ -103,25 +105,10 @@ internal class StoreImpl( } override fun hashCode() = name?.hashCode() ?: super.hashCode() + override fun toString(): String = name ?: super.toString() override fun equals(other: Any?): Boolean { if (other !is Store<*, *, *>) return false if (other.name == null && name == null) return other === this return name == other.name } - - companion object { - - const val NonSuspendingSubscriberMessage = """ -You have subscribed to the store, but your subscribe() block has returned early (without throwing a -CancellationException). When you subscribe, make sure to continue collecting values from the store until the Job -Returned from the subscribe() is cancelled as you likely don't want to stop being subscribed to the store -(i.e. complete the subscription job on your own). - """ - - const val UnhandledIntentMessage = """ -An intent has not been handled after calling all plugins. -You likely don't want this to happen because intents are supposed to be acted upon. -Make sure you have at least one plugin that handles intents, such as reducePlugin(). - """ - } } diff --git a/core/src/jvmTest/kotlin/pro/respawn/flowmvi/test/store/StoreExceptionsText.kt b/core/src/jvmTest/kotlin/pro/respawn/flowmvi/test/store/StoreExceptionsText.kt index 48dc1516..182bc4d4 100644 --- a/core/src/jvmTest/kotlin/pro/respawn/flowmvi/test/store/StoreExceptionsText.kt +++ b/core/src/jvmTest/kotlin/pro/respawn/flowmvi/test/store/StoreExceptionsText.kt @@ -8,14 +8,11 @@ import io.kotest.matchers.collections.shouldContainExactly import io.kotest.matchers.nulls.shouldNotBeNull import io.kotest.matchers.shouldBe import kotlinx.coroutines.CancellationException -import kotlinx.coroutines.cancelAndJoin -import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.currentCoroutineContext import kotlinx.coroutines.launch -import pro.respawn.flowmvi.api.DelicateStoreApi -import pro.respawn.flowmvi.api.Recoverable import pro.respawn.flowmvi.dsl.intent import pro.respawn.flowmvi.dsl.send +import pro.respawn.flowmvi.modules.RecoverModule import pro.respawn.flowmvi.plugins.init import pro.respawn.flowmvi.plugins.recover import pro.respawn.flowmvi.test.subscribeAndTest @@ -25,7 +22,6 @@ import pro.respawn.flowmvi.util.idle import pro.respawn.flowmvi.util.testStore import pro.respawn.flowmvi.util.testTimeTravelPlugin -@OptIn(DelicateStoreApi::class) class StoreExceptionsText : FreeSpec({ asUnconfined() @@ -48,23 +44,21 @@ class StoreExceptionsText : FreeSpec({ } } "then store is not closed when thrown" { - val job = store.start(this) - idle() - job.isActive shouldBe true - job.cancelAndJoin() - idle() - plugin.starts shouldBe 1 - plugin.exceptions.shouldContainExactly(e) + store.test { job -> + job.isActive shouldBe true + idle() + plugin.starts shouldBe 1 + plugin.exceptions.shouldContainExactly(e) + } } "then exceptions in processing scope do not cancel the pipeline" { - val job = store.start(this) - store.send { } - idle() - job.isActive shouldBe true - job.cancelAndJoin() - idle() - plugin.intents.shouldBeSingleton() + store.test { job -> + send { } + idle() + job.isActive shouldBe true + plugin.intents.shouldBeSingleton() + } } } "given store that throws on subscribe" - { @@ -80,20 +74,18 @@ class StoreExceptionsText : FreeSpec({ } } "then exceptions in subscriber scope do not cancel the pipeline" { - store.test { - subscribe { - println("Subscribed") - }.cancelAndJoin() + store.subscribeAndTest { + println("Subscribed") } - println("should have stopped") idle() + println("should have stopped") } } "then nested coroutines propagate exceptions to handler" { val store = testStore(plugin) { recover { null } } - store.test { + store.test { job -> send { launch a@{ println("job 1 started") @@ -109,22 +101,22 @@ class StoreExceptionsText : FreeSpec({ } } idle() - plugin.exceptions shouldContain e - plugin.intents.shouldBeSingleton() + job.isActive shouldBe true } + idle() + plugin.intents.shouldBeSingleton() + plugin.exceptions shouldContain e } - "and store that does not handle exceptions" - { - val store = testStore(plugin) { - init { - launch { throw e } - } - } - - "then exceptions are rethrown".config(enabled = false) { + // this test crashes the whole test suite because we can't catch exceptions in coroutine exception handlers + "and store that does not handle exceptions".config(enabled = false) - { + "then exceptions are rethrown" { shouldThrowExactly { - coroutineScope { - // TODO: cancels the scope, figure out how to not cancel the parent scope - store.start(this).join() + val store = testStore(plugin) { + recover { it } + } + store.test { + intent { throw e } + idle() } } } @@ -132,7 +124,7 @@ class StoreExceptionsText : FreeSpec({ "and store that handles exceptions" - { val store = testStore(plugin) { recover { - currentCoroutineContext()[Recoverable].shouldNotBeNull() + currentCoroutineContext()[RecoverModule].shouldNotBeNull() null } } @@ -142,5 +134,25 @@ class StoreExceptionsText : FreeSpec({ } } } + // this test passes (verified manually), but the execution leads to the whole test suite crashing as the + // assertion does not catch exceptions correctly + "and store that throws in recover()".config(enabled = false) - { + val store = testStore(plugin) { + recover { + throw IllegalStateException(it) + } + } + "then exceptions in that store are rethrown" { + shouldThrowExactly { + store.test { + intent { + throw e + } + idle() + } + idle() + } + } + } } }) diff --git a/core/src/jvmTest/kotlin/pro/respawn/flowmvi/util/TestStore.kt b/core/src/jvmTest/kotlin/pro/respawn/flowmvi/util/TestStore.kt index 341d903d..5a8e0a02 100644 --- a/core/src/jvmTest/kotlin/pro/respawn/flowmvi/util/TestStore.kt +++ b/core/src/jvmTest/kotlin/pro/respawn/flowmvi/util/TestStore.kt @@ -8,7 +8,7 @@ import pro.respawn.flowmvi.dsl.LambdaIntent import pro.respawn.flowmvi.dsl.reduceLambdas import pro.respawn.flowmvi.dsl.store import pro.respawn.flowmvi.plugins.TimeTravelPlugin -import pro.respawn.flowmvi.plugins.consoleLoggingPlugin +import pro.respawn.flowmvi.plugins.platformLoggingPlugin import pro.respawn.flowmvi.plugins.timeTravelPlugin internal fun testTimeTravelPlugin() = timeTravelPlugin, TestAction>() @@ -23,7 +23,7 @@ internal fun testStore( name = "TestStore" actionShareBehavior = behavior install(timeTravel) - install(consoleLoggingPlugin(name)) + install(platformLoggingPlugin(name)) configure() reduceLambdas() } diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index cd114190..4dae128b 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -11,7 +11,7 @@ detekt = "1.23.4" detektFormattingPlugin = "1.23.4" dokka = "1.9.10" fragment = "1.6.2" -gradleAndroid = "8.3.0-alpha17" +gradleAndroid = "8.3.0-alpha18" gradleDoctorPlugin = "0.9.1" junit = "4.13.2" koin = "3.5.0" From 40369cae28056f4057711bac71723875cef8763f Mon Sep 17 00:00:00 2001 From: Nek-12 Date: Thu, 21 Dec 2023 22:57:06 +0300 Subject: [PATCH 3/4] bump version --- buildSrc/src/main/kotlin/Config.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/buildSrc/src/main/kotlin/Config.kt b/buildSrc/src/main/kotlin/Config.kt index 7f479d3e..cf0d7087 100644 --- a/buildSrc/src/main/kotlin/Config.kt +++ b/buildSrc/src/main/kotlin/Config.kt @@ -17,7 +17,7 @@ object Config { const val majorRelease = 2 const val minorRelease = 2 - const val patch = 1 + const val patch = 2 const val postfix = "rc" const val versionName = "$majorRelease.$minorRelease.$patch-$postfix" const val url = "https://github.com/respawn-app/FlowMVI" From 244c29707360d00c43a072a0c80b16ff43d1805a Mon Sep 17 00:00:00 2001 From: Nek-12 Date: Thu, 21 Dec 2023 23:38:32 +0300 Subject: [PATCH 4/4] fix unnecessary check in recover() --- .../kotlin/pro/respawn/flowmvi/modules/RecoverModule.kt | 2 +- .../commonMain/kotlin/pro/respawn/flowmvi/store/StoreImpl.kt | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/RecoverModule.kt b/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/RecoverModule.kt index 3c3c98a2..faf4d36a 100644 --- a/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/RecoverModule.kt +++ b/core/src/commonMain/kotlin/pro/respawn/flowmvi/modules/RecoverModule.kt @@ -35,8 +35,8 @@ internal fun interface RecoverModule withContext(this@RecoverModule) { block() } } catch (expected: Exception) { when { - alreadyRecovered() -> throw RecursiveRecoverException(expected) expected is CancellationException || expected is UnrecoverableException -> throw expected + alreadyRecovered() -> throw RecursiveRecoverException(expected) else -> recover(expected) } } diff --git a/core/src/commonMain/kotlin/pro/respawn/flowmvi/store/StoreImpl.kt b/core/src/commonMain/kotlin/pro/respawn/flowmvi/store/StoreImpl.kt index 86e450c1..a448a01d 100644 --- a/core/src/commonMain/kotlin/pro/respawn/flowmvi/store/StoreImpl.kt +++ b/core/src/commonMain/kotlin/pro/respawn/flowmvi/store/StoreImpl.kt @@ -24,7 +24,6 @@ import pro.respawn.flowmvi.modules.RecoverModule import pro.respawn.flowmvi.modules.StateModule import pro.respawn.flowmvi.modules.SubscribersModule import pro.respawn.flowmvi.modules.actionModule -import pro.respawn.flowmvi.modules.alreadyRecovered import pro.respawn.flowmvi.modules.intentModule import pro.respawn.flowmvi.modules.launchPipeline import pro.respawn.flowmvi.modules.observeSubscribers @@ -83,7 +82,6 @@ internal class StoreImpl( } override suspend fun PipelineContext.recover(e: Exception) { - if (alreadyRecovered()) throw e withContext(this@StoreImpl) { onException(e)?.let { throw UnhandledStoreException(it) } }