From f986e65a1f4669685e275ac2aa2eec096d8bfd86 Mon Sep 17 00:00:00 2001 From: Divanshu Chauhan Date: Sun, 20 Sep 2026 00:25:20 -0500 Subject: [PATCH] fix(cache): propagate deduplicated nested "use cache" metadata before writes settle (#3321) An inner `"use cache"` invocation now publishes its value, tags, lifetime, and root-param dependencies to the enclosing request as soon as collection completes. Its handler write is handed to the request lifecycle (`waitUntil` plus the pending cache-work queue that action and route-handler boundaries drain) instead of being awaited on the value path, and a repeat lookup for the same key is served from a request-scoped retained record until that write settles. Two outer cache scopes that share an inner cache therefore inherit the inner metadata while persistence is still in flight, instead of re-executing the inner function and keying themselves without the root params it read. The retained record is dropped once the write settles, and cross-request reads are unchanged: a later request still reads the handler and follows the root-param redirect marker to rebuild a root-param-specific key, so an entry produced for one root-param value is never reused for another. Upstream: vercel/next.js#98808. --- .../vinext/src/shims/cache-request-state.ts | 82 ++++++ packages/vinext/src/shims/cache-runtime.ts | 243 +++++++++++++----- packages/vinext/src/shims/cache.ts | 13 +- .../src/shims/unified-request-context.ts | 1 + tests/use-cache-root-params.test.ts | 149 +++++++++++ 5 files changed, 408 insertions(+), 80 deletions(-) diff --git a/packages/vinext/src/shims/cache-request-state.ts b/packages/vinext/src/shims/cache-request-state.ts index 7ebe2c2158..052881df3c 100644 --- a/packages/vinext/src/shims/cache-request-state.ts +++ b/packages/vinext/src/shims/cache-request-state.ts @@ -1,5 +1,7 @@ import { getHeadersAccessPhase } from "./headers.js"; import { getOrCreateAls } from "./internal/als-registry.js"; +import { getRequestExecutionContext } from "./request-context.js"; +import type { CacheControlMetadata } from "./cache-handler.js"; import { getRequestContext, isInsideUnifiedScope, @@ -60,11 +62,26 @@ export type UnstableCacheObservation = Readonly<{ tagHash: string | null; }>; +/** + * A shared `"use cache"` invocation this request already collected, keyed by the + * key its value was stored under. Retained only while the invocation's handler + * write is still settling, so a repeat lookup can reuse the collected value and + * inherit its metadata without waiting for persistence (issue #3321). + */ +export type RetainedUseCacheInvocation = { + /** The value the producing invocation returned to its caller. */ + result: unknown; + tags: readonly string[]; + cacheControl: CacheControlMetadata; + rootParamNames: ReadonlySet | undefined; +}; + export type CacheState = { actionRevalidationKind: ActionRevalidationKind; pendingRevalidatedTags: Set; pendingRevalidations: Set>; requestScopedCacheLife: CacheLifeConfig | null; + retainedUseCacheInvocations: Map; unstableCacheObservations: Map; unstableCacheRevalidation: UnstableCacheRevalidationMode; }; @@ -82,6 +99,7 @@ const fallbackState = (globalState[FALLBACK_KEY] ??= { pendingRevalidatedTags: new Set(), pendingRevalidations: new Set>(), requestScopedCacheLife: null, + retainedUseCacheInvocations: new Map(), unstableCacheObservations: new Map(), unstableCacheRevalidation: "foreground", } satisfies CacheState) as CacheState; @@ -109,6 +127,7 @@ export function _runWithCacheState(fn: () => T | Promise): T | Promise pendingRevalidatedTags: new Set(), pendingRevalidations: new Set>(), requestScopedCacheLife: null, + retainedUseCacheInvocations: new Map(), unstableCacheObservations: new Map(), unstableCacheRevalidation: "foreground", }; @@ -205,6 +224,69 @@ export async function _drainPendingRevalidations(): Promise { if (didReject) throw firstRejection; } +/** + * Hand cache work to the request lifecycle instead of the caller. + * + * Runtimes with an ExecutionContext keep the isolate alive through + * `waitUntil`, and request boundaries await queued work before the response is + * finalized. Returns false when no lifecycle owns the work, so the caller + * decides how to settle it. + * + * @internal + */ +export function _scheduleRequestScopedCacheWork(promise: Promise): boolean { + const executionContext = getRequestExecutionContext(); + const queued = _queuePendingRevalidation(promise); + if (executionContext) { + executionContext.waitUntil(promise); + } else if (!queued) { + void promise.catch((error) => { + console.error("[vinext] cache revalidation failed:", error); + }); + } + return queued || executionContext !== null; +} + +/** + * Retain a just-collected `"use cache"` invocation for the rest of the request, + * keyed by the key its value is stored under. + * + * The record bridges the window between collection and the handler write + * settling: a repeat lookup in the same request reads it instead of re-executing + * the cached function. A no-op outside a request scope — there, the write is + * awaited before the value is returned, so no window exists. + * + * @internal + */ +export function _retainUseCacheInvocation( + key: string, + invocation: RetainedUseCacheInvocation, +): void { + if (!hasRequestScopedCacheState()) return; + getCacheState().retainedUseCacheInvocations.set(key, invocation); +} + +/** + * Read the retained `"use cache"` invocation stored under `key`, if any. + * + * @internal + */ +export function _readRetainedUseCacheInvocation(key: string): RetainedUseCacheInvocation | null { + if (!hasRequestScopedCacheState()) return null; + return getCacheState().retainedUseCacheInvocations.get(key) ?? null; +} + +/** + * Drop a retained invocation once its handler write has settled. Later lookups + * then read the entry from the handler like any other request. + * + * @internal + */ +export function _releaseRetainedUseCacheInvocation(key: string): void { + if (!hasRequestScopedCacheState()) return; + getCacheState().retainedUseCacheInvocations.delete(key); +} + export function _setRequestScopedCacheLife(config: CacheLifeConfig): void { const state = getCacheState(); if (state.requestScopedCacheLife === null) { diff --git a/packages/vinext/src/shims/cache-runtime.ts b/packages/vinext/src/shims/cache-runtime.ts index c887b4f704..64c9b3aab1 100644 --- a/packages/vinext/src/shims/cache-runtime.ts +++ b/packages/vinext/src/shims/cache-runtime.ts @@ -30,15 +30,20 @@ import { getDataCacheHandler, - type CachedFetchValue, type CacheControlMetadata, + type CacheHandler, type CacheHandlerValue, + type CachedFetchValue, } from "./cache-handler.js"; import { cacheLifeProfiles, _hasPendingRevalidatedTag, - _setRequestScopedCacheLife, + _readRetainedUseCacheInvocation, _registerCacheContextAccessor, + _releaseRetainedUseCacheInvocation, + _retainUseCacheInvocation, + _scheduleRequestScopedCacheWork, + _setRequestScopedCacheLife, type CacheLifeConfig, } from "./cache-request-state.js"; import { VINEXT_RSC_MARKER_HEADER } from "../server/headers.js"; @@ -746,6 +751,14 @@ export function registerCachedFunction( // by the handler's own exception. let existing: CacheHandlerValue | null = null; if (!_hasPendingRevalidatedTag(softTags)) { + // An earlier invocation in this request may have collected this entry + // while its handler write is still settling. Serve that value and its + // metadata: a lookup that waited for the write would instead re-execute + // the function and inherit nothing (issue #3321). Retained records live + // only for this request and only until the write settles, so this cannot + // serve another request's data. + const retained = serveRetainedUseCacheInvocation(cacheKey, softTags); + if (retained !== null) return retained.result as TResult; try { existing = await handler.get(cacheKey, { kind: "FETCH", softTags }); } catch (error) { @@ -829,76 +842,87 @@ export function registerCachedFunction( // Serialization ran while the cache ALS was active so lazy Server // Component work is reflected in `ctx` before selecting the final key. if (collectedResult?.cacheEntry) { - try { - let cacheFunctionInvocation: VinextCacheFunctionInvocation | undefined; - if (options.serverReferenceId && options.encodeInvocationArgs) { - try { - cacheFunctionInvocation = { - encryptedArgs: await options.encodeInvocationArgs(admittedArgs), - referenceId: options.serverReferenceId, - rootParams: Object.fromEntries( - Object.entries(rootParams ?? {}).filter((entry) => entry[1] !== undefined), - ) as Record, - softTags, - }; - } catch { - // Some request-local values cannot be replayed after this render. - } + let cacheFunctionInvocation: VinextCacheFunctionInvocation | undefined; + if (options.serverReferenceId && options.encodeInvocationArgs) { + try { + cacheFunctionInvocation = { + encryptedArgs: await options.encodeInvocationArgs(admittedArgs), + referenceId: options.serverReferenceId, + rootParams: Object.fromEntries( + Object.entries(rootParams ?? {}).filter((entry) => entry[1] !== undefined), + ) as Record, + softTags, + }; + } catch { + // Some request-local values cannot be replayed after this render. } - const serialized = collectedResult.cacheEntry; - const cacheValue = { - kind: "FETCH", - data: { - headers: serialized.headers, - body: serialized.body, - url: cacheKey, - }, - tags: ctx.tags, - revalidate: revalidateSeconds, - } satisfies CachedFetchValue; - const cacheContext = { - fetchCache: true, + } + const serialized = collectedResult.cacheEntry; + const cacheValue = { + kind: "FETCH", + data: { + headers: serialized.headers, + body: serialized.body, + url: cacheKey, + }, + tags: ctx.tags, + revalidate: revalidateSeconds, + } satisfies CachedFetchValue; + const cacheControl: CacheControlMetadata = { + revalidate: revalidateSeconds, + expire: effectiveLife.expire, + // Persisted so a later hit re-registers the same claim; otherwise + // the enclosing render's minimum depends on cache temperature. + stale: effectiveLife.stale, + }; + const cacheContext = { + fetchCache: true, + tags: ctx.tags, + ...(cacheFunctionInvocation ? { cacheFunctionInvocation } : {}), + cacheControl, + }; + const rootParamSpecificKey = + rootParamNames && rootParamNames.size > 0 && rootParams + ? coarseCacheKey + computeRootParamsCacheKeySuffix(rootParams, rootParamNames) + : null; + // The key a repeat lookup in this request recomputes, and therefore the + // key the retained record must use. + const storedKey = rootParamSpecificKey ?? cacheKey; + // `revalidate: 0` entries are dynamic by construction: the read path + // treats them as stale and re-executes, so retention must not resurrect + // them. + const retainable = revalidateSeconds > 0; + + const cacheWrite = persistUseCacheEntry({ + handler, + coarseCacheKey, + cacheKey, + rootParamSpecificKey, + rootParamNames, + revalidateSeconds, + cacheValue, + cacheContext, + tags: ctx.tags, + }); + if (retainable) { + _retainUseCacheInvocation(storedKey, { + result: collectedResult.result, tags: ctx.tags, - ...(cacheFunctionInvocation ? { cacheFunctionInvocation } : {}), - cacheControl: { - revalidate: revalidateSeconds, - expire: effectiveLife.expire, - // Persisted so a later hit re-registers the same claim; otherwise - // the enclosing render's minimum depends on cache temperature. - stale: effectiveLife.stale, - }, - }; - - if (rootParamNames && rootParamNames.size > 0 && rootParams) { - const specificCacheKey = - coarseCacheKey + computeRootParamsCacheKeySuffix(rootParams, rootParamNames); - const redirectTags = [ - ...ctx.tags, - ...[...rootParamNames].map((name) => ROOT_PARAM_TAG_PREFIX + name), - ]; - await handler.set( - coarseCacheKey, - { - kind: "FETCH", - data: { - headers: { [ROOT_PARAM_REDIRECT_HEADER]: "1" }, - body: "", - url: coarseCacheKey, - }, - tags: redirectTags, - revalidate: revalidateSeconds, - }, - { ...cacheContext, tags: redirectTags }, - ); - // Write the useful entry last. A bounded LRU that can retain only - // one of the pair must keep the specific value, not the redirect. - cacheValue.data.url = specificCacheKey; - await handler.set(specificCacheKey, cacheValue, cacheContext); - } else { - await handler.set(cacheKey, cacheValue, cacheContext); + cacheControl, + rootParamNames, + }); + } + if (_scheduleRequestScopedCacheWork(cacheWrite)) { + // The value and the metadata above are already published; only + // persistence is still outstanding, so nothing on the render path may + // wait for it. The retained record is dropped once the write settles, + // after which lookups read the entry from the handler again. + if (retainable) { + void cacheWrite.finally(() => _releaseRetainedUseCacheInvocation(storedKey)); } - } catch { - // A handler failure skips caching but must not fail the render. + } else { + await cacheWrite; + if (retainable) _releaseRetainedUseCacheInvocation(storedKey); } } @@ -1027,6 +1051,87 @@ function propagateCacheTagsToRequest(tags: readonly string[] | undefined): void addCollectedRequestTags(tags); } +/** + * Serve a lookup from a `"use cache"` invocation this request already collected + * but whose handler write is still settling. + * + * The collected value is returned with the same metadata the data-cache HIT path + * propagates: the invocation's cache control (which also feeds the enclosing + * scope's minimum-wins lifetime), its tags, and the root params it read. That is + * what lets an outer cache scope key and describe itself correctly without + * waiting for persistence (issue #3321). + * + * Returns null when nothing is retained for the key, mirroring a cache miss. + */ +function serveRetainedUseCacheInvocation( + cacheKey: string, + softTags: readonly string[], +): { result: unknown } | null { + const retained = _readRetainedUseCacheInvocation(cacheKey); + if (retained === null) return null; + if (_hasPendingRevalidatedTag([...retained.tags, ...softTags])) return null; + recordRequestScopedCacheControl(retained.cacheControl); + propagateCacheTagsToRequest(retained.tags); + propagateRootParamNamesToParent(retained.rootParamNames); + return { result: retained.result }; +} + +/** + * Persist a collected `"use cache"` value. + * + * Never rejects: a handler failure (a transient KV error, or a key the store + * rejects) skips caching but must not fail the render. + * + * When the invocation read root params, the coarse key receives a redirect entry + * naming those params so a reader can rebuild the specific key, and the value + * goes to the specific key. The value is written last so a bounded LRU that can + * retain only one of the pair keeps the value, not the redirect. + */ +async function persistUseCacheEntry(options: { + handler: CacheHandler; + coarseCacheKey: string; + cacheKey: string; + rootParamSpecificKey: string | null; + rootParamNames: ReadonlySet | undefined; + revalidateSeconds: number; + cacheValue: CachedFetchValue; + cacheContext: Record; + tags: readonly string[]; +}): Promise { + try { + if (options.rootParamSpecificKey === null) { + await options.handler.set(options.cacheKey, options.cacheValue, options.cacheContext); + return; + } + const redirectTags = [ + ...options.tags, + ...[...(options.rootParamNames ?? [])].map((name) => ROOT_PARAM_TAG_PREFIX + name), + ]; + await options.handler.set( + options.coarseCacheKey, + { + kind: "FETCH", + data: { + headers: { [ROOT_PARAM_REDIRECT_HEADER]: "1" }, + body: "", + url: options.coarseCacheKey, + }, + tags: redirectTags, + revalidate: options.revalidateSeconds, + } satisfies CachedFetchValue, + { ...options.cacheContext, tags: redirectTags }, + ); + options.cacheValue.data.url = options.rootParamSpecificKey; + await options.handler.set( + options.rootParamSpecificKey, + options.cacheValue, + options.cacheContext, + ); + } catch { + // A handler failure skips caching but must not fail the render. + } +} + // --------------------------------------------------------------------------- // Helper: execute function within cache context // --------------------------------------------------------------------------- diff --git a/packages/vinext/src/shims/cache.ts b/packages/vinext/src/shims/cache.ts index 9d8cd4926b..38e7fadf6b 100644 --- a/packages/vinext/src/shims/cache.ts +++ b/packages/vinext/src/shims/cache.ts @@ -31,7 +31,6 @@ import { makeHangingPromise } from "./internal/make-hanging-promise.js"; import { encodeCacheTag, encodeCacheTags } from "../utils/encode-cache-tag.js"; import { getCdnCacheAdapter } from "./cdn-cache.js"; import { getDataCacheHandler, type CachedFetchValue } from "./cache-handler.js"; -import { getRequestExecutionContext } from "./request-context.js"; import { isStagedCacheabilityProbeActive } from "./cacheability-classification.js"; import { addCollectedRequestTags, getCurrentFetchSoftTags } from "./fetch-cache.js"; import { @@ -39,7 +38,7 @@ import { ACTION_DID_REVALIDATE_STATIC_AND_DYNAMIC, _hasPendingRevalidatedTag, _markPendingRevalidatedTag, - _queuePendingRevalidation, + _scheduleRequestScopedCacheWork, _setRequestScopedCacheLife, cacheLifeProfiles, getRegisteredCacheContext, @@ -66,15 +65,7 @@ export type { ExecutionContextLike } from "./request-context.js"; export { runWithExecutionContext, getRequestExecutionContext } from "./request-context.js"; function scheduleRevalidation(promise: Promise): undefined { - const executionContext = getRequestExecutionContext(); - const queued = _queuePendingRevalidation(promise); - if (executionContext) { - executionContext.waitUntil(promise); - } else if (!queued) { - void promise.catch((error) => { - console.error("[vinext] cache revalidation failed:", error); - }); - } + _scheduleRequestScopedCacheWork(promise); return undefined; } diff --git a/packages/vinext/src/shims/unified-request-context.ts b/packages/vinext/src/shims/unified-request-context.ts index 52a950a55e..3d511f1e7f 100644 --- a/packages/vinext/src/shims/unified-request-context.ts +++ b/packages/vinext/src/shims/unified-request-context.ts @@ -112,6 +112,7 @@ export function createRequestContext(opts?: Partial): Uni serverContext: null, serverInsertedHTMLCallbacks: [], requestScopedCacheLife: null, + retainedUseCacheInvocations: new Map(), unstableCacheObservations: new Map(), unstableCacheRevalidation: "foreground", _privateCache: null, diff --git a/tests/use-cache-root-params.test.ts b/tests/use-cache-root-params.test.ts index 7b21a87b42..0a3db07046 100644 --- a/tests/use-cache-root-params.test.ts +++ b/tests/use-cache-root-params.test.ts @@ -1,5 +1,6 @@ import { createElement } from "react"; import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { IncrementalCacheValue } from "../packages/vinext/src/shims/cache-handler.js"; vi.mock("@vitejs/plugin-rsc/react/rsc", () => { const encoder = new TextEncoder(); @@ -121,3 +122,151 @@ describe('"use cache" root-param entry generation', () => { expect(calls).toBe(1); }); }); + +function deferred() { + let resolve!: (value: T) => void; + const promise = new Promise((r) => { + resolve = r; + }); + return { promise, resolve }; +} + +function entryTags( + data: IncrementalCacheValue | null, + ctx: Record | undefined, +): string[] { + const fromData = data && "tags" in data && Array.isArray(data.tags) ? data.tags : []; + const fromCtx = Array.isArray(ctx?.tags) ? (ctx.tags as string[]) : []; + return [...(fromData as string[]), ...fromCtx]; +} + +describe('"use cache" nested invocation propagation', () => { + beforeEach(async () => { + const { setCacheHandler, MemoryCacheHandler } = + await import("../packages/vinext/src/shims/cache.js"); + setCacheHandler(new MemoryCacheHandler()); + const knownRootParams = Reflect.get( + globalThis, + Symbol.for("vinext.cacheRuntime.knownRootParamsByFunctionId"), + ) as Map> | undefined; + knownRootParams?.clear(); + }); + + // Ported from Next.js: test/e2e/app-dir/app-root-params-getters/use-cache.test.ts + // (the `use-cache-dedup` fixture) + // https://github.com/vercel/next.js/blob/canary/test/e2e/app-dir/app-root-params-getters/use-cache.test.ts + // + // The fixture's cache handler holds the English inner write until both outer + // entries reach `set()`. Two outer caches share one inner cache, and the inner + // must hand its root-param dependency to both outer scopes as soon as it has + // collected — a render that waited for the handler write instead would either + // stall or key `outerTwo` without `lang`, letting the French request reuse the + // English entry. Failing this test without the fix shows up as a timeout, + // because the render never finishes while the inner write is held. + it("propagates a nested invocation's root params while its write is pending", async () => { + const { setCacheHandler, MemoryCacheHandler, cacheTag } = + await import("../packages/vinext/src/shims/cache.js"); + const { registerCachedFunction } = + await import("../packages/vinext/src/shims/cache-runtime.js"); + const { getRootParam, runWithRootParamsScope } = + await import("../packages/vinext/src/shims/root-params.js"); + const { createRequestContext, runWithRequestContext } = + await import("../packages/vinext/src/shims/unified-request-context.js"); + const knownRootParams = Reflect.get( + globalThis, + Symbol.for("vinext.cacheRuntime.knownRootParamsByFunctionId"), + ) as Map>; + + const observedOuters = new Set(); + const writtenKeys: string[] = []; + const innerWriteHeld = deferred(); + const bothOuterWritesSeen = deferred(); + + class DelayingHandler extends MemoryCacheHandler { + override async set( + key: string, + data: IncrementalCacheValue | null, + ctx?: Record, + ): Promise { + const tags = entryTags(data, ctx); + writtenKeys.push(key); + const isRedirect = tags.some((tag) => tag.startsWith("__vinext_use_cache_root_param__:")); + const outer = tags.find((tag) => tag === "nested-outer-one" || tag === "nested-outer-two"); + if (outer !== undefined && !isRedirect) { + observedOuters.add(outer); + if (observedOuters.size === 2) bothOuterWritesSeen.resolve(); + } + // Hold only the English inner value: a French request calls one outer. + if (tags.includes("nested-language-en") && outer === undefined && !isRedirect) { + await innerWriteHeld.promise; + } + return super.set(key, data, ctx); + } + } + setCacheHandler(new DelayingHandler()); + + let innerCalls = 0; + const inner = registerCachedFunction(async () => { + innerCalls++; + const language = await getRootParam("lang"); + cacheTag("nested-inner", `nested-language-${String(language)}`); + return language; + }, "test:nested-propagation-inner"); + const outerOne = registerCachedFunction(async () => { + cacheTag("nested-outer-one"); + return inner(); + }, "test:nested-propagation-outer-one"); + const outerTwo = registerCachedFunction(async () => { + cacheTag("nested-outer-two"); + return inner(); + }, "test:nested-propagation-outer-two"); + + const render = (lang: string, prime: boolean) => { + const pendingWrites: Promise[] = []; + return { + pendingWrites, + result: runWithRequestContext( + createRequestContext({ + executionContext: { + waitUntil(promise: Promise) { + pendingWrites.push(promise); + }, + passThroughOnException() {}, + }, + }), + () => + runWithRootParamsScope({ lang }, async () => { + const first = prime ? await outerOne() : null; + return { first, second: await outerTwo() }; + }), + ), + }; + }; + + const english = render("en", true); + await expect(english.result).resolves.toEqual({ first: "en", second: "en" }); + // The second outer inherited the collected value instead of re-running the + // inner cache while its write was still held. + expect(innerCalls).toBe(1); + + // Both outer entries reached the handler before the inner write was released: + // propagation happens at collection, not at persistence. + await bothOuterWritesSeen.promise; + expect(observedOuters.size).toBe(2); + innerWriteHeld.resolve(); + await Promise.all(english.pendingWrites); + // Each outer cache stored its value under the root params its inner cache + // read, so a request with other root params cannot read it. + const outerValueKeys = writtenKeys.filter( + (key) => + (key.includes("outer-one") || key.includes("outer-two")) && key.includes("root-params"), + ); + expect(outerValueKeys).toHaveLength(2); + + // A later request with different root params must not reuse the English entry. + knownRootParams.clear(); + const french = render("fr", false); + await expect(french.result).resolves.toEqual({ first: null, second: "fr" }); + await Promise.all(french.pendingWrites); + }, 5000); +});