fix(upgrade): Reconnect billing instantly on user actions

2026-09-15 02:36:12 -04:00 · 2026-07-14 16:43:58 +02:00
parent c8179c5c19
commit f4facd3e86
2 changed files with 83 additions and 24 deletions
@@ -35,21 +35,30 @@ class BillingDataRepo @Inject constructor(
    // Null until the first attempt, so devices with less than an hour of uptime still refresh.
    private var lastForegroundRefreshAt: Long? = null

+    // Explicit billing operations kick a waiting connection-retry backoff so a user action (restore
+    // tap, buy tap, opening the upgrade screen) reconnects immediately after Play was fixed instead
+    // of waiting out the timer. Zero replay: kicks only matter while a retry is actively waiting —
+    // a healthy connection must not accumulate stale wake-ups. (Ported from sdmaid-se#2562.)
+    private val connectionKicks = MutableSharedFlow<Unit>(extraBufferCapacity = 1)
+
    private val connectionProvider = billingClientConnectionProvider.connection
        .retryWhen { cause, attempt ->
            if (cause is CancellationException) return@retryWhen false
            log(TAG, ERROR) { "Unable to provide client connection (attempt=$attempt):\n${cause.asLog()}" }
            // Capped backoff: don't hammer a persistently broken Play from the always-hot process
-            // (upstream already did 5 quick retries). A foreground *entry* short-circuits the
-            // wait — e.g. the user just returned from signing into the missing Google account
-            // and shouldn't have to wait out the full backoff.
+            // (upstream already did 5 quick retries). An explicit billing operation or a foreground
+            // *entry* short-circuits the wait — e.g. the user just returned from signing into the
+            // missing Google account and shouldn't have to wait out the full backoff.
            val backoffMs = (RETRY_BACKOFF_BASE_MS * (attempt + 1)).coerceAtMost(RETRY_BACKOFF_MAX_MS)
-            // StateFlow dedupes, so after dropping the current value the next `true` is a real
-            // foreground *entry*, not the pre-existing foreground state.
            val kicked = withTimeoutOrNull(backoffMs) {
-                appForegroundState.isForeground.drop(1).first { it }
+                merge(
+                    connectionKicks,
+                    // StateFlow dedupes, so after dropping the current value the next `true` is a
+                    // real foreground *entry*, not the pre-existing foreground state.
+                    appForegroundState.isForeground.drop(1).filter { it }.map { },
+                ).first()
            }
-            if (kicked != null) log(TAG) { "App came to foreground, retrying billing connection early" }
+            if (kicked != null) log(TAG) { "User action or foreground entry, retrying billing connection early" }
            true
        }
        .replayingShare(scope)
@@ -160,6 +169,7 @@ class BillingDataRepo @Inject constructor(
    }

    suspend fun refresh(): BillingData = try {
+        connectionKicks.tryEmit(Unit)
        val clientConnection = connectionProvider.first()
        val purchases = clientConnection.refreshPurchases()

@@ -169,6 +179,7 @@ class BillingDataRepo @Inject constructor(
    }

    suspend fun querySkus(vararg skus: Sku): Collection<SkuDetails> = try {
+        connectionKicks.tryEmit(Unit)
        val clientConnection = connectionProvider.first()
        clientConnection.querySkus(*skus)
    } catch (e: Exception) {
@@ -181,6 +192,7 @@ class BillingDataRepo @Inject constructor(
        offer: Sku.Subscription.Offer? = null,
    ) {
        try {
+            connectionKicks.tryEmit(Unit)
            val clientConnection = connectionProvider.first()
            clientConnection.launchBillingFlow(activity, sku, offer)
        } catch (e: CancellationException) {
@@ -18,6 +18,7 @@ import io.mockk.coVerify
 import io.mockk.every
 import io.mockk.mockk
 import kotlinx.coroutines.cancel
+import kotlinx.coroutines.launch
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.emptyFlow
 import kotlinx.coroutines.flow.flow
@@ -107,26 +108,72 @@ class BillingDataRepoTest : BaseTest() {
            every { isForeground } returns foreground
        }

-        BillingDataRepo(provider, testScope, foregroundState, TestTimeSource())
-        testScope.testScheduler.runCurrent()
-        attempts shouldBe 1
+        try {
+            BillingDataRepo(provider, testScope, foregroundState, TestTimeSource())
+            testScope.testScheduler.runCurrent()
+            attempts shouldBe 1

-        // First backoff is 60s — not a second sooner.
-        testScope.testScheduler.advanceTimeBy(59_000)
-        testScope.testScheduler.runCurrent()
-        attempts shouldBe 1
-        testScope.testScheduler.advanceTimeBy(2_000)
-        testScope.testScheduler.runCurrent()
-        attempts shouldBe 2
+            // First backoff is 60s — not a second sooner.
+            testScope.testScheduler.advanceTimeBy(59_000)
+            testScope.testScheduler.runCurrent()
+            attempts shouldBe 1
+            testScope.testScheduler.advanceTimeBy(2_000)
+            testScope.testScheduler.runCurrent()
+            attempts shouldBe 2

-        // Second backoff would be 120s — a foreground entry short-circuits it, so a user
-        // returning from e.g. Google sign-in doesn't wait out the full backoff.
-        testScope.testScheduler.advanceTimeBy(5_000)
-        foreground.value = true
-        testScope.testScheduler.runCurrent()
-        attempts shouldBe 3
+            // Second backoff would be 120s — a foreground entry short-circuits it, so a user
+            // returning from e.g. Google sign-in doesn't wait out the full backoff. A foreground
+            // entry drives two independent early-retry paths (the retry loop's own foreground
+            // branch, plus the init foreground-refresh calling refresh() which emits a kick), so
+            // the exact count is interleaving-dependent — either can produce attempt 3 or 4.
+            testScope.testScheduler.advanceTimeBy(5_000)
+            foreground.value = true
+            testScope.testScheduler.runCurrent()
+            (attempts in 3..4) shouldBe true

-        testScope.cancel()
+            // ...and then it settles: with no further action/lifecycle signal and time still well
+            // under the next backoff, the double-kick must not compound into a busy-loop.
+            val settled = attempts
+            testScope.testScheduler.advanceTimeBy(5_000)
+            testScope.testScheduler.runCurrent()
+            attempts shouldBe settled
+        } finally {
+            // The repo pipelines and retry loop are infinite — a leaked scope after a failed
+            // assertion would keep them alive for the rest of the JVM.
+            testScope.cancel()
+        }
+    }
+
+    @Test
+    fun `explicit billing operations kick a waiting connection retry`() = runTest2 {
+        val testScope = TestScope(UnconfinedTestDispatcher(testScheduler))
+        var attempts = 0
+        val provider = mockk<BillingClientConnectionProvider> {
+            every { connection } returns flow {
+                attempts++
+                throw BillingException("still broken")
+            }
+        }
+        val foregroundState = mockk<AppForegroundState> {
+            every { isForeground } returns MutableStateFlow(false)
+        }
+
+        try {
+            val repo = BillingDataRepo(provider, testScope, foregroundState, TestTimeSource())
+            testScope.testScheduler.runCurrent()
+            attempts shouldBe 1
+
+            // A restore-style refresh() while the retry is waiting out its 60s backoff kicks it
+            // immediately — the user shouldn't wait out the timer after fixing Play themselves.
+            testScope.testScheduler.advanceTimeBy(5_000)
+            val refreshJob = testScope.launch { runCatching { repo.refresh() } }
+            testScope.testScheduler.runCurrent()
+            attempts shouldBe 2
+
+            refreshJob.cancel()
+        } finally {
+            testScope.cancel()
+        }
    }

    @Test