Fix: Optimistic routing bugs leading to repeated prefetch loops - #97128
Conversation
Stats from current PR🔴 1 regression, 1 improvement
📊 All Metrics📖 Metrics GlossaryDev Server Metrics:
Build Metrics:
Change Thresholds:
⚡ Dev Server
📦 Dev Server (Webpack) (Legacy)📦 Dev Server (Webpack)
⚡ Production Builds
📦 Production Builds (Webpack) (Legacy)📦 Production Builds (Webpack)
📦 Bundle SizesBundle Sizes⚡ TurbopackClient Main Bundles
Server Middleware
Build DetailsBuild Manifests
Build Cache
📦 WebpackClient Main Bundles
Polyfills
Pages
Server Edge SSR
Middleware
Build DetailsBuild Manifests
Build Cache
🔄 Shared (bundler-independent)Runtimes
📝 Changed Files (33 files)Files with changes:
View diffsapp-page-exp..ntime.dev.jsfailed to diffapp-page-exp..time.prod.jsfailed to diffapp-page-tur..ntime.dev.jsfailed to diffapp-page-tur..time.prod.jsfailed to diffapp-page-tur..ntime.dev.jsfailed to diffapp-page-tur..time.prod.jsfailed to diffapp-page.runtime.dev.jsfailed to diffapp-page.runtime.prod.jsfailed to diffapp-route-ex..ntime.dev.jsDiff too large to display app-route-ex..time.prod.jsDiff too large to display app-route-tu..ntime.dev.jsDiff too large to display app-route-tu..time.prod.jsDiff too large to display app-route-tu..ntime.dev.jsDiff too large to display app-route-tu..time.prod.jsDiff too large to display app-route.runtime.dev.jsDiff too large to display app-route.ru..time.prod.jsDiff too large to display dev-validati..ntime.dev.jsDiff too large to display dev-validati..ntime.dev.jsDiff too large to display dev-validati..ntime.dev.jsDiff too large to display dev-validati..ntime.dev.jsDiff too large to display pages-api-tu..ntime.dev.jsDiff too large to display pages-api-tu..time.prod.jsDiff too large to display pages-api.runtime.dev.jsDiff too large to display pages-api.ru..time.prod.jsDiff too large to display pages-turbo...ntime.dev.jsDiff too large to display pages-turbo...time.prod.jsDiff too large to display pages.runtime.dev.jsDiff too large to display pages.runtime.prod.jsDiff too large to display server.runtime.prod.jsDiff too large to display use-cache-pr..ntime.dev.jsDiff too large to display use-cache-pr..ntime.dev.jsDiff too large to display use-cache-pr..ntime.dev.jsDiff too large to display use-cache-pr..ntime.dev.jsDiff too large to display 📎 Tarball URLCommit: b109b08 |
Tests PassedCommit: b109b08 |
|
We just hit this in production 😨 |
313b896 to
2206254
Compare
When a proxy rewrites every request to inject a leading path segment (what i18n libraries do when the default locale is hidden from the URL, e.g. next-intl's `localePrefix: 'as-needed'`), the client learns the route pattern from the rendered pathname reported by `x-nextjs-rewritten-path` — so it learns `/[locale]/[...pages]` for a URL the browser only ever saw as `/one/two`. Predicting the route for a sibling URL then matches the URL parts against that pattern without accounting for the injected segment, binding `locale` to `one` and `pages` to `['two']`. The server renders `locale=en` and `pages=['one','two']`, so it can never return the segments the request asked for, the prefetch task never settles, and it retries in a tight loop — hundreds of requests per second, starving the other links in the queue. Reproduced from MarkBekooy/prefetching-request-waterfall-bug#1 and reduced to plain Next (no next-intl). Fails in start mode, passes in dev.
When a proxy injects a leading path segment (`/one/two` → `/en/one/two`, what i18n libraries do when the default locale is hidden from the URL) and the target route is entirely dynamic (`/[locale]/[...pages]`), optimistic routing learned a pattern indexed by the requested URL's parts but shaped by the rendered pathname. Every prediction made from that pattern bound the wrong params, the server could never satisfy the resulting prefetch, and nothing on the prefetch path ever marked the pattern as bad — so the task re-derived the same prediction and spun at ~140 requests per second, starving every other prefetch in the queue. On the learning side, discoverKnownRoutePart now checks the response for evidence of a rewrite before storing a pattern. A route tree's dynamic segments carry param cache keys parsed from the rendered pathname, so each dynamic segment's cache key is compared against the (canonicalized) URL part(s) it would consume. A mismatch means the URL and the tree describe different paths, so no pattern is stored — the same bail static segments already perform. This covers every learn site (initial load, route tree prefetch, unprefetched navigations) and is a no-op for non-rewritten routes, where the cache key equals the URL part by construction. Learning can't detect a rewrite that preserves the route's shape but only applies to some param values (e.g. `/products/promo/[id]` → `/products/sale/[id]` where `/products/[category]/[id]` exists). So the transport decoder now reports divergence as part of the same traversal that decodes the response: while overlaying the server patch onto the base tree, decodeTransportNode compares each rendered segment's identity against the base tree's at the same position (inactive parallel route branches are expected to differ and are excluded) and records the result on the NavigationSeed as treeDivergedFromBase. During a prefetch the base is the request tree derived from the cached route entry, so divergence means the entry doesn't describe what the server actually renders. When that happens, fetchSegmentPrefetchesUsingDynamicRequest marks the entry as a dynamic rewrite and invalidates the entries derived from it — the same response dispatchRetryDueToTreeMismatch already performs on the navigation path. An entry constructed by route prediction doubles as the stored pattern, so marking it disables the pattern that produced the misprediction: one bad request instead of an unbounded loop. Bounding prefetch retries in general (a counter on the task object) is left as a TODO next to the new check. The repro added in the previous commit now passes in start mode. A new optimistic-routing test covers the value-dependent rewrite case; it fails with the divergence check reverted.
2206254 to
b41b315
Compare
The same class of loop occurs without any rewrite. When two parallel routes define dynamic children with different param names or types at the same level (`@modal/[...catchAll]` alongside `[username]`), route discovery reused the same trie node for both branches, so one branch's route entry was stored under the other's param pattern. Any URL that matched the poisoned pattern predicted the wrong route, diverged from the server response on every prefetch, and looped. The misprediction was head-only — a metadata-only request tree — which is exactly the case the treeDivergedFromBase backstop skips. Now, when discovery encounters a dynamic child whose param name or type disagrees with the one already stored, it marks the trie node with a sticky hasConflictingDynamicChildren flag and bails the same way it does for a rewrite mismatch. Matching refuses the dynamic subtree at a flagged level, falling back to a regular dynamic navigation. The better long-term fix — sending conflicting sibling dynamic params in the route tree like static siblings and matching both — is left as a TODO. The new e2e suite renders a modal catch-all alongside a dynamic sibling; on canary it fails with a sustained stream of metadata-only prefetches for a single URL. Fixes #97135
b41b315 to
b109b08
Compare
Next.js asks that reproductions be verified against canary before being reported, since the issue may already be fixed there. It isn't: the loop reproduces unchanged on 16.3.1-canary.11. Re-verified every row of the surface area table against canary rather than carrying over the 16.3.0 results. All rows hold, including that `export const instant = false` still does not stop the loop. Also links vercel/next.js#97128, which adds the missing divergence check for rewrite-affected prefetches and names this scenario explicitly. It is unmerged, which matches canary still failing here. `pnpm-workspace.yaml` is required for `pnpm install` to accept the canary packages under pnpm 11's minimum release age policy. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BKPaC9zccuP3EQsZidfXt9
| // immediately expired. The next prefetch will re-fetch the tree with | ||
| // correct hints from the /_tree response. | ||
| const acc = { metadataVaryPath: null } | ||
| const acc = { metadataVaryPath: null, treeDivergedFromBase: false } |
There was a problem hiding this comment.
nit:
| const acc = { metadataVaryPath: null, treeDivergedFromBase: false } | |
| const acc: RouteTreeAccumulator = { | |
| metadataVaryPath: null, | |
| treeDivergedFromBase: false, | |
| } |
| } | ||
|
|
||
| const acc = { metadataVaryPath: null } | ||
| const acc = { metadataVaryPath: null, treeDivergedFromBase: false } |
There was a problem hiding this comment.
nit:
| const acc = { metadataVaryPath: null, treeDivergedFromBase: false } | |
| const acc: RouteTreeAccumulator = { | |
| metadataVaryPath: null, | |
| treeDivergedFromBase: false, | |
| } |
Fixes a few bugs related to optimistic routing.
Originally reported as a next-intl request waterfall in MarkBekooy/prefetching-request-waterfall-bug#1, then extracted into an isolated regression test: A proxy that rewrites every URL to inject a leading path segment (
/one/two→/en/one/two, i.e. i18n with the default locale hidden from the URL) plus a fully dynamic target route like/[locale]/[...pages]leads to an infinite prefetch loop that never resolves.This regression uncovered several oversights in the optimistic routing implementation: when receiving a prefetch response, we did not check whether the response matched the expected result. If the tree mismatched, in some cases the prefetch task would fall into a loop, repeatedly attempting to fulfill the missing data.
Now when this happens, we record on the local route definition that a dynamic rewrite occurred, disabling further attempts to optimistically resolve the route. This is the same strategy we were already using for normal navigation responses, now applied to the prefetch path.
We also received another bug report with a similar root cause: parallel routes with conflicting dynamic params at the same level (
@modal/[...catchAll]next to[username]) cannot be distinguished using the current traversal algorithm, because it assumes that each segment is resolvable independently without inspecting the children or sibling branches. To work around this issue for now, when this scenario is detected, we disable optimistic routing for the conflicting route. We can model this properly in a future PR by having the server send down the "sibling" dynamic route segments, similar to what we do for static siblings already.Fixes #97135