From 1e1f0b13c6a273dd53f09a42d6a775a604f52e2f Mon Sep 17 00:00:00 2001 From: darken Date: Wed, 5 Aug 2026 15:28:33 +0200 Subject: [PATCH] fix(overview): Keep the review prompt from crashing or stacking Corrupt review settings terminated the shared state flow on AppScope, so the exception crashed the process instead of reaching the ViewModel's catch. Absorb it upstream of both replayingShare calls. The review card also no longer stacks on top of the enable-Bluetooth prompt. Fixes review findings F1, F2 --- .../capod/common/review/GplayReviewTool.kt | 13 +++++++++ .../main/ui/overview/OverviewViewModel.kt | 7 +++-- .../main/ui/overview/OverviewViewModelTest.kt | 10 +++++++ .../common/review/GplayReviewToolTest.kt | 28 +++++++++++++++++++ 4 files changed, 55 insertions(+), 3 deletions(-) diff --git a/app/src/gplay/java/eu/darken/capod/common/review/GplayReviewTool.kt b/app/src/gplay/java/eu/darken/capod/common/review/GplayReviewTool.kt index dac10bd1..a0ee0e5e 100644 --- a/app/src/gplay/java/eu/darken/capod/common/review/GplayReviewTool.kt +++ b/app/src/gplay/java/eu/darken/capod/common/review/GplayReviewTool.kt @@ -20,6 +20,7 @@ import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.catch import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.flatMapLatest @@ -65,6 +66,13 @@ class GplayReviewTool @Inject constructor( hasPaidForPro && !isSnoozed && !hasReviewed } .distinctUntilChanged() + // Upstream of the shares below: an exception there would kill the sharing coroutine on + // AppScope (crashing the process) instead of reaching any downstream `catch`. + .catch { e -> + if (e is CancellationException) throw e + log(TAG, ERROR) { "Eligibility failed: ${e.asLog()}" } + emit(false) + } // Only probed once the user is eligible: Play counts requests against the app's quota, and an // `isNoOp` answer is Play's deliberate verdict, i.e. an answer and not a failure to retry. @@ -107,6 +115,11 @@ class GplayReviewTool @Inject constructor( } .throttleLatest(500) .onStart { emit(ReviewTool.State()) } + .catch { e -> + if (e is CancellationException) throw e + log(TAG, ERROR) { "State failed: ${e.asLog()}" } + emit(ReviewTool.State()) + } .replayingShare(appScope) // Single-flight: a second tap must not queue up behind the first, or Play's flow would be diff --git a/app/src/main/java/eu/darken/capod/main/ui/overview/OverviewViewModel.kt b/app/src/main/java/eu/darken/capod/main/ui/overview/OverviewViewModel.kt index f01fd9aa..ce92d1c7 100644 --- a/app/src/main/java/eu/darken/capod/main/ui/overview/OverviewViewModel.kt +++ b/app/src/main/java/eu/darken/capod/main/ui/overview/OverviewViewModel.kt @@ -332,14 +332,15 @@ class OverviewViewModel @Inject constructor( /** * Whether the overview is currently showing a card that outranks the review prompt: a - * missing permission, the troubleshooter hint, the background-monitoring-off notice or the - * no-profiles setup card. All of those ask the user to do something, so the review prompt - * stays hidden while any of them is on screen. + * missing permission, the troubleshooter hint, the background-monitoring-off notice, the + * no-profiles setup card or the enable-Bluetooth prompt. All of those ask the user to do + * something, so the review prompt stays hidden while any of them is on screen. */ val hasHigherPriorityCard: Boolean get() = permissions.isNotEmpty() || showTroubleshootSuggestion || monitoringStatus == MonitoringStatus.BACKGROUND_OFF || + !isBluetoothEnabled || (profiles.isEmpty() && !isScanBlocked && isBluetoothEnabled) val soleProfileId: ProfileId? get() = profiles.singleOrNull()?.id diff --git a/app/src/test/java/eu/darken/capod/main/ui/overview/OverviewViewModelTest.kt b/app/src/test/java/eu/darken/capod/main/ui/overview/OverviewViewModelTest.kt index b4bd9c77..cbcdc56d 100644 --- a/app/src/test/java/eu/darken/capod/main/ui/overview/OverviewViewModelTest.kt +++ b/app/src/test/java/eu/darken/capod/main/ui/overview/OverviewViewModelTest.kt @@ -927,6 +927,16 @@ class OverviewViewModelTest : BaseTest() { vm.state.first().showReviewCard shouldBe false } + @Test + fun `suppressed by the enable-Bluetooth prompt`() = runTest(testDispatcher) { + quietOverview() + isBluetoothEnabledFlow.value = false + + val vm = createViewModel() + + vm.state.first().showReviewCard shouldBe false + } + @Test fun `suppressed by the troubleshooter suggestion`() = runTest(testDispatcher) { quietOverview() diff --git a/app/src/testGplay/java/eu/darken/capod/common/review/GplayReviewToolTest.kt b/app/src/testGplay/java/eu/darken/capod/common/review/GplayReviewToolTest.kt index 02ba6dc3..82d589a6 100644 --- a/app/src/testGplay/java/eu/darken/capod/common/review/GplayReviewToolTest.kt +++ b/app/src/testGplay/java/eu/darken/capod/common/review/GplayReviewToolTest.kt @@ -19,11 +19,13 @@ import io.mockk.verify import kotlinx.coroutines.CancellationException import kotlinx.coroutines.flow.drop import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.launch import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.withTimeoutOrNull +import kotlinx.serialization.SerializationException import org.junit.jupiter.api.Test import testhelpers.BaseTest import testhelpers.coroutine.runTest2 @@ -234,6 +236,32 @@ class GplayReviewToolTest : BaseTest() { verify(exactly = 3) { manager.requestReviewFlow() } } + @Test fun `corrupt review settings fall back to the default state`() = runTest2 { + every { manager.requestReviewFlow() } returns Tasks.forResult(reviewInfo()) + lastDismissedMock = rwSetting(null) + // A malformed stored timestamp throws when the DataStore flow is read. + reviewedAtMock = mockk>(relaxed = true).apply { + every { flow } returns flow { throw SerializationException("corrupt") } + } + every { settings.lastDismissed } returns lastDismissedMock + every { settings.reviewedAt } returns reviewedAtMock + + val upgradeInfo = mockk() + every { upgradeInfo.upgradedAt } returns Instant.now().minus(Duration.ofDays(30)) + every { upgradeRepo.upgradeInfo } returns flowOf(upgradeInfo) + + val tool = GplayReviewTool( + appScope = backgroundScope, + settings = settings, + manager = manager, + upgradeRepo = upgradeRepo, + ) + + // The failure has to be absorbed upstream of the share: on the shared coroutine it would go + // to AppScope (no handler, i.e. a process crash) and never reach a downstream collector. + tool.computedState() shouldBe ReviewTool.State() + } + @Test fun `cancellation during reviewNow is not swallowed`() = runTest2 { every { manager.requestReviewFlow() } throws CancellationException("scope died") val tool = tool()