From 9f705cfb6c267af02f8b61345a01f539456c8070 Mon Sep 17 00:00:00 2001 From: Static Date: Fri, 21 Aug 2026 17:56:42 -0400 Subject: [PATCH] fix: treat Play's generic ERROR as a shrug, not as an answer The settle gate exists so the claim prompt never acts on a provisional reading of what a user owns. Its retryable set held SERVICE_DISCONNECTED, SERVICE_UNAVAILABLE and NETWORK_ERROR, and left out ERROR. ERROR reads like a hard failure and is not one. Google documents it as Play's generic "something went wrong on our side" and recommends retrying. So it belongs in the set the gate refuses to settle on, by that set's own stated definition: codes where Play said NOTHING, as opposed to saying "no". What it cost, concretely. An unlock owner reinstalls. The Play-grant cache is excluded from backup on purpose, so entitlement reads free until the first query answers. That query returns a transient ERROR, the gate settles on it, and the claim dialog offers them a free unlock code for the thing they already bought. Not an entitlement bug: revocation still demands a SUCCESSFUL query and PurchaseEvaluator's asymmetry is untouched. It costs them a confusing dialog and us a support email. This is the same finding an earlier review round raised. That round fixed the transport-failure case and built a set that was incomplete. WHY THERE IS A NEW FILE. The set had no test, which is how ERROR stayed missing through three review rounds. It could not have one where it lived: classifying a response code meant reaching through a suspend function that needs a BillingClient. So the classification moves to PurchaseGatePolicy, holding no Billing types, for the reason PurchaseEvaluator already gives in its own header. BillingManager keeps the behaviour and loses the untestable copy. The mirrored constants carry a drift guard. If Google renumbers a code, the test fails instead of silently reclassifying it. CANARIES, both observed failing before revert: - Removing ERROR from the set turned the two shrug tests red. - Setting NETWORK_ERROR to 99 turned the drift guard red. 427 tests, 0 failures, 52 of 52 tasks executed. assembleDebug clean. Co-Authored-By: Claude Opus 5 --- .../app/entitlement/BillingManager.kt | 26 ++----- .../app/entitlement/PurchaseGatePolicy.kt | 41 ++++++++++++ .../app/entitlement/PurchaseGatePolicyTest.kt | 67 +++++++++++++++++++ 3 files changed, 115 insertions(+), 19 deletions(-) create mode 100644 app/src/main/java/com/ninelivesaudio/app/entitlement/PurchaseGatePolicy.kt create mode 100644 app/src/test/java/com/ninelivesaudio/app/entitlement/PurchaseGatePolicyTest.kt diff --git a/app/src/main/java/com/ninelivesaudio/app/entitlement/BillingManager.kt b/app/src/main/java/com/ninelivesaudio/app/entitlement/BillingManager.kt index a1249f4..ff53fa2 100644 --- a/app/src/main/java/com/ninelivesaudio/app/entitlement/BillingManager.kt +++ b/app/src/main/java/com/ninelivesaudio/app/entitlement/BillingManager.kt @@ -104,10 +104,10 @@ class BillingManager @Inject constructor( * survive a reinstall or a device move. Anything that acts on "user is free" * during that window acts on a value that has not been established yet. * - * "Answer" excludes retryable transport failures. A disconnected service or a - * dead network is Play saying NOTHING, not Play saying "you own nothing", and - * flipping this on one hands a consumer a provisional free reading dressed up - * as an established one. See [RETRYABLE_RESPONSE_CODES]. + * "Answer" excludes retryable failures. A disconnected service, a dead + * network, or Play's generic ERROR is Play saying NOTHING, not Play saying + * "you own nothing", and flipping this on one hands a consumer a provisional + * free reading dressed up as an established one. See [PurchaseGatePolicy]. * * This flow can therefore stay false forever, and that is deliberate: a device * with no Play Store never completes setup, so [refreshPurchases] never runs @@ -255,7 +255,9 @@ class BillingManager @Inject constructor( // and us the sale. if (responseOk) acknowledgeIfNeeded(result.purchasesList) - if (!responseOk && responseCode in RETRYABLE_RESPONSE_CODES) { + // Classification lives in PurchaseGatePolicy so it can be tested without + // a Billing client. See PurchaseGatePolicyTest. + if (!responseOk && !PurchaseGatePolicy.settles(responseCode)) { Log.d(TAG, "purchase query not answered ($responseCode), gate stays open") return false } @@ -377,20 +379,6 @@ class BillingManager @Inject constructor( */ const val BILLING_TIMEOUT_MS = 30_000L - /** - * Codes where Play said NOTHING, as opposed to saying "no". - * - * These are the ones auto-reconnection can plausibly fix on its own - * within seconds, so they must not settle [purchaseQuerySettled]. - * Everything else, including BILLING_UNAVAILABLE and DEVELOPER_ERROR, - * is a stable answer that a retry would only repeat. - */ - val RETRYABLE_RESPONSE_CODES = setOf( - BillingClient.BillingResponseCode.SERVICE_DISCONNECTED, - BillingClient.BillingResponseCode.SERVICE_UNAVAILABLE, - BillingClient.BillingResponseCode.NETWORK_ERROR, - ) - /** * One Play purchase can carry several product ids, so flatten rather * than assuming index zero. diff --git a/app/src/main/java/com/ninelivesaudio/app/entitlement/PurchaseGatePolicy.kt b/app/src/main/java/com/ninelivesaudio/app/entitlement/PurchaseGatePolicy.kt new file mode 100644 index 0000000..e61a344 --- /dev/null +++ b/app/src/main/java/com/ninelivesaudio/app/entitlement/PurchaseGatePolicy.kt @@ -0,0 +1,41 @@ +package com.ninelivesaudio.app.entitlement + +/** + * Decides whether a `queryPurchasesAsync` response is a final answer from Play. + * + * Deliberately holds no Billing types, for the same reason [PurchaseEvaluator] + * does not: the classification is the part that must be exhaustively tested, + * and it should not need a Billing client, a Play Store, or a network to run. + * `PurchaseGatePolicyTest` pins the mirrored values against the real library so + * they cannot drift silently. + */ +object PurchaseGatePolicy { + + const val SERVICE_DISCONNECTED = -1 + const val SERVICE_UNAVAILABLE = 2 + const val NETWORK_ERROR = 12 + const val ERROR = 6 + + /** + * Codes where Play said NOTHING, as opposed to saying "no". + * + * These are the ones auto-reconnection or a plain retry can plausibly turn + * into a real answer within seconds, so they must not settle the gate. + * Everything else, including BILLING_UNAVAILABLE and DEVELOPER_ERROR, is a + * stable answer that a retry would only repeat. + */ + private val RETRYABLE = setOf( + SERVICE_DISCONNECTED, + SERVICE_UNAVAILABLE, + NETWORK_ERROR, + // ERROR is Play's generic "something went wrong on our side", which + // Google documents as retryable. It reads like a hard failure and is + // not one. Omitting it is what let a reinstalled unlock owner, whose + // local Play cache is empty, get offered a free code for the unlock + // they already bought. + ERROR, + ) + + /** True when this response is an answer the gate may settle on. */ + fun settles(responseCode: Int): Boolean = responseCode !in RETRYABLE +} diff --git a/app/src/test/java/com/ninelivesaudio/app/entitlement/PurchaseGatePolicyTest.kt b/app/src/test/java/com/ninelivesaudio/app/entitlement/PurchaseGatePolicyTest.kt new file mode 100644 index 0000000..ad05478 --- /dev/null +++ b/app/src/test/java/com/ninelivesaudio/app/entitlement/PurchaseGatePolicyTest.kt @@ -0,0 +1,67 @@ +package com.ninelivesaudio.app.entitlement + +import com.android.billingclient.api.BillingClient.BillingResponseCode +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The settle gate decides WHEN the claim prompt is allowed to trust its reading + * of a user's purchases. Settling early on a code that a retry could have + * answered means offering a free unlock code to somebody who already bought + * one, which costs them a confusing dialog and us a support email. + * + * This set had no test before 2026-08-21, which is how ERROR stayed missing + * from it through three review rounds. + */ +class PurchaseGatePolicyTest { + + @Test + fun `a Play shrug does not settle the gate`() { + // Codes where Play said nothing usable and a retry can plausibly fix it. + assertFalse(PurchaseGatePolicy.settles(BillingResponseCode.SERVICE_DISCONNECTED)) + assertFalse(PurchaseGatePolicy.settles(BillingResponseCode.SERVICE_UNAVAILABLE)) + assertFalse(PurchaseGatePolicy.settles(BillingResponseCode.NETWORK_ERROR)) + assertFalse(PurchaseGatePolicy.settles(BillingResponseCode.ERROR)) + } + + @Test + fun `a stable answer settles the gate`() { + // A retry would only repeat these, so waiting only delays a correct call. + assertTrue(PurchaseGatePolicy.settles(BillingResponseCode.OK)) + assertTrue(PurchaseGatePolicy.settles(BillingResponseCode.BILLING_UNAVAILABLE)) + assertTrue(PurchaseGatePolicy.settles(BillingResponseCode.DEVELOPER_ERROR)) + assertTrue(PurchaseGatePolicy.settles(BillingResponseCode.FEATURE_NOT_SUPPORTED)) + assertTrue(PurchaseGatePolicy.settles(BillingResponseCode.ITEM_UNAVAILABLE)) + } + + /** + * ERROR is the regression this test exists for. + * + * A reinstalled unlock owner has no local Play cache. If that first + * queryPurchasesAsync returns a transient ERROR and the gate settles on it, + * the claim dialog reads "free" as established and offers them a code for + * the unlock they already own. Google documents ERROR as retryable. + */ + @Test + fun `transient ERROR is treated as retryable, not as an answer`() { + assertFalse( + "ERROR is a Play shrug. Settling on it offers a paid user a free code.", + PurchaseGatePolicy.settles(BillingResponseCode.ERROR), + ) + } + + /** + * Guards against the pure layer drifting from the Billing library it + * mirrors. If Google renumbers a code, this fails rather than silently + * reclassifying it. + */ + @Test + fun `mirrored codes still match the Billing library`() { + assertEquals(BillingResponseCode.SERVICE_DISCONNECTED, PurchaseGatePolicy.SERVICE_DISCONNECTED) + assertEquals(BillingResponseCode.SERVICE_UNAVAILABLE, PurchaseGatePolicy.SERVICE_UNAVAILABLE) + assertEquals(BillingResponseCode.NETWORK_ERROR, PurchaseGatePolicy.NETWORK_ERROR) + assertEquals(BillingResponseCode.ERROR, PurchaseGatePolicy.ERROR) + } +}