Stale-while-revalidate for the workflow detail fetch#3540
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3540 +/- ##
==========================================
+ Coverage 80.15% 80.18% +0.02%
==========================================
Files 371 371
Lines 15210 15233 +23
Branches 2110 2111 +1
==========================================
+ Hits 12192 12214 +22
- Misses 2168 2169 +1
Partials 850 850 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
d595495 to
8e422d6
Compare
This stack of pull requests is managed by Graphite. Learn more about stacking. |
| * linger on disk. | ||
| */ | ||
| @Synchronized | ||
| private fun pruneWorkflowDetailEnvelopesToList(workflowIds: Set<String>) { |
There was a problem hiding this comment.
I think it might be worth a note here saying the store is per-API-key and not per-user.
I guess its not a big deal, but for any PR about this topic makes me wonder if we should be more defensive.
There was a problem hiding this comment.
Yes, the store is per-API-key, not per-user, but that’s safe here. IdentityManager calls workflowsCache.clearCache() on every logIn/logOut transition, which wipes the entire disk envelope store before the new user’s data lands. So the store always reflects the current user.
Looks like there's one existing race (a prefetch completing after the clear can repopulate the store with old data). But it looks like it's the same we have in offerings, so maybe something we can tackle in a future PR?
| onSuccess: (WorkflowDataResult) -> Unit, | ||
| onError: (PurchasesError) -> Unit, | ||
| callbackDispatcher: Dispatcher? = null, | ||
| persistEnvelopeOnResolve: Boolean = false, |
There was a problem hiding this comment.
The kdoc explains the reasoning: on-demand fetches (not automatically prefetched) skip persistence to avoid unbounded disk growth. A user could open many distinct paywalls that are not prefetched, and there's no LRU cap today (will come in a future PR). The idea is to only store workflows that are prefetched.
| * bounded by wholesale-replacing its single response blob: the persisted set always equals what | ||
| * the latest backend response says exists, so workflows the backend stopped sending don't |
There was a problem hiding this comment.
Only only prefetch = true workflows that successfully resolved are ever persisted, right?
There was a problem hiding this comment.
Yes, exactly. Only cacheWorkflowDetailEnvelope writes to this store, and it is called only from the prefetch path after a successful resolve. Updated the pruneWorkflowDetailEnvelopesToList kdoc to make that invariant explicit.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tale prefetch still persists Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
8e422d6 to
8509e19
Compare
Linear https://linear.app/revenuecat/issue/WFL-281/stale-while-revalidate-for-the-workflow-detail-fetch
Stacked on #3537.
I noticed that in
OfferingsManagerwe do stale-while-revalidate serving (vendCachedOfferingsAndMaybeRefresh). Meaning that we serve stale data while fetching in the background more fresh offerings. We were not doing that for workflows, so there was a change in behavior. Before this PR a "stale" cache was a "miss" that blocked the paywall on a full backend round-trip.To understand it better, I've generated these diagrams:
How offerings already behaves
When a cached value exists it is always served right away; staleness only decides whether a background refresh is also kicked off. The caller never waits on the network when the cache has something to give.
Workflows before this PR
getWorkflowonly short-circuited on a fresh hit. A cached-but-stale workflow fell through to the same path as a cache miss and blocked the render on a full refetch, even though a perfectly usable workflow was sitting in the cache.Workflows after this PR
Now
getWorkflowserves the cached value immediately, then fire a background refresh that delivers no callbacks (a success just updates the cache, a failure is logged and swallowed).📋 Design context (for reviewers and AI agents)
Why
This is follow-up #3 from the workflow-detail-envelope-persistence decision. It is a different layer from the backend-down recovery work in #3537 underneath it: this is about normal in-memory serving, not disk fallback. The goal is parity with
OfferingsManager, which has always done stale-while-revalidate. The parity question applies to any SDK that caches workflow details (iOS once it has the cache layer — it is a stage behind; no iOS action here).The change
getWorkflowis rewritten from a two-way (fresh → serve, everything-else → block-and-fetch) into a three-way branch mirroringvendCachedOfferingsAndMaybeRefresh:onSuccess, then fire a background refresh that updates the cache only — no calleronSuccess/onError; a failed refresh is logged and swallowedonSuccess/onErrorMechanically: the fetch + resolve + cache + callback block was extracted into a private
fetchAndCacheWorkflow(...)(pure extraction, no behavior change), andgetWorkflownow dispatches into it from the three branches.Prefetch carve-out — the one deliberate exception
The split is expressed as a single parameter:
getWorkflow(..., staleWhileRevalidate: Boolean = true). Fresh and miss are identical across callers, so one boolean captures the difference cleanly (it only affects the stale-present case).PurchasesOrchestrator.getWorkflow):staleWhileRevalidate = true(default).prefetchWorkflow):staleWhileRevalidate = false.Prefetch must NOT adopt SWR. Its job is to force a real refresh and persist the detail envelope (the #3537 backend-down-recovery feature). If prefetch served stale + background-refreshed, its stale case would stop blocking-and-persisting and weaken that guarantee. So prefetch keeps today's blocking-fetch-and-persist behavior on a stale workflow.
The background refresh inherits the caller's
persistEnvelopeOnResolve. On the render path that isfalse, so the SWR background refresh does not persist — keeping this work cleanly out of the scope of the on-demand-persistence follow-up (#1).No new dedup
Two concurrent stale render calls can each fire a background refresh. This matches offerings —
fetchAndCacheOfferingson the stale path isn't deduped either — so it is parity-acceptable. Documented in a code comment rather than guarded.Test coverage
Note
Medium Risk
Changes paywall load timing when workflow cache is stale (immediate render vs blocking fetch); prefetch path is explicitly preserved but behavior diverges by caller.
Overview
getWorkflownow matches offerings-style stale-while-revalidate: a fresh hit is unchanged; a stale in-memory hit returns the cached workflow immediately and kicks off a background refetch that only updates the cache (failures are logged, not delivered to the caller); a miss still blocks on the network.The fetch/resolve/cache path is factored into
fetchAndCacheWorkflow. A newstaleWhileRevalidateflag (defaulttruefor on-demand paywall loads) controls the stale branch;prefetchWorkflowpassesfalseso prefetch still blocks, refetches, and persists detail envelopes when the cache is stale.Tests cover immediate stale serving, background cache updates, swallowed background errors, blocking behavior when SWR is off, and stale prefetch persistence.
WorkflowsCacheprune docs are clarified only.Reviewed by Cursor Bugbot for commit 8509e19. Bugbot is set up for automated code reviews on this repo. Configure here.