fix(app-router): redirect() returns 307 on document load#1571
fix(app-router): redirect() returns 307 on document load#1571james-elicx wants to merge 1 commit into
Conversation
The App Router prerender harness forwarded requests through `fetch` with fetch's default `redirect: "follow"` behavior. When a page called `redirect()` the prod server emitted a 307, fetch silently followed the Location header to the destination page, and the harness wrote the destination's HTML under the redirecting route's filename. The seeded cache then served that body with status 200 for every document request, breaking Next.js parity (test/e2e/app-dir/rsc-redirect/rsc-redirect.test.ts expects 307). Set `redirect: "manual"` on the App Router prerender `rscHandler` so the original 3xx surfaces back. `htmlRender.ok` is then false, the route is marked skipped, and at runtime the document request hits the live render path that already builds a proper 307 + Location response via `buildAppPageSpecialErrorResponse`. RSC flight-payload handling (#1347) is untouched. The pages-prerender `renderPage` helper already used `redirect: "manual"`, so this brings the App Router phase in line. Closes #1530
|
/bigbonk review |
commit: |
|
There was a problem hiding this comment.
Clean fix. The root cause analysis is correct — fetch's default redirect: "follow" was silently chasing the 307 inside the prerender harness, so the destination page's HTML got persisted under the redirecting route's filename. At runtime the seeded cache then served that body as a 200.
Code change — redirect: "manual" on the App Router rscHandler fetch (line 914) is the right one-liner. The 307 now propagates back as a non-ok response, the existing !response.ok check at line 1187 catches it, htmlRender.ok is false, and the speculative route falls through to "skipped". At runtime the live render path builds the proper 307 + Location via buildAppPageSpecialErrorResponse. Matches the pages-prerender renderPage helper at line 561 that already had redirect: "manual".
Tests are solid:
- Prerender test asserts the route is
"skipped"and neither.htmlnor.rscfiles are written for the redirecting filename — this is the key invariant that was violated. - Prod server test asserts 307 +
Location: /abouton a document load, which is the user-facing symptom from #1530. - Both tests reference the ported Next.js test (
test/e2e/app-dir/rsc-redirect/rsc-redirect.test.ts), per AGENTS.md guidance.
No concerns. The comment block explaining why redirect: "manual" is needed is appreciated — future readers will understand the interaction between fetch semantics and the prerender cache.
|
Approved PR #1571. The fix is correct and minimal — adding |
Closes #1530
Summary
redirect()now returns HTTP 307 with aLocationheader.fetchwith its defaultredirect: "follow"behavior, so the harness silently followed the 307 server-side and persisted the destination page's HTML under the redirecting route's filename. The seeded cache then served that body with status 200 for every document request.redirect: "manual"on the App Router prerenderrscHandler, matching what the pages-prerender helper already does. The redirect now propagates back, the route is markedskippedduring prerender, and at runtime the live render path emits the proper 307 + Location viabuildAppPageSpecialErrorResponse.redirect-test.rscstill returns 200 withNEXT_REDIRECTin the flight body;metadata-redirect-teststill returns 200 on document load.Test plan
tests/prerender.test.tsasserts/redirect-testis skipped and no HTML/RSC are written under the redirecting filenametests/app-router.test.tsasserts the production server returns 307 + Location for a document load/redirect-test.rscand/metadata-redirect-testtests still pass (RSC + metadata-redirect path untouched)vinext build --prerender-all+vinext start: document fetch of/redirect-testreturns 307,.rscfetch returns 200 with flight payloadvp checkpasses