cloudflare / cloudflare/vinext
App Router ISR stores a failed render as a status-200 cache entry when the error is thrown after the shell flushes
- Dominant language
- TypeScript
- Stars
- 8.8k
- Forks
- 406
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 120
Description
A Server Component error that is thrown *after* the HTML shell has flushed does not stop the App Router ISR cache write. The partially-rendered HTML and the error-carrying Flight payload are both stored under status 200, and later requests are served that entry as a normal cache HIT until the entry revalidates.
Next.js does the opposite: a render that populates the ISR cache runs through `prerenderToStream`, which rethrows the first captured RSC error, so the cache entry is never produced.
## Possible root cause
`createAppPageRscErrorTracker` (`packages/vinext/src/server/app-page-stream.ts:354-380`) splits what React reports through `onError` into two buckets: errors carrying a `digest` (`NEXT_REDIRECT`, `NEXT_NOT_FOUND`, …) go to `capturedSpecialError`, everything else to `capturedError`.
Only the first bucket is consulted after the shell resolves. `packages/vinext/src/server/app-page-render.ts:1032-1041` reads `getCapturedSpecialError()` and swaps in a 307/404. `getCapturedError()` has exactly one reader in the whole lifecycle — `renderErrorBoundaryResponse` at `app-page-render.ts:942`, which only runs on the shell-error recovery path added in #1908. Once the shell resolved, nothing looks at it again.
So the render falls through to the cache decision with no notion that it failed:
- `app-page-render.ts:1129-1140` computes `shouldSpeculativelyWriteCache` / `htmlResponsePolicy.shouldWriteToCache` from cacheability inputs only.
- `app-page-render.ts:1164-1187` builds both observations with a hardcoded `boundaryOutcome: { kind: "success" }`, `cacheability: "public"`, `completeness: "complete"`.
- `finalizeAppPageHtmlCacheResponse` (`packages/vinext/src/server/app-page-cache-finalizer.ts:122-215`) checks dynamic usage and cache policy, then calls `isrSet` with `buildAppPageCacheValue(cachedHtml, undefined, 200, …)` and `buildAppPageCacheValue("", rscData, 200, …)`.
The same gap exists on the RSC request path: `app-page-render.ts:881-923` hands off to `scheduleAppPageRscCacheWrite` (`app-page-cache-finalizer.ts:246-300`, `isrSet(… , 200, …)`) without reading the tracker at all.
The error tracker owns the knowledge that the render failed, the finalizers own the cache write, and nothing carries the first fact to the second. Pre-shell errors are covered — `htmlRender.shellErrorRecovered` at `app-page-render.ts:1111-1127` returns 500 with `NEVER_CACHE_CONTROL` — but that is the branch a `loading.tsx` prevents from being taken.
## Reaching it
Production build. An App Router page that is cache-write eligible — `export const revalidate = 60`, or the `revalidateSeconds === null` speculative write path — with a `loading.tsx` or route-level `Suspense` so the shell flushes before the page body settles, and no `error.tsx` between the throwing component and the root.
```
app/posts/[slug]/loading.tsx // shell flushes here
app/posts/[slug]/page.tsx // export const revalidate = 60
// await fetch(upstream) -> throws
```
The render must not touch `headers()`, `cookies()`, or `searchParams`; those mark dynamic usage and the finalizer skips the write. Route params do not mark dynamic usage, so the common triggers all still qualify: an upstream 5xx or timeout during a scheduled revalidation, a database blip, a `.json()` on a malformed body. A requester who can pick a route param that steers the render toward a failing upstream can aim it, but no attacker is needed for this to fire.
What gets stored:
- HTML key → the flushed shell plus React's client-side error trigger, status 200.
- RSC key → the Flight payload containing the serialized error, status 200.
Later requests get a HIT. The browser renders the loading fallback, then the client boundary takes over and shows the global error UI. The HTTP status is 200, so CDNs cache it and uptime monitoring reports the route healthy. It persists for the full `revalidate` window, and every revalidation attempt that hits the same failure re-poisons it.
## Next.js comparison
For an ISR request that will populate `ssgCacheKey`, Next sets `supportsDynamicResponse = false` (`build/templates/app-page-runtime.ts:557-605`), which makes `workStore.isStaticGeneration` true (`server/async-storage/work-store.ts:116-119`). `app-render.tsx:2686` then routes to `prerenderToStream`, and `app-render.tsx:2829-2839` rethrows:
```js
if (response.digestErrorsMap.size) {
const buildFailingError = response.digestErrorsMap.values().next().value
if (buildFailingError) throw buildFailingError
}
if (response.ssrErrors.length) { … }
```
The response generator throws, so no `CachedRouteKind.APP_PAGE` entry is built and the previous entry stays. `digestErrorsMap` is keyed by the digests `createErrorHandler` assigns to ordinary RSC errors, so this covers exactly the errors vinext currently ignores.
## Fix shape
`getCapturedError() !== null` after the shell should suppress the cache write on both paths, the same way dynamic usage already does — one check before `isrSet` in `finalizeAppPageHtmlCacheResponse` and `scheduleAppPageRscCacheWrite`, driven by a flag threaded from the tracker. Suppressing the write (serve the errored response, keep any previous entry) is closer to Next than writing a non-200 entry.
## Failing test sketch
Against `renderAppPageLifecycle`, in the style of the existing `tests/app-page-render.test.ts` special-error test at line 1435:
```ts
const common = createCommonOptions();
const boom = new Error("upstream failed"); // no digest -> ordinary RSC error
let capturedOnError: ((e: unknown, ...a: unknown[]) => void) | null = null;
await renderAppPageLifecycle({
...common.options,
isProduction: true,
hasLoadingBoundary: true,
revalidateSeconds: 60,
loadSsrHandler: async () => ({
async handleSsr(_rsc, _nav, _font, opts) {
capturedOnError?.(boom, null, null); // fires after the shell
opts?.capturedRscDataRef && (opts.capturedRscDataRef.value =
Promise.resolve(new TextEncoder().encode("flight").buffer));
return createStream(["shell"]);
},
}),
renderToReadableStream(_el, o) { capturedOnError = o.onError; return createStream(["flight"]); },
});
await Promise.all(common.waitUntilPromises);
expect(common.isrSet).not.toHaveBeenCalled();
```
Today `isrSet` is called twice, both with `status: 200`.
## Verified how
Read, on `main` at c9a4a843c:
- `getCapturedError` has two call sites repo-wide (`app-page-render.ts:942`, the tracker itself); neither is on the post-shell path.
- Every `isrSet` call site under `packages/vinext/src/server/` — `app-page-cache-finalizer.ts:180`, `:198`, `:290`, `app-page-cache.ts:450`, `:480` — passes a literal `200` and none reads the error tracker.
- `git log -S"shellErrorRecovered"` shows #1908 as the only commit introducing that path; it gates on shell errors only.
- PR #2731 changes cache-policy proving for dynamic usage. It does not touch render errors.
- `gh issue list` on cloudflare/vinext for cache/error/ISR terms returned nothing related.
- Next.js parity from the vendored `.nextjs-ref` at the file:line references above.
Not executed. I did not run a repro against a built app or run the test sketch, so the exact cached HTML bytes and the client-visible result are reasoned from the code path, not observed.
Unconfirmed, secondary: the build-time prerender path (`app-page-render.ts:1152` returns early with a 200 response, `build/prerender.ts:1460` stores `response.status`) looks like it would write a post-shell-errored page as a static 200 artifact as well, where Next fails the build. I did not trace whether React's `allReady` resolves or rejects for a recovered post-shell error under `waitForAllReady`, which decides this.
Contributor guide
Research direction
Start with createAppPageRscErrorTracker in packages/vinext/src/server/app-page-stream.ts, then trace the post-shell paths in app-page-render.ts and the cache writes in app-page-cache-finalizer.ts. Run the renderAppPageLifecycle test style from tests/app-page-render.test.ts and verify that an ordinary post-shell RSC error prevents ISR writes on both HTML and RSC paths while preserving the existing response behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- next.js, react, typescript
- Domain
- backend, performance, web-dev
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100