Publish config before the StoreKit read for cached subscribers - #519
Publish config before the StoreKit read for cached subscribers#519jakemor wants to merge 1 commit into
Conversation
A subscriber with a cached config takes the async config path, so the config itself is available at once. But configState was only published after processConfig finished, and processConfig awaits loadPurchasedProducts, which reads StoreKit. On a weak network that read took 18 to 23 seconds in production. Every register call waits on configState, so gated features froze for that long at cold launch, and a feature closure that never ran before the app was killed was lost. fetchConfiguration now publishes configState right after the config is applied and before the purchases load starts, on the cached-config path only. The sync path keeps its order: with no known subscriber, purchases are still read before a paywall can be shown. The purchases load is kept in ConfigManager.initialPurchasesLoad, and DependencyContainer.isFreeTrialAvailable awaits it. That preserves the upgrade/crossgrade trial gate, which depends on the active subscription groups the load computes. Three tests: the reproduction (config must be available while a 2s StoreKit read is still running), the sync path staying unchanged, and trial eligibility waiting for the load. 🌸 Shipped with Kanna — https://kanna.sh Co-Authored-By: Kanna <noreply@kanna.sh> Kanna-Agent: claude/fable
|
PR author is not in the allowed authors list. |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Publishing configState early opens a window in which several other consumers of loadPurchasedProducts read empty or stale state. Only trial eligibility was given a gate, and the new ReceiptManager comment asserts a guarantee that TransactionManager falsifies.
Reviewed changes — the full diff at 6d954b73 (5 files, 1 commit), plus the consumers of everything ReceiptManager.loadPurchasedProducts populates.
- Early
configStatepublish on the cached-config path —processConfiggainspublishBeforeLoadingPurchases, wrapsloadPurchasedProductsin an unstructuredTask, sendsconfigStatebefore awaiting it, and returns whether it published. fetchConfigurationstep reordering —processConfigis hoisted above the device-attributes block whenshouldFetchAsync; the sync path keeps the original order and publishes at the old site.initialPurchasesLoadhandle — newprivate(set) varonConfigManager, awaited byDependencyContainer.isFreeTrialAvailableso the upgrade/crossgrade trial gate still seesactiveSubscriptionGroupIds.ReceiptManagercomment rewrite — restates the invariant thatactiveSubscriptionGroupIdsis loaded before any caller reads it.ConfigManagerStoreKitStallTests— three tests over aSlowReceiptManagerTypewhose firstloadPurchasessleeps. The assertions are real:#expect(waited < 1)with a 2 s load genuinely fails ondevelop.- CHANGELOG — added under the existing unreleased
## 4.17.0section, which matches theCLAUDE.mdrule.
⚠️ The "await the load" invariant is a one-off, not a contract
initialPurchasesLoad is referenced exactly once in the codebase, and no test covers any consumer of the window other than trial eligibility. Every future reader of post-load state has to independently remember that config being .retrieved no longer implies purchases are loaded, and nothing in the type system or the tests says so. The two inline findings are both instances of this; the general question is whether the fix should stay a per-consumer await or become a single gate that the presentation pipeline passes through.
Technical details
# `initialPurchasesLoad` has no enforcement or coverage beyond one call site
## Affected sites
- `Sources/SuperwallKit/Config/ConfigManager.swift:41` — `initialPurchasesLoad` is public-read but its "you must await this" contract lives only in a doc comment.
- `Sources/SuperwallKit/Dependencies/DependencyContainer.swift:620` — the only reader in the whole repo.
- `Tests/SuperwallKitTests/Config/ConfigManagerTests.swift:768` — the only test of the window covers trial eligibility; nothing covers audience attributes, template variables, or the purchase path.
## Required outcome
- Either every consumer of `loadPurchasedProducts` output that is reachable during the window awaits the load, or the ones that deliberately do not are enumerated with the staleness they accept.
- A regression test that fails if a new consumer reads post-load state without the await, rather than three tests that each pin one known-good path.
## Open questions for the human
- Is the intended long-term shape a per-consumer `await`, or a single choke point (e.g. inside `waitForSubsStatusAndConfig`, or a `PaywallRequestManager` precondition) that the whole presentation pipeline passes through?
- Which staleness is acceptable by design here? `device.subscriptionStatus` / `activeEntitlements` come from disk and can describe a subscriber who lapsed since last launch; letting `register` answer from that value is arguably the point of the PR, but it is not stated anywhere.ℹ️ Nitpicks
Tests/SuperwallKitTests/Config/ConfigManagerTests.swift:794—didStartLoad/didFinishLoadare plainvars on an@unchecked Sendableclass, written by the load task and read by the test task. No sanitizer is enabled inproject.ymlorscripts/test.shso CI won't flag it, but the release these tests ship in also fixes a TSan-reported configure race, so anNSLockor an actor would keep the suite clean if anyone turns TSan on.Sources/SuperwallKit/Config/ConfigManager.swift:467— the purchases load moves from structured concurrency into an unstructuredTask, so it no longer inherits cancellation fromfetchConfiguration. Nothing cancels that call today, so this is an observation rather than a bug.
Claude Opus ｜ 𝕏
| await factory.loadPurchasedProducts(config: config) | ||
| } | ||
| initialPurchasesLoad = purchasesLoad | ||
| configState.send(.retrieved(config)) |
There was a problem hiding this comment.
loadPurchasedProducts populates more than the trial gate, and only that one consumer got an await. manager.purchases has no disk backing (SK1ReceiptManager.swift:13, SK2ReceiptManager.swift:66 are plain = []), so getActiveProductIds() → DeviceTemplate.activeProducts (DeviceHelper.swift:1018) is empty for every audience filter evaluated between this send and the load finishing — for exactly the subscribed users this path targets. Separately, receiptDelegate?.syncSubscriptionStatus (ReceiptManager.swift:216) is what demotes a lapsed or refunded subscriber, and it now lands after register has already been answered from the disk-cached status.
Technical details
# Post-load state readable while empty/stale on the cached-config path
## Affected sites
- `Sources/SuperwallKit/Config/ConfigManager.swift:471` — `configState.send(.retrieved(config))` now precedes `await purchasesLoad.value`, unblocking every `register`/`track` awaiter.
- `Sources/SuperwallKit/StoreKit/Products/Receipt Manager/Receipt Manager/ReceiptManager.swift:272` — `getActiveProductIds()` reads `manager.purchases`, which is in-memory only and populated solely by `loadPurchases` inside the deferred task.
- `Sources/SuperwallKit/Network/Device Helper/DeviceHelper.swift:1018` — `activeProducts:` in `getTemplateDevice()`, which feeds `DependencyContainer.makeAudienceFilterAttributes` (`:471`, the `device` dict handed to CEL) and `makeJsonVariables` (`:359`, paywall template variables).
- `Sources/SuperwallKit/StoreKit/Products/Receipt Manager/Receipt Manager/ReceiptManager.swift:264` — `isSubscribed(to:)`, same backing, surfaced as the per-product `isSubscribed` template variable via `TemplateLogic.swift:29`.
- `Sources/SuperwallKit/StoreKit/Products/Receipt Manager/Receipt Manager/ReceiptManager.swift:216` — `syncSubscriptionStatus(purchases:)`, the correction that deactivates refunded/expired App Store subscriptions, now runs concurrently with presentation instead of before it.
## Required outcome
- Audience-filter evaluation during the window must not silently report `device.activeProducts` as empty for a user who owns products; either the value is restored/derived from disk, or the attribute is omitted rather than reported as an empty set, or the evaluation awaits the load.
- The staleness accepted for `device.subscriptionStatus` / `activeEntitlements` during the window should be a stated decision, since the neighbouring CHANGELOG entry promises that refunded and expired App Store subscriptions "still deactivate immediately".
## Open questions for the human
- How widely is `device.activeProducts` used in customer audience filters? If it is rare, documenting the window may be enough; if it is common, an empty set is a silent wrong-audience match for the whole cold-launch window.
- Is a lapsed subscriber getting gated features unlocked for the duration of the StoreKit read an accepted trade-off for the latency win?| // `activeSubscriptionGroupIds` is populated in `loadPurchasedProducts`. On the | ||
| // sync config path that load completes before config is published. On the | ||
| // cached-config path config is published first, so paywall callers go through | ||
| // `DependencyContainer.isFreeTrialAvailable`, which awaits the load before | ||
| // reaching here. |
There was a problem hiding this comment.
The claim that paywall callers all go through DependencyContainer.isFreeTrialAvailable does not hold: TransactionManager.isFreeTrialAvailable (TransactionManager.swift:734) calls receiptManager.isFreeTrialAvailable directly and is reached from prepareToPurchase (:655) the moment a user taps buy — newly possible during the load window that this PR opens. With activeSubscriptionGroupIds still empty, an in-group upgrade gets reported as .freeTrialStart instead of .subscriptionStart.
Technical details
# `ReceiptManager.isFreeTrialAvailable` has a caller that bypasses `initialPurchasesLoad`
## Affected sites
- `Sources/SuperwallKit/StoreKit/Products/Receipt Manager/Receipt Manager/ReceiptManager.swift:255-259` — the new comment asserts a universal guarantee.
- `Sources/SuperwallKit/StoreKit/Transactions/TransactionManager.swift:734` — `return await receiptManager.isFreeTrialAvailable(for: product)`, via a directly held `receiptManager`, not `factory`. `TransactionManager.Factory` is `OptionsFactory` only (`:21`), so it currently cannot reach the awaited wrapper.
- Downstream: `TransactionManager.swift:655` → `:725` → `PurchasingCoordinator.isFreeTrialAvailable` → `TransactionManager.swift:1092` `didStartOffer` → `:1103` `TransactionType.freeTrialStart` vs `.subscriptionStart`.
- Secondary, lower severity: `Sources/SuperwallKit/Paywall/Request/Operators/AddPaywallProducts.swift:311` — the `.eligible` branch calls `hasActiveIntroOffer`, which reads `Superwall.shared.customerInfo.subscriptions` (`:336-338`) with no `isPlaceholder` guard, unlike `hasEverHadEntitlement` at `:368`. That value is disk-restored so it is stale rather than empty, but it drives the user-facing `paywall.isFreeTrialAvailable` template value.
## Required outcome
- Either the purchase path observes the same `activeSubscriptionGroupIds` guarantee as the paywall path, or the comment is corrected to say which callers are unprotected and why that is acceptable.
## Suggested approach
- Widen `TransactionManager.Factory` to `OptionsFactory & ReceiptFactory` and route `:734` through `factory.isFreeTrialAvailable(for:)`. `DependencyContainer` already conforms, so every production and test call site is unchanged.| ) | ||
| } | ||
|
|
||
| // Step 6: Track device attributes |
There was a problem hiding this comment.
Hoisting processConfig above this block means the device_attributes track now happens after the StoreKit read on the async path, since processConfig still awaits purchasesLoad.value before returning. That delays the event by the same 10–25 s the PR is removing from register, on the exact launches the PR description measures. Was that intended, or should the track stay ahead of the load and only configState move?
1 participant
The problem
Follow-up to #506. That PR stopped subscribers from flipping to
inactiveat cold launch. Production data from the same app (Natural, application 1128, build 3.7 (42) on the #506 branch) shows the entitlement now holds, butregistercalls still stall at cold launch on a weak network:app_launchtoconfig_attributesregisterwaitOn those launches nothing is tracked between the launch events and
config_attributes. The app's own placements, fired 3 s and 10 s after launch, are only created once config is ready, becauseregisterwaits onconfigStateand eachregisterwaits on the previous one. The user is a Stripe subscriber with a cached config, so the async config path was taken and the config itself was available at once. The gap isprocessConfig, which awaitsloadPurchasedProductsbeforeconfigState.send(.retrieved). That call reads StoreKit, and the background config refresh that ran right after took 13 s with 2 retries, so the network was poor.For a camera app that gates the capture pipeline on
register, that is a 14 s stall before a photo is processed, and a lost photo if the app is killed first.The fix
On the cached-config path,
fetchConfigurationnow publishesconfigStateas soon as the config is applied (stored, triggers mapped, variants chosen, test mode evaluated) and before the purchases load starts. Nothing on that path needs the network or StoreKit to answer a presentation request: the config is on disk and the subscription status was restored from disk inSuperwall.init.The sync path keeps its order. With no known subscriber, purchases are still read before config is published, so a paywall never shows to someone whose App Store subscription has not been read yet.
One invariant needed care.
ReceiptManager.isFreeTrialAvailablerelies on the active subscription groups computed inloadPurchasedProductsto suppress trials on upgrades, and its comment said the load always completes before a paywall opens. The load task is now kept inConfigManager.initialPurchasesLoad, andDependencyContainer.isFreeTrialAvailable(the paywall products path) awaits it, so that gate still sees the loaded groups.Tests
Three new tests in
ConfigManagerTests.swift, using aReceiptManagerTypewhose firstloadPurchasessleeps:configis non-nil within 1 s while the 2 s StoreKit read is still running, andfetchConfigurationstill waits for the read to finish. Fails ondevelop(config was nil after 1.5 s).Full suite: 997 tests in 101 suites pass.
Not covered
I could not tell from the event data which StoreKit call inside
loadPurchasedProductsis the slow one. The fix does not need to know: config readiness no longer depends on any of them.🌸 Shipped with Kanna — an open-source workspace for all your coding agents. Written by
claude/fable.