From 7e3f9cd2ff927de161c4fd90b3efdf82734be42a Mon Sep 17 00:00:00 2001
From: kirich1409 <kirill@androidbroadcast.dev>
Date: Mon, 18 May 2026 11:39:25 +0300
Subject: [PATCH] Fix ConfigValues.observe to catch provider exceptions

ConfigValues KDoc promises that observe wraps provider calls in try/catch
and routes errors through onProviderError, but the implementation collected
provider Flows raw, so a provider whose Flow throws downstream would crash
the host coroutine. Wrap local and remote Flows in .catch { onProviderError }
before merging to restore the documented contract. Add a regression test
asserting that a throwing local provider does not propagate and that the
error is reported.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---
 .../androidbroadcast/featured/ConfigValues.kt |  5 +-
 .../featured/ProviderErrorObserveTest.kt      | 90 +++++++++++++++++++
 2 files changed, 93 insertions(+), 2 deletions(-)
 create mode 100644 core/src/commonTest/kotlin/dev/androidbroadcast/featured/ProviderErrorObserveTest.kt

diff --git a/core/src/commonMain/kotlin/dev/androidbroadcast/featured/ConfigValues.kt b/core/src/commonMain/kotlin/dev/androidbroadcast/featured/ConfigValues.kt
index ab8df1e..dd755c8 100644
--- a/core/src/commonMain/kotlin/dev/androidbroadcast/featured/ConfigValues.kt
+++ b/core/src/commonMain/kotlin/dev/androidbroadcast/featured/ConfigValues.kt
@@ -4,6 +4,7 @@ package dev.androidbroadcast.featured
 
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableSharedFlow
+import kotlinx.coroutines.flow.catch
 import kotlinx.coroutines.flow.distinctUntilChanged
 import kotlinx.coroutines.flow.flow
 import kotlinx.coroutines.flow.map
@@ -169,8 +170,8 @@ public class ConfigValues(
      * @return A [Flow] of [ConfigValue] for the specified parameter.
      */
     public fun <T : Any> observe(param: ConfigParam<T>): Flow<ConfigValue<T>> {
-        val localFlow = localProvider?.observe(param)
-        val remoteFlow = fetchSignal.map { getValue(param) }
+        val localFlow = localProvider?.observe(param)?.catch { e -> onProviderError(e) }
+        val remoteFlow = fetchSignal.map { getValue(param) }.catch { e -> onProviderError(e) }
 
         return flow<ConfigValue<T>> {
             emit(getValue(param))
diff --git a/core/src/commonTest/kotlin/dev/androidbroadcast/featured/ProviderErrorObserveTest.kt b/core/src/commonTest/kotlin/dev/androidbroadcast/featured/ProviderErrorObserveTest.kt
new file mode 100644
index 0000000..f1d61c8
--- /dev/null
+++ b/core/src/commonTest/kotlin/dev/androidbroadcast/featured/ProviderErrorObserveTest.kt
@@ -0,0 +1,90 @@
+package dev.androidbroadcast.featured
+
+import app.cash.turbine.test
+import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.flow
+import kotlinx.coroutines.test.runTest
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertTrue
+
+class ProviderErrorObserveTest {
+    private val testParam = ConfigParam("observe_error_key", "DEFAULT")
+
+    /**
+     * Fake local provider whose [observe] emits one value successfully,
+     * then throws. All other required methods are no-ops.
+     */
+    private class OnceThrowingLocalProvider(
+        private val emittedValue: String,
+        private val error: Throwable,
+    ) : LocalConfigValueProvider {
+        override suspend fun <T : Any> get(param: ConfigParam<T>): ConfigValue<T>? = null
+
+        override suspend fun <T : Any> set(
+            param: ConfigParam<T>,
+            value: T,
+        ) = Unit
+
+        override suspend fun <T : Any> resetOverride(param: ConfigParam<T>) = Unit
+
+        override suspend fun clear() = Unit
+
+        @Suppress("UNCHECKED_CAST")
+        override fun <T : Any> observe(param: ConfigParam<T>): Flow<ConfigValue<T>> =
+            flow {
+                emit(ConfigValue(emittedValue as T, ConfigValue.Source.LOCAL))
+                throw error
+            }
+    }
+
+    @Test
+    fun observeDoesNotPropagateWhenLocalProviderFlowThrows() =
+        runTest {
+            val errors = mutableListOf<Throwable>()
+            val provider =
+                OnceThrowingLocalProvider(
+                    emittedValue = "local_value",
+                    error = IllegalStateException("simulated provider error"),
+                )
+            val configValues =
+                ConfigValues(
+                    localProvider = provider,
+                    onProviderError = { errors.add(it) },
+                )
+
+            val collected = mutableListOf<String>()
+
+            // Collection must complete without throwing; the Flow stays alive after the
+            // local provider's Flow terminates with an exception.
+            configValues.observe(testParam).test {
+                // Initial emission from getValue(param) — local provider's get() returns null
+                // so this resolves to the default value.
+                val initial = awaitItem()
+                assertEquals("DEFAULT", initial.value)
+                assertEquals(ConfigValue.Source.DEFAULT, initial.source)
+                collected.add(initial.value)
+
+                // Emission from the local provider's observe() before it throws.
+                // distinctUntilChanged passes it because "local_value" ≠ "DEFAULT".
+                val fromLocal = awaitItem()
+                assertEquals("local_value", fromLocal.value)
+                assertEquals(ConfigValue.Source.LOCAL, fromLocal.source)
+                collected.add(fromLocal.value)
+
+                cancelAndIgnoreRemainingEvents()
+            }
+
+            // The thrown exception must have been routed to onProviderError, not re-thrown.
+            assertEquals(1, errors.size)
+            assertTrue(
+                errors[0] is IllegalStateException,
+                "Expected IllegalStateException but was ${errors[0]::class}",
+            )
+            assertEquals("simulated provider error", errors[0].message)
+
+            // Both values were collected — flow did not crash before the second emission.
+            assertTrue("DEFAULT" in collected)
+            assertTrue("local_value" in collected)
+        }
+}