fix(middleware): run on missing build assets#2242
Conversation
|
/bigbonk review for issues |
commit: |
|
Performance benchmarksCompared 0 improved · 0 regressed · 6 within ±1.5%
View detailed results and traces 🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head |
|
@james-elicx Bonk workflow was cancelled. View workflow run · To retry, trigger Bonk again. |
|
/bigbonk review for issues |
|
|
Posted my review to PR #2242. Summary of findings: Blocking issue: incomplete fix for the primary deployment targetThe PR fixes the Node prod servers (
Result: a middleware rewrite of a missing static chunk works on the Node server but silently fails when deployed to Workers — exactly the dev/prod divergence AGENTS.md warns against, left as an in-scope "known bug." Test gapThe new assertions only exercise the Pages Router Node prod server. There's no coverage for the App Router prod server (also modified here) or either Workers entry, so CI stays green while the Workers regression ships unnoticed. Correct parts
Recommendation: apply the same change to both Workers entries and add coverage that actually exercises them before merge. |
|
/bigbonk review for issues |
|
|
Posted my review to PR #2242. I independently verified every finding against the four parity sites in AGENTS.md (the PR is unchanged at SummaryBlocking: incomplete fix for the primary deployment target (Workers) The Node prod-server fix is correct, but the same short-circuit still exists in both Cloudflare Workers entries:
Result: a middleware rewrite of a missing Test gap The new assertions only exercise the Pages Router Node prod server ( Verified correct
Recommendation: Apply the fix to both Workers entries and add coverage that exercises them before merge. |
Summary
/_next/static/*requests reach middleware instead of returning from the static-file fast path404 Not Foundresponse when middleware/routing does not handle the missing assetFailure cluster
The latest Next.js deploy-suite artifact for
test/e2e/middleware-general/test/node-runtime.test.tsshowed the non-cache assertion:should be able to rewrite on _next/static/chunks/pages/ 404: expected200, received404The related original-request-shape assertion also observed no middleware header for a missing
/_next/static/*URL. Data-request assertions from the same suite are owned separately by #2239; the Suspense/app-static cluster is also actively owned elsewhere.Root cause
The Node production servers served existing build assets before middleware, which is correct, but also returned a static-layer 404 immediately for missing asset-shaped paths. Next.js allows middleware to rewrite or respond to a missing
/_next/static/*request before falling back to its plain-text asset 404.Next.js parity
Regression coverage is ported from:
test/e2e/middleware-general/test/index.test.tsThe local fixture verifies all three parts of the contract:
200without middleware headers/_next/static/*request404 text/plain; charset=utf-8with bodyNot FoundValidation
Targeted one-worker tests only:
vp test run tests/asset-prefix.test.ts -t "Pages Router with basePath alone serves assets" --maxWorkers=1vp test run tests/asset-prefix.test.ts -t "unset: emits URLs under /_next/static/" --maxWorkers=1vp check packages/vinext/src/server/prod-server.ts tests/asset-prefix.test.ts