fix: firebase FetchException wrapping and missing ADR doc

[kirich1409]

Summary

• Firebase tests: FirebaseConfigValueProvider.fetch() had a catch (e: RuntimeException) { throw e } block that re-threw RuntimeException bare, bypassing the FetchException wrapper. Tests inject a RuntimeException task failure and assert FetchException — this fixes 2 failing tests in FirebaseConfigValueProviderTest.
• Build Docs: adr/001-configvalues-multi-module.md was referenced in the MkDocs nav and linked from guides/best-practices.md but the file was never created. mkdocs build --strict aborted on 2 warnings. Removed the nav entry and dead link.

Test plan

• Tests & Coverage job passes (FirebaseConfigValueProviderTest no longer fails)
• Build Docs job passes (mkdocs build --strict no longer aborts)

🤖 Generated with Claude Code

[qodo-code-review]

Review Summary by Qodo

Fix Firebase exception wrapping and remove missing ADR references

🐞 Bug fix

Walkthroughs

Description

• Fix Firebase exception handling to wrap RuntimeException in FetchException
• Remove references to non-existent ADR 001 documentation file
• Resolve failing tests and documentation build failures

Diagram

flowchart LR
  A["RuntimeException re-throw"] -->|Remove bare catch| B["FetchException wrapper"]
  C["ADR 001 nav entry"] -->|Delete| D["Valid mkdocs config"]
  E["Dead link in best-practices"] -->|Remove| F["Clean documentation"]

Loading

File Changes

1. providers/firebase/src/main/kotlin/dev/androidbroadcast/featured/firebase/FirebaseConfigValueProvider.kt 🐞 Bug fix +0/-2

Wrap RuntimeException in FetchException properly

• Removed bare RuntimeException catch block that bypassed FetchException wrapping
• Now all exceptions are properly wrapped in FetchException as per contract
• Fixes failing FirebaseConfigValueProviderTest tests

providers/firebase/src/main/kotlin/dev/androidbroadcast/featured/firebase/FirebaseConfigValueProvider.kt

---

2. docs/guides/best-practices.md 📝 Documentation +0/-1

Remove dead ADR 001 documentation link

• Removed dead link reference to non-existent adr/001-configvalues-multi-module.md
• Eliminates documentation build warning

docs/guides/best-practices.md

---

3. mkdocs.yml ⚙️ Configuration changes +0/-2

Remove missing ADR 001 from navigation

• Removed ADRs section from navigation containing reference to missing ADR 001 file
• Resolves mkdocs build --strict build failure

mkdocs.yml

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX Issues (0)

1. Cancellation wrapped into FetchException 🐞 Bug ☼ Reliability

Description

FirebaseConfigValueProvider.fetch() now wraps all Exceptions thrown by task.await() into
FetchException, which can also wrap coroutine cancellation exceptions and break cooperative
cancellation/timeout behavior. Previously, RuntimeException (including cancellation) would bypass
FetchException wrapping due to the removed catch block.

Code

providers/firebase/src/main/kotlin/dev/androidbroadcast/featured/firebase/FirebaseConfigValueProvider.kt[R99-103]

        try {
            task.await()
-        } catch (e: RuntimeException) {
-            throw e
        } catch (e: Exception) {
            throw FetchException("Firebase Remote Config fetch failed", e)
        }

Evidence

fetch() is a suspend function and awaits a Task, but it catches Exception broadly and always throws
FetchException, so any Exception subtype used for cancellation/timeouts will be converted into a
regular failure. ConfigValues.fetch() calls remoteProvider.fetch(true) and does not catch/translate
exceptions, so this altered exception type propagates to callers and can change control-flow for
cancellation/timeout handling.

providers/firebase/src/main/kotlin/dev/androidbroadcast/featured/firebase/FirebaseConfigValueProvider.kt[92-104]
core/src/commonMain/kotlin/dev/androidbroadcast/featured/ConfigValues.kt[155-159]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`FirebaseConfigValueProvider.fetch()` wraps all `Exception`s from `task.await()` into `FetchException`. This can inadvertently wrap coroutine cancellation/timeouts (e.g., `CancellationException`) and prevent proper cooperative cancellation semantics.

### Issue Context
This behavior changed because the previous `catch (e: RuntimeException) { throw e }` bypass was removed, meaning runtime-based cancellation exceptions can now be wrapped.

### Fix Focus Areas
- providers/firebase/src/main/kotlin/dev/androidbroadcast/featured/firebase/FirebaseConfigValueProvider.kt[92-104]

### Suggested fix
Add a dedicated catch before the generic `Exception` catch:
```kotlin
import kotlinx.coroutines.CancellationException

try {
 task.await()
} catch (e: CancellationException) {
 throw e
} catch (e: Exception) {
 throw FetchException("Firebase Remote Config fetch failed", e)
}
```
This preserves cancellation while still wrapping genuine fetch failures (including injected RuntimeExceptions used by tests).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[Copilot]

Pull request overview

Fixes Firebase Remote Config fetch error propagation to consistently throw FetchException, and resolves MkDocs strict build failures caused by references to a missing ADR document.

Changes:

• Remove RuntimeException passthrough in FirebaseConfigValueProvider.fetch() so task failures are wrapped in FetchException.
• Remove missing ADR entry from mkdocs.yml nav.
• Remove dead ADR link from docs/guides/best-practices.md.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
providers/firebase/src/main/kotlin/dev/androidbroadcast/featured/firebase/FirebaseConfigValueProvider.kt	Adjusts exception handling so fetch failures are wrapped as FetchException.
mkdocs.yml	Removes nav entry pointing to a non-existent ADR doc to satisfy strict docs build.
docs/guides/best-practices.md	Removes a dead link to the missing ADR doc.

[Copilot]

fetch() wraps all Exceptions from task.await() into FetchException. Since await() is cancellable, coroutine cancellation will throw CancellationException and should be rethrown (not wrapped), otherwise cancellation semantics are broken and callers may fail to cancel work properly. Add a catch (e: CancellationException) { throw e } before the generic catch (and import kotlinx.coroutines.CancellationException).