From 72fb1d3b17de19588d96657ff3085bea6f9cc641 Mon Sep 17 00:00:00 2001 From: darken Date: Sun, 2 Aug 2026 11:51:42 +0200 Subject: [PATCH] fix(upgrade): Never let a failed cache stamp abort entitlement bookkeeping stampLastProState() was bounded against a wedged file lock, but a write that failed outright (corrupt preferences file, no disk space) still threw straight through into the entitlement path it only decorates. Non-cancellation exceptions from the edit now log a warning and skip the stamp, same as the timeout does. Cancellation keeps propagating - caught first on purpose, swallowing it would break the caller's structured concurrency. Reads stay loud: a snapshot that couldn't be read must not be mistaken for a never-bought install. --- .../capod/common/upgrade/core/BillingCache.kt | 28 +++++++++---- .../common/upgrade/core/BillingCacheTest.kt | 39 +++++++++++++++++++ 2 files changed, 59 insertions(+), 8 deletions(-) diff --git a/app/src/gplay/java/eu/darken/capod/common/upgrade/core/BillingCache.kt b/app/src/gplay/java/eu/darken/capod/common/upgrade/core/BillingCache.kt index f35d5cec..211f75b8 100644 --- a/app/src/gplay/java/eu/darken/capod/common/upgrade/core/BillingCache.kt +++ b/app/src/gplay/java/eu/darken/capod/common/upgrade/core/BillingCache.kt @@ -13,8 +13,10 @@ import eu.darken.capod.common.datastore.basicReader import eu.darken.capod.common.datastore.basicWriter import eu.darken.capod.common.datastore.createValue import eu.darken.capod.common.debug.logging.Logging.Priority.WARN +import eu.darken.capod.common.debug.logging.asLog import eu.darken.capod.common.debug.logging.log import eu.darken.capod.common.debug.logging.logTag +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.flow.first import kotlinx.coroutines.withTimeoutOrNull import java.io.IOException @@ -99,14 +101,24 @@ class BillingCache internal constructor( // not erase. suspend fun stampLastProState(skuId: String, at: Long) { // Fail-soft: this decorates the entitlement path, it must never be the thing that blocks it. - withTimeoutOrNull(cacheTimeoutMs) { - dataStore.edit { prefs -> - prefs[lastProStateSkuKey] = skuId - prefs[lastProStateAtKey] = at - val episodeStart = prefs[proUnconfirmedSinceKey] ?: 0L - if (episodeStart in 1..at) prefs[proUnconfirmedSinceKey] = 0L - } - } ?: log(TAG, WARN) { "stampLastProState($skuId, $at) timed out after ${cacheTimeoutMs}ms, write skipped" } + // A wedged file lock (timeout) and a broken write (IOException, corrupt file, no disk space) + // are the same to the caller — the stamp is lost, the bookkeeping around it carries on. + try { + withTimeoutOrNull(cacheTimeoutMs) { + dataStore.edit { prefs -> + prefs[lastProStateSkuKey] = skuId + prefs[lastProStateAtKey] = at + val episodeStart = prefs[proUnconfirmedSinceKey] ?: 0L + if (episodeStart in 1..at) prefs[proUnconfirmedSinceKey] = 0L + } + } ?: log(TAG, WARN) { "stampLastProState($skuId, $at) timed out after ${cacheTimeoutMs}ms, write skipped" } + } catch (e: CancellationException) { + // Caught before the general case on purpose: our caller going away is not a write + // failure, and swallowing it would break their structured concurrency. + throw e + } catch (e: Exception) { + log(TAG, WARN) { "stampLastProState($skuId, $at) failed, write skipped: ${e.asLog()}" } + } } companion object { diff --git a/app/src/testGplay/java/eu/darken/capod/common/upgrade/core/BillingCacheTest.kt b/app/src/testGplay/java/eu/darken/capod/common/upgrade/core/BillingCacheTest.kt index d0e64f67..89f1ed98 100644 --- a/app/src/testGplay/java/eu/darken/capod/common/upgrade/core/BillingCacheTest.kt +++ b/app/src/testGplay/java/eu/darken/capod/common/upgrade/core/BillingCacheTest.kt @@ -7,6 +7,7 @@ import androidx.test.core.app.ApplicationProvider import eu.darken.capod.common.datastore.value import io.kotest.assertions.throwables.shouldThrow import io.kotest.matchers.shouldBe +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.awaitCancellation import kotlinx.coroutines.flow.Flow @@ -105,6 +106,28 @@ class BillingCacheTest : BaseTest() { // The write only decorates the entitlement path, it must never block it. cache.stampLastProState(OurSku.Iap.PRO_UPGRADE.id, 1234L) } + + @Test + fun `a failing datastore write does not abort the entitlement bookkeeping`() = runTest { + // A broken write (corrupt file, no disk space) is not the same failure as a wedged lock, and + // the timeout alone doesn't cover it -- the exception would propagate straight through the + // stamp into the caller that was only decorating its entitlement work. + val cache = BillingCache(ThrowingPreferencesDataStore()) + + cache.stampLastProState(OurSku.Iap.PRO_UPGRADE.id, 1234L) + + // Reads stay loud: a snapshot that can't be read must not look like "never bought". + shouldThrow { cache.snapshot() } + } + + @Test + fun `a cancelled stamp still cancels`() = runTest { + // Fail-soft must not extend to cancellation -- swallowing it would break the structured + // concurrency of whatever entitlement work is being torn down. + val cache = BillingCache(CancellingPreferencesDataStore()) + + shouldThrow { cache.stampLastProState(OurSku.Iap.PRO_UPGRADE.id, 1234L) } + } } /** DataStore that never answers -- stands in for a wedged file lock. */ @@ -114,3 +137,19 @@ internal class HangingPreferencesDataStore : DataStore { override suspend fun updateData(transform: suspend (Preferences) -> Preferences): Preferences = awaitCancellation() } + +/** DataStore whose I/O fails -- stands in for a corrupt file or a full disk. */ +internal class ThrowingPreferencesDataStore : DataStore { + override val data: Flow = flow { throw IOException("Preferences file is broken") } + + override suspend fun updateData(transform: suspend (Preferences) -> Preferences): Preferences = + throw IOException("Preferences file is broken") +} + +/** DataStore whose write is cancelled from the outside. */ +internal class CancellingPreferencesDataStore : DataStore { + override val data: Flow = flow { throw CancellationException("Torn down") } + + override suspend fun updateData(transform: suspend (Preferences) -> Preferences): Preferences = + throw CancellationException("Torn down") +}