Skip to content

Fix ConfigValues.observe to catch provider exceptions#196

Merged
kirich1409 merged 1 commit into
developfrom
fix/configvalues-observe-catch
May 18, 2026
Merged

Fix ConfigValues.observe to catch provider exceptions#196
kirich1409 merged 1 commit into
developfrom
fix/configvalues-observe-catch

Conversation

@kirich1409
Copy link
Copy Markdown
Contributor

Summary

  • ConfigValues.observe collected provider Flows raw via merge with no .catch, so any exception thrown downstream by localProvider.observe(param) propagated straight into the consumer's coroutine and crashed the host — violating the documented contract ("flow does not terminate on provider error").
  • Fix: wrap both localFlow and remoteFlow in .catch { e -> onProviderError(e) } before passing them to merge. The Flow operator .catch ignores CancellationException by design, so no manual rethrow is needed.
  • getValue was already guarded via runCatching; this patch makes observe consistent with it.

Regression test

ProviderErrorObserveTest.observeDoesNotPropagateWhenLocalProviderFlowThrows
(core/src/commonTest/kotlin/dev/androidbroadcast/featured/ProviderErrorObserveTest.kt)

Constructs a fake LocalConfigValueProvider whose observe() emits one value then throws IllegalStateException("simulated provider error"), collects via Turbine, and asserts:

  • collection does not throw
  • onProviderError receives exactly one IllegalStateException with the expected message
  • both the default emission and the local-provider emission were collected

Follow-up PRs enabled

This fix unblocks PRs that register providers with types they don't support (e.g. DataStore enum converters) — the flow will now degrade gracefully rather than crashing.

Test plan

  • ./gradlew :core:allTests — green (JVM + iOS simulator)
  • ./gradlew spotlessCheck — green
  • ./gradlew :core:koverVerify — ≥90% coverage gate passes

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>
@kirich1409 kirich1409 marked this pull request as ready for review May 18, 2026 10:30
Copilot AI review requested due to automatic review settings May 18, 2026 10:30
@qodo-code-review
Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@kirich1409 kirich1409 merged commit 3f43b24 into develop May 18, 2026
10 checks passed
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a bug in ConfigValues.observe where exceptions thrown by localProvider.observe(param) propagated to consumers and crashed the host coroutine, contradicting the documented contract that the flow does not terminate on provider error. The fix wraps both upstream flows in .catch so provider errors are routed to the onProviderError callback, matching the existing behavior of getValue.

Changes:

  • Apply .catch { e -> onProviderError(e) } to both localFlow and remoteFlow in observe before merging.
  • Add a regression test ProviderErrorObserveTest using Turbine to verify that a throwing local provider does not propagate the error and that emissions before the error are still collected.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
core/src/commonMain/kotlin/dev/androidbroadcast/featured/ConfigValues.kt Wraps the local and remote upstream flows in .catch to forward provider exceptions to onProviderError instead of crashing the consumer.
core/src/commonTest/kotlin/dev/androidbroadcast/featured/ProviderErrorObserveTest.kt New test verifying observe survives a throwing local provider flow and routes the error to onProviderError.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants