fix(cache): serve stale use cache entries during refresh#2682
Open
NathanDrake2406 wants to merge 11 commits into
Open
fix(cache): serve stale use cache entries during refresh#2682NathanDrake2406 wants to merge 11 commits into
NathanDrake2406 wants to merge 11 commits into
Conversation
Valid stale shared cache entries currently fall through to the miss path, so the triggering request waits for regeneration even though the entry remains usable until expiry. Return stale data immediately and coordinate one background refresh per cache key. Keep prerender refreshes in the foreground, isolate regeneration metadata while preserving supported cache-scope inputs, and attach guarded work to the request lifetime when available.
Runtime ISR regeneration could store a fresh route artifact containing stale use-cache data because stale hits always detached their refresh outside build-time prerendering. That ignored the request policy already used by unstable_cache to require fresh dependencies while regenerating an artifact.\n\nUse one cache revalidation mode for both APIs, isolate background refresh request state behind a named boundary, and share the per-key scheduler lifecycle. The runtime ISR regression now proves a foreground stale read blocks, stores the fresh value, and registers no background work.
Draft requests could return stale published data and regenerate preview output into the persistent use-cache entry because the shared handler path did not consult the active draft state.\n\nExecute public and remote use-cache callbacks directly inside their normal cache scope when draft mode is enabled. Background refreshes no longer inherit headers or cookies, since persistent regeneration now runs only for non-draft requests.
Background use-cache regeneration lost its headers context entirely, so cached callbacks that read draftMode().isEnabled failed after a stale hit even though that read is supported inside cache scopes. Give detached refreshes a disabled draft-mode provider with empty header and cookie stores. This keeps preview state isolated while allowing the callback to regenerate and persist the fresh entry.
Expired values returned by cache handlers were treated as ordinary hits because only stale entries received special handling. Memory-backed entries could therefore remain visible indefinitely without triggering regeneration. Choose an explicit read action from the entry state and request revalidation policy. Stale values remain eligible for background refresh during ordinary requests, while expired and unrecognized states regenerate in the foreground for both unstable_cache and "use cache".
Contributor
Performance benchmarksCompared 0 improved · 0 regressed · 6 within ±1.5%
View detailed results and traces 🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head |
commit: |
A stale unstable_cache entry served during an App Router request scheduled its background refresh inside the triggering request's unified context. The detached callback could therefore mutate that request's tag, observation, and fetch containers after the response was already being finalized, and nested stale function-cache reads still saw the request's "background" policy, so a refreshed outer entry could be assembled from stale nested data. Run the refresh through the same isolated synthetic work unit that "use cache" refreshes use, whose foreground mode forces nested dependencies fresh before the regenerated entry is stored. Also rename cacheRevalidationMode to functionCacheRevalidationMode: patched fetch keeps its separate refreshStaleFetchesInForeground flag whose default is fail-open, so the field only governs the function caches and the broader name misrepresented that boundary. Converging both onto one request-level freshness policy is deliberate follow-up work, documented on the type.
NathanDrake2406
marked this pull request as ready for review
July 23, 2026 09:31
The functionCacheRevalidationMode comment promised the fetch-policy convergence as tracked follow-up without saying where. Point it at issue cloudflare#2685, which records the per-path audit and the Pages Router semantics decision that keeps the convergence out of this PR.
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 277d7885a1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ound Address two review findings on the "use cache" stale-while-revalidate path: - Propagate a served data-cache HIT entry's cache-control into the enclosing cache scope's lifeConfigs, not only the request store. On a hit the child function never runs, so this is the only place its (shorter) lifetime can constrain an outer "use cache" entry. Without it, an outer miss embedding a stale child stored the outer entry under the default 900s window while the child refreshed separately, serving stale nested data past the child's own expire. Mirrors the tag propagation already done on the hit path. - Seed foreground function-cache revalidation for prerender requests in createAppRscHandler. VINEXT_PRERENDER=1 requests bake a static artifact, so a stale persistent entry must be refreshed in the foreground rather than served stale and refreshed in the background. Matches the static-generation dispatch paths. Both are covered by regressions that fail against the prior behavior.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2672
Summary
use cachestale-while-revalidate semanticsrevalidatewindowunstable_cacheanduse cache"use cache"execution scopedraftMode().isEnabledwithout inheriting request datawaitUntilattachment have one ownerWhy
A cache value is usable only when both its stored state and its consumer allow it. An ordinary dynamic request may return a stale value immediately and refresh it in the background. A page or route-handler ISR regeneration must await that refresh so the new full-route artifact contains fresh data. An expired value is no longer usable in either context and must regenerate before the caller continues.
The request boundary owns this distinction because a cached function cannot know whether it is answering a user request or constructing a persistent artifact. Both function/data-cache APIs now consume the same decision:
cacheStateabsent)Draft mode is a separate cache boundary. Preview data can differ from published data, so a draft request executes the callback without reading or populating the persistent shared cache.
Non-draft background refreshes still need the request APIs supported inside public cache scopes.
draftMode().isEnabledmust returnfalse; it must not fail because the detached context omitted its provider. The refresh receives a disabled provider with empty header and cookie stores. The draft-mode secret is retained, but no request data is copied.What changed
use cachedata blocked on regenerationfunctionCacheRevalidationMode: "foreground""stale"was accepted as a hit, so the memory handler could return an expired value indefinitelyget,set, or background refreshdraftMode()isEnabled: falsefrom a minimal provider and completes the cache writeunstable_cacheanduse cacheowned separate pending maps and cleanup logicunstable_cachebackground refreshuse cache, with foreground nested readsStored tags and cache-control metadata are applied to a stale response only after successful deserialization. Refresh-time metadata stays in the isolated context.
Validation
unstable_cacheanduse cache. Both behavior tests failed against the previous implementation by returning the stored expired value.unstable_cacherefresh that reads another stale entry regenerates it in the foreground, stores the fully fresh outer result, and leaves the triggering request's tags and observations untouched. It fails when the refresh runs inside the triggering request's context.waitUntilownership, metadata isolation, disabled draft-mode access without request-data inheritance, draft-mode bypass, write-failure retry, missingwaitUntil, build-time foreground refresh, and runtime ISR foreground refresh.isDraftModeEnabled()shim export.tests/shims.test.ts(1,266 tests) locally and all 51 CI checks (unit shards, integration, and e2e suites) at the current head.Commands
Risk
Moderate. The behavior change is confined to persistent function/data-cache entries. Draft requests now match
unstable_cacheby bypassing persistent storage; request-local"use cache: private"remains available. Fresh published hits, misses, invalidated entries, the development bypass, and cache serialization retain their existing behavior. Unknown handler states fail closed by regenerating, so a custom handler that labels fresh values instead of leavingcacheStateabsent will lose a cache hit rather than serve a potentially unusable value.The request policy is scoped to the function caches and named
functionCacheRevalidationModeto make that boundary explicit; patchedfetch()retains its response-stream, refetch-map, and timeout lifecycle. Converging both onto one request-level freshness policy is tracked in #2685.A refresh that never settles remains deduplicated for the isolate lifetime. This preserves the existing
unstable_cachelifecycle behavior and does not introduce a new timeout policy.References
use cachestale-while-revalidate fixexpireas a miss and regenerates before use"use cache"entries while draft mode is enableduse cacheSWR testscacheLifedocumentationcacheState: "expired", which exposed the previous negative-state testuse cacheread pathunstable_cacheread path