[pull] canary from vercel:canary#1242
Merged
Merged
Conversation
…ion (#96148) In dev, a render that uses a request-time API inside `'use cache'` records an invalid dynamic usage error on its work store. This is an error in the render the user received, not a validation insight, yet it was forwarded to the dev overlay from inside `runDevValidationInBackground`, entangled with the validation lifecycle, and a separate non-validating code path forwarded the same error a second way. This change moves the forwarding, together with the `cacheReady()` wait that finalizes the error's verdict, out to the render's completion handler, where it runs once the render has settled regardless of whether the route validates. Validation becomes a pure consumer of the settled render: `runDevValidationInBackground` no longer reads or forwards the error, takes the settled render result rather than its promise, and is simply not scheduled when the render recorded such an error, since there is then nothing worth validating. Deferring the scheduling until the render has settled costs nothing, because validation already waits for the response to finish before it does any work, by which point the render result is always available. Consolidating the forwarding into one place at the render level disentangles this main-render concern from validation and prepares the ground for the upstack refactor that remodels the render-result types without the validation task reaching into them.
…on (#96149) The settled output of a staged dev render and the inputs that dev Cache Components validation consumes were both modeled as extensions of a single shared interface that carried a nullable `syncInterruptReason: Error | null` alongside the render's accumulated chunks and stage timings. That flat shape let a sync-interrupted render and an uninterrupted one share one type even though their usable contents are disjoint, so every consumer had to read `syncInterruptReason` and treat the chunks as possibly present, and an interrupted validation input still carried a request store and debug channel that nothing downstream ever read. This change splits that shape into a proper discriminated union. `DevValidationInputs` becomes `ResolvedValidationInputs | SyncInterruptedStagedDevRender`, discriminated by the presence of `syncInterruptReason`: an uninterrupted render carries its artifacts, the request store, and the debug channel, while a sync-interrupted render carries only the interrupt reason (its debug channel is dropped at construction). A render's own settled output is a `StagedDevRenderResult` whose `outcome` is that same `StagedDevRenderArtifacts | SyncInterruptedStagedDevRender` union, with `hadCacheMiss` lifted onto the result rather than the outcome, because it is orthogonal to how the render ended (either kind can miss caches) and is consumed before the result is split into validation inputs. The render constructors drop the debug channel at the point of interruption, so `forwardErrorsFromWarmRender` no longer needs to, and the consumers that only ever handle a resolved render (`runValidationInDevImpl`, `validateStaticShell`, and the reuse path in `prepareValidationInputs`) take `ResolvedValidationInputs` directly. This is a behavior-preserving refactor.
This adds the `experimental.devValidationWorker` boolean option: its type in `config-shared.ts` and its schema entry in `config-schema.ts`. The flag is the configuration seam for running development Cache Components validation on a worker thread rather than the dev server's main thread, which keeps the event loop responsive during rapid navigation. It is enabled by default, and setting it to `false` runs validation in-process, as an escape hatch to isolate a problem or fall back if the worker misbehaves. We land the flag on its own, ahead of the preparatory refactor, the benchmark, and the worker implementation that follow in the stack, so those changes can reference and toggle it. On its own the option is recognized by config validation but is not yet read anywhere. Because the flag is consumed only server-side in `next dev`, it needs no `define-env.ts` wiring for user-bundled code, nor `renderOpts` plumbing.
) This is a behavior-preserving refactor of the dev-mode Cache Components validation, ahead of the change that moves it onto a worker thread. In-process behavior is unchanged. It narrows the context the validation functions require. `runValidationInDev` and its helpers (`validateStaticShell`, `warmupClientModulesForStagedValidation`, `validateStagedShell`, `validateInstantConfigs`) now take a `ValidationRenderContext` (a `Pick` of `AppRenderContext` plus the two `renderOpts` fields and the debug-channel flag they actually read) instead of the full render context, with `toValidationRenderContext` mapping the in-process and build-time callers. The worker cannot reconstruct a full `AppRenderContext` from a serialized snapshot, so narrowing the contract to what validation genuinely consumes is what lets the same code run there. It also separates computing the validation errors from delivering them. `runValidationInDev` now returns the errors instead of sending them to the dev overlay inline, and the caller `runDevValidationInBackground` delivers them via `logMessagesAndSendErrorsToBrowser`. The test-mode lifecycle markers and delay move into `runWithDevValidationLogging`, which brackets both the render and the delivery. When validation runs on the worker, the worker computes the errors but the main thread still delivers them (delivery needs the response object), so splitting the two halves is a prerequisite. Finally, it extracts the test-only validation lifecycle markers into a shared `dev-validation-events` module. The `<VALIDATION_MESSAGE>` wrapping and the event shape had been hand-rolled at each emission site in `app-render.tsx` and duplicated again in the test harness; they now live in one place as `formatValidationEvent` and an exported `ValidationEvent` union that the in-process emitters call and the test utilities import rather than redeclare. The worker emits the same markers through the same helper once it is added, so a single definition keeps every producer and the test parser in sync. Build validation's marker `requestId`, previously the numeric `Date.now()`, is stringified to fit the shared `string` type; it appears only in the logged marker payload, which nothing matches on, so this too is unobservable.
<img width="428" height="179" alt="Screenshot 2026-07-24 at 22 42 04" src="https://github.com/user-attachments/assets/38478940-d23e-4faa-bd88-5758190be158" /> [Flakiness metrics](https://app.datadoghq.com/ci/test/runs?query=test_level%3Atest%20%40git.repository.id%3A%22github.com%2Fvercel%2Fnext.js%22%20%40test.name%3A%22enabled%20features%20in%20trace%20should%20denormalize%20inherited%20enabled%20features%20during%20upload%22%20%40test.type%3A%22nextjs%22%20%40test.status%3A%28%22fail%22%20OR%20pass%29&agg_m=count&agg_m_source=base&agg_t=count&fromUser=true&index=citest&start=1784320894753&end=1784925694753&paused=false) The `render-path` span is recorded when a request's response closes, which is too late for any flush other than the one the dev server performs while shutting down. The parent `next dev` process escalates to SIGKILL 100ms after signalling the child, and on a machine running eight test files at once the child does not reliably get scheduled to run its cleanup within that window, so the span never reached the trace file and the upload assertions failed. This change raises the budget for the test through `NEXT_EXIT_TIMEOUT_MS`, which was added alongside that timeout in #67165 so that it can be increased when the child's exit work matters more than a fast exit. The same approach is already used in `test/e2e/filesystem-cache/warm-restart-task-stats.test.ts`, where the timeout would otherwise cut off a Rust `on_exit` handler before it writes its task statistics. Both test cases previously guarded their request with a check for the existence of the trace file, which the dev server creates on its own once the first compile finishes. When that happened before the first test body ran, neither case issued a request and the trace file contained no `compile-path` or `render-path` span at all. The request and the shutdown now happen once in `beforeAll`, and the fixed 500ms sleep that followed the shutdown is replaced by a `retry` that waits for the spans the assertions depend on. --- <sub>Stack created with <a href="https://github.com/github/gh-stack">GitHub Stacks CLI</a> • <a href="https://gh.io/stacks-feedback">Give Feedback 💬</a></sub>
#96152) This adds `bench/dev-validation/`, wired as `pnpm bench:dev-validation`, which measures how much dev-mode Cache Components validation contends for the dev server's event loop during rapid navigation, and how much running it on a worker thread relieves that. It toggles `experimental.devValidationWorker` (added in the previous commit) to A/B the two configurations on the same build. Until the worker implementation lands the flag is inert and the A/B shows no delta. The fixture generates one route per family (`client`, `server`, `sprite`), each nested several layout segments deep under a `(routes)` route group. Validation renders a combined payload at every URL depth, so a deeper route means more validation work per navigation, which mirrors a realistically deep app rather than a single flat segment. The runner clicks a family's `<Link>` repeatedly, since navigating to the current route re-renders and re-validates it on every click. The routes carry no `instant` config because dev validation applies to page segments by default at the warning level. The three families isolate the client prerender, the Flight re-encode plus owner-stack work, and the Flight payload size, respectively. The signal is browser-observed TTFB taken from Playwright's own network timing, because it includes the time a request waits for the event loop while validation monopolizes it. We deliberately do not use the CLI's logged request durations: the dev server starts that clock inside the request handler, after the loop has already yielded to the request, so the queue wait is invisible to it. The runner prints each configuration's absolute TTFB (p50/p95/max) side by side rather than a ratio. The time the worker frees is the validation render's CPU, which is bounded, route-dependent, and does no IO, so a ratio would overstate a win that does not scale with total request time. Because the clicks are back-to-back the numbers are a worst case — navigations that land inside the validation window — and the `max` tail is the honest headline: it is the main-thread stall the worker removes.
This moves the dev-mode Cache Components validation renders off the dev server's main thread onto a worker thread, so rapid navigation no longer starves the event loop. The worker crosses into the app-page bundle exactly once, calling a new `ComponentMod.routeModule.runValidationInDev` entry that rebuilds the render context, work store, and request store from a serializable snapshot and runs the whole validation there, so the client prerender and the user's client components resolve the single app-page React instance rather than a second copy. A thin worker shell (`dev-validation-worker.ts`) plus a single-worker pool (`dev-validation-worker-pool.ts`) load the user bundle via `loadComponents`, install code-frame support for CLI output, and forward the validation errors back as Flight bytes; the main thread only delivers them to the overlay through `sendErrorsToBrowser`. The snapshot, the globalThis-symbol handoff, and the shared error-delivery helpers live in their own modules. The dev server installs the worker when `experimental.devValidationWorker` is not `false`, and `runDevValidationInBackground` uses it when present, falling back to the in-process path otherwise. A one-slot `SharedArrayBuffer` propagates a supersede abort into the worker so a newer navigation cancels an in-flight validation. The runtime bundle gains an `app-worker` entry for the worker with the `build/swc` boundary externalized for the code-frame native binding, and `patch-error-inspect.ts` now backs its code-frame renderer with a globalThis symbol so all copies of the module share it across the thread. The synthetic `bench/dev-validation` benchmark navigates back-to-back, so every click lands inside the validation window — the worst case for main-thread contention. Browser-observed navigation TTFB in that window, worker vs in-process: | Route | Worker (p50/p95/max) | In-process (p50/p95/max) | | ------ | -------------------- | ------------------------ | | client | 19 / 24 / 27 ms | 40 / 66 / 7762 ms | | server | 42 / 45 / 46 ms | 122 / 158 / 252 ms | | sprite | 109 / 117 / 169 ms | 196 / 208 / 299 ms | The steady-state difference is only tens of milliseconds; the effect that matters is the tail. In-process, a navigation that collides with an in-flight validation render can stall for seconds (this run peaked at ~7.8s on the client route, and the peak varies run to run) because a staged render does not yield until it finishes; off-thread the main thread stays free and that stall disappears. Read these as an upper bound, not a speedup that generalizes. The work moved off-thread is the validation render's CPU — bounded, route-dependent, and free of IO — so it does not grow with the main render's cost. In an app whose main render is dominated by IO the same absolute saving is a small fraction of the request, and it only appears when a navigation lands in the brief validation window; at ordinary click speed it is largely invisible. This is a dev-only responsiveness improvement whose benefit varies widely with the app and the navigation pattern. As a follow-up, the validation work that still runs on the main thread could move to the worker as well. When the main render can't be reused for validation (for example after a cache miss), the validation re-renders on the main thread to produce its inputs, resuming from the Resume Data Cache (RDC) the main render already filled, and only then hands the resulting Flight chunks to the worker. A later iteration could run those renders inside the worker too, transporting the (serializable) RDC so the worker can resume from the filled caches rather than reading them back on the main thread. closes NAR-895
…96173) ## Summary Fixes #94919. When a client disconnects mid-response, the compression middleware's zlib stream is never released, and every aborted compressed response permanently retains its deflate state (~256 KiB). On a server taking traffic that aborts mid-stream (bots, CDNs, users navigating away) RSS climbs until the process is OOM-killed. `router-server.ts` applies the vendored `compression` middleware, which ends its zlib stream *only* from inside its own `res.end()` wrapper. On a client disconnect Next destroys the response instead, so that wrapper never runs and the stream stays open. An open zlib stream is pinned by its native handle, so it is not merely uncollected-for-now — it survives GC entirely. This is why the reported symptom looks unlike a normal JS leak: the JS heap stays nearly flat while RSS grows without bound, because the retained deflate state is native memory. It affects `next start`, `next dev`, and custom servers — anything going through the router server with the default `compress: true`. Deployments that let a proxy handle compression never see it. Note that ending the stream is not sufficient to recover it: its `data` handler writes to the already-destroyed response, `res.write()` returns `false`, the middleware pauses the stream, and it never reaches `end`. It has to be destroyed. `releaseCompressionStream` carries a comment explaining this so the cleanup isn't "simplified" back into a no-op later. The stream is reached through the one handle the middleware exposes: it forwards `res.on('drain', …)` to the zlib stream once that stream exists, and `EventEmitter#on` returns the emitter it was called on. Upstream `compression@1.8.1` still has no premature-close handling, so this can't be fixed by bumping the vendored copy. The added test asserts that assumption and will fail if a future vendored version handles this itself, signalling that the workaround can be dropped. ### Before / after Measured with a minimal custom Node server (no Express/axios) rendering a 1.25 MB dynamic App Router page, 8 rounds × 300 requests cancelled after the first chunk, `heapUsed`/`rss` sampled **after forced GC** each round. Same build, cleanup toggled behind a temporary env var: | | without cleanup | with cleanup | | --- | --- | --- | | zlib contexts never released | 2400 of 2400 (100%) | **0** of 2600 | | RSS | 98.2 → 1069.6 MiB, linear | 100 → 401.5 MiB, plateaus | | `external` | 3.6 → 76.5 MiB | 3.6 → 4.5 MiB | | `heapUsed` | 21.2 → 79.1 MiB | 21.2 → 27.4 MiB | Without the cleanup, RSS grows by a steady ~93 MiB per 300 aborted requests across rounds 2–8 (≈310 KiB per request, consistent with zlib's default deflate state plus per-stream buffers), and every unreleased stream is *still reachable after repeated forced GC* — permanent retention rather than GC lag. With the cleanup, all 2600 contexts are closed and RSS plateaus, with the residual difference from baseline being allocator retention rather than per-request growth. As a control, 200 fully-read responses issued into the same un-fixed process all closed normally, isolating mid-stream disconnects as the trigger. The issue also attributes the leak to a retained RSC element tree held via the `WeakMap` in `use-flight-response.tsx`. That part did not reproduce on canary: across 8000 aborted requests spanning dynamic renders, `redirect()`, `notFound()` and render throws, and 3000 full reads of the same 1.25 MB payload, post-GC heap stayed flat at ~29 MiB. The native `zlib_memory` line in the reporter's heap-snapshot diff was the actual cause. ## Verification - `pnpm test-start-turbo test/e2e/custom-server/custom-server.test.ts` and `pnpm test-dev-turbo test/e2e/custom-server/custom-server.test.ts` — covers `Content-Encoding: gzip` through a custom server, i.e. the code path being changed (46 and 45 passing) - `npx jest packages/next/src/server/lib` — 188 passing, including the 4 new cases - Manual: verified a gzipped response still decompresses byte-identical to the same response with `accept-encoding: identity` (1,250,721 bytes), with no errors or `MaxListenersExceededWarning` under sustained load - Manual: the before/after table above, re-measured against this branch's HEAD <!-- NEXT_JS_LLM --> Co-authored-by: Pete Hunt <239742+petehunt@users.noreply.github.com>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )