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.
This commit is contained in:
darken
2026-08-02 12:40:32 +02:00
committed by Matthias Urhahn
parent 31c3b47bcd
commit 72fb1d3b17
2 changed files with 59 additions and 8 deletions
@@ -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 {
@@ -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<IOException> { 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<CancellationException> { 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<Preferences> {
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<Preferences> {
override val data: Flow<Preferences> = 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<Preferences> {
override val data: Flow<Preferences> = flow { throw CancellationException("Torn down") }
override suspend fun updateData(transform: suspend (Preferences) -> Preferences): Preferences =
throw CancellationException("Torn down")
}