Skip to content

Keep subscribers active through a Play read that answered nothing - #470

Merged
ianrumac merged 3 commits into
developfrom
ytor/empty-read-no-downgrade
Oct 5, 2026
Merged

ianrumac merged 3 commits into
developfrom
ytor/empty-read-no-downgrade

Conversation

@yusuftor

Copy link
Copy Markdown
Contributor

Changes in this pull request

Ports the iOS anti-downgrade guard (Superwall-iOS 4.17.0) to Android. An empty Play Billing read set the status to Inactive no matter why it came back empty.

Three holes this closes:

  1. A failed read still demoted. failed is computed when either query gives up after its retries, but the status was published first and the retry only scheduled after — so a live subscriber went inactive for at least a second, up to three retries' worth.
  2. A broken billing connection did the same. onBillingSetupFinished syncs even when the response code isn't OK, the queries then fail fast with "Billing client not ready", and the user was deactivated. This is the Android equivalent of the iOS cold-launch case.
  3. A mapping failure demoted. An active purchase whose product config no longer knows produced an empty entitlement set, which read as an authoritative demotion of a paying subscriber.

The rule is now: nothing that isn't an answer may demote a subscriber whose entitlement hasn't expired. Each entitlement is judged on its own, with two separate questions — whether it can hold the status up (needs an unexpired date), and whether it survives (the read had no authority over it or confirmed it, and its own expiry hasn't passed).

The downgrade was also sticky: Entitlements.setSubscriptionStatus's Inactive branch clears the backing sets and the status is persisted, so the wrong answer was what the next cold launch restored.

Behaviour that deliberately did not change

A read that succeeds and reports no purchases is still an answer and deactivates straight away — Play lists every active purchase it knows about. Refunds and cancellations behave exactly as before. Web/Stripe entitlements were never exposed here; internallySetSubscriptionStatus already merges them back from the redeem cache.

Android-specific wrinkle

The status carries config-shaped entitlements, which have no expiresAt and often no store — the enriched ones live in the device CustomerInfo that ReceiptManager builds from Play receipts (and which persists across launches). The guard fills in only what an entitlement doesn't already state, so a web entitlement keeps its own store and dates even if a device record shares its id.

The guard is a pure function (resolveStatusForEmptyRead) so it's testable without mocking billing or the singleton. 13 new tests; full suite is 1334 passing.

Note for the release manager

version.env and CHANGELOG.md start a 2.8.5 section, since 2.8.4 is already tagged and develop had no unreleased entries. Happy to drop the bump if you'd rather do it at release time.

Checklist

  • All unit tests pass.
  • All UI tests pass.
  • Demo project builds and runs.
  • I added/updated tests or detailed why my change isn't tested.
  • I added an entry to the CHANGELOG.md for any breaking changes, enhancements, or bug fixes.
  • I have run ktlint in the main directory and fixed any issues.
  • I have updated the SDK documentation as well as the online docs.
  • I have reviewed the contributing guide

🤖 Generated with Claude Code

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The third hole the description claims to close — "a mapping failure demoted" — is not actually closed: isRefuted refutes every real Play entitlement, and the test covering that case passes because its inputs are hand-matched literals that don't occur in production. Separately, the new activeDeviceEntitlements write leaks web entitlements into a field WebPaywallRedeemer.clear() relies on being Play-only.

Reviewed changes — the full diff at 1af7890: the new empty-read guard, its two call sites, and the 13 new tests, traced against the real shapes of Entitlement as they flow from config, from LatestDeviceCustomerInfo, and from Play Billing.

  • resolveStatusForEmptyRead — new pure function in superwall/src/main/java/com/superwall/sdk/store/EmptyReadStatus.kt that decides whether an entitlement-less Play read may demote a subscriber, backfilling expiresAt/store from the stored device CustomerInfo.
  • AutomaticPurchaseController wiring — both SubscriptionStatus.Inactive returns in syncSubscriptionStatusAndWait now route through a private statusForEmptyRead, which also keeps entitlementsInfo().activeDeviceEntitlements in step; a new deviceEntitlementRecords constructor lambda reads LatestDeviceCustomerInfo.
  • activeProductIds — new set built from PURCHASED purchases, used to decide whether a read refuted a given entitlement.
  • Tests — EmptyReadStatusTest.kt, 13 cases covering failed reads, successful empty reads, lapsed entitlements, web-vs-Play authority, and lifetime unlocks.
  • Release metadata — version.env 2.8.4 → 2.8.5 and a matching CHANGELOG section.

The two holes the guard does close (readFailed after retries, and a never-ready billing client — both short-circuit isRefuted via readFailed) are genuinely fixed, and the "succeeded and found nothing still demotes" fast path is correct.

⚠️ The device wall clock is the only thing that can end a held-up Active

On a non-answer the guard keeps Active as long as expiresAt.after(now), and now defaults to Date() — local device time, never overridden by the caller. Before this PR an empty read always fell to Inactive, so the clock was never load-bearing for entitlement state. It now is, and the loop has no terminal state: queryPurchasesOfType fails fast whenever billingClient.isReady is false, onBillingSetupFinished re-syncs on every reconnect, and each resolved Active is persisted to StoredSubscriptionStatus and restored on the next cold launch. This is a deliberate trade-off on iOS too, so it may well be the intended cost — but it is a new fail-open surface and worth an explicit decision rather than an accident.

Technical details
# Held-up `Active` has no clock-independent expiry

## Affected sites
- `superwall/src/main/java/com/superwall/sdk/store/EmptyReadStatus.kt:57` — `now: Date = Date()`; `AutomaticPurchaseController.statusForEmptyRead` never passes a value, so the guard always evaluates against local time.
- `superwall/src/main/java/com/superwall/sdk/store/EmptyReadStatus.kt:76-79` — `isLapsed` is the sole exit from the held-up state once `readFailed` is permanently true.
- `superwall/src/main/java/com/superwall/sdk/store/AutomaticPurchaseController.kt:511-513` — `queryPurchasesOfType` returns failure immediately when `!billingClient.isReady`, so no network is required to keep `readFailed` true indefinitely.

## Required outcome
- An explicit decision on whether an unbounded, locally-verified `Active` is acceptable, or whether the guard should be bounded by something the device cannot move — e.g. a cap on how long a status may be held without a successful read (iOS-style grace window), or a server-side entitlement check.

## Open questions for the human
- Does iOS 4.17.0 bound the hold-up period, or is it purely `expiresAt`-vs-local-clock there too? Matching iOS exactly may be the right call here, in which case this only needs a comment recording the trade-off.

ℹ️ Nitpicks

  • readReturnedPurchases = allPurchases.isNotEmpty() (AutomaticPurchaseController.kt:434, :446) counts PENDING and UNSPECIFIED_STATE purchases, not just PURCHASED. I could not find an outcome that diverges because of it (activeProductIds is empty in exactly those cases, so every Play entitlement is refuted anyway and the result matches the fast path), but the name promises a narrower predicate than the expression delivers.
  • The deviceEntitlementRecords catch block (AutomaticPurchaseController.kt:68-77) binds e and then drops it — every other Logger.debug in this file passes error = e, and a swallowed storage exception here silently disables the whole guard.
  • On the version.env question in the description: build.gradle.kts:22-36 resolves superwallVersion from SUPERWALL_VERSION (env first, then version.env), so the bump is self-consistent and nothing else needs updating — it is purely a release-timing call.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using Claude Opus | 𝕏

Comment thread superwall/src/main/java/com/superwall/sdk/store/EmptyReadStatus.kt Outdated
Comment thread superwall/src/main/java/com/superwall/sdk/store/AutomaticPurchaseController.kt Outdated
Comment thread superwall/src/main/java/com/superwall/sdk/store/EmptyReadStatus.kt

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found. Every finding from the previous review is addressed. The mapping-failure hole is now actually closed, and tests built from production-shaped entitlements cover it.

Reviewed changes: I reviewed 7090d42 ("Address review on the empty-read guard") against the prior review at 1af7890.

  • Fixed product-id refutation: productIdsOf now falls back to the device record's product ids when the entitlement has none, and maps full config ids to DecomposedProductIds.subscriptionId so they compare against bare Play ids. An entitlement with no known product ids is no longer refuted.
  • Kept activeDeviceEntitlements Play-only: statusForEmptyRead now writes only survivors whose own or device-record store is PLAY_STORE, plus store-less ones that Entitlements.web doesn't claim. This keeps WebPaywallRedeemer.clear() and askToRestoreFromWeb correct.
  • Dropped inactive entitlements from survivors: the filter now requires isActive, matching the .copy(isActive = true) branch where the read returns entitlements.
  • Documented the trade-offs: the KDoc now states that entitlements with no expiry date (lifetime unlocks, or product details unavailable when receipts were processed) still demote on a non-answer. It also states that the device clock is the only bound on how long a held-up status lasts, the same as iOS.
  • Added realistic tests: four new cases. Two pair a config-shaped status entitlement with a device record holding a full id (pro_sub:monthly:sw-auto) against a bare Play id. Both would fail against the previous isRefuted.
  • Passed the error to the logger: the deviceEntitlementRecords catch now passes error = e.

Pullfrog  | View workflow run | Using claude-opus-5.5 | 𝕏

yusuftor and others added 3 commits October 5, 2026 16:16
An empty Play Billing read set the status to inactive no matter why it
was empty. The queries can fail outright - the billing client is not
ready at launch, the query times out, the retries run out - and the
code published inactive first and only then scheduled its retry, so a
paying subscriber lost access for at least a second. An active purchase
that config no longer maps to an entitlement did the same.

Nothing that is not an answer may now demote a subscriber whose
entitlement has not expired. A read that succeeds and reports no
purchases is still an answer and deactivates straight away, so refunds
and cancellations behave as before.

The status carries config-shaped entitlements, which have no expiry
date, so the dates come from the device CustomerInfo that ReceiptManager
builds from Play receipts and persists across launches.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- Match refuting product ids across namespaces: backfill productIds from
  the device records and compare on the subscription id, so config-shaped
  entitlements and full config ids line up with raw Play ids. An
  entitlement with no known product ids is never refuted.
- Keep web entitlements out of activeDeviceEntitlements so
  WebPaywallRedeemer.clear() and the restore message stay Play-only.
- Don't carry entitlements flagged inactive into the published Active.
- Document the null-expiry limitation and the device-clock trade-off.
- Log the swallowed storage error.
- Add tests built from config-shaped entitlements and real id shapes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ianrumac
ianrumac force-pushed the ytor/empty-read-no-downgrade branch from 7090d42 to 57be470 Compare October 5, 2026 14:18
@ianrumac
ianrumac merged commit 1bfc5bc into develop Oct 5, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants