[PR #1057] [MERGED] fix(app-page): align redirect()/notFound() handling under loading.tsx with Next.js #1057

New issue

Closed

opened 2026-05-06 13:11:46 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-06 13:11:46 +02:00

Owner

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/1057
Author: @james-elicx
Created: 5/4/2026
Status: ✅ Merged
Merged: 5/5/2026
Merged by: @james-elicx

Base: main ← Head: claude/condescending-matsumoto-430ee1

📝 Commits (10+)

e26849e fix(app-page-probe): always await page promise so redirect()/notFound() under loading.tsx return 307/404
c5fd96c refactor: capture special-error digests post-render instead of awaiting probe
baf26a6 Revert "refactor: capture special-error digests post-render instead of awaiting probe"
04010bd refactor: align with Next.js — skip page probe + post-shell race for special errors
3b6f178 refactor: drop 50ms swap-window — deterministic post-shell digest check
ea3be94 test: regression for permanentRedirect() under loading.tsx returning 308
eb22c16 test: regression for forbidden()/unauthorized() under loading.tsx
995ef63 fix(app-page): apply basePath to redirect Location header
d3798a2 fix(app-page): preserve cookies().set() on redirect responses
e5dd63b test: regression for notFound() under loading.tsx

📊 Changes

19 files changed (+431 additions, -13 deletions)

View changed files

📝 packages/vinext/src/entries/app-rsc-entry.ts (+1 -0)
📝 packages/vinext/src/server/app-page-dispatch.ts (+7 -0)
📝 packages/vinext/src/server/app-page-execution.ts (+65 -2)
📝 packages/vinext/src/server/app-page-probe.ts (+16 -1)
📝 packages/vinext/src/server/app-page-render.ts (+23 -0)
📝 packages/vinext/src/server/app-page-stream.ts (+21 -1)
📝 tests/app-page-execution.test.ts (+153 -0)
📝 tests/app-page-probe.test.ts (+13 -9)
📝 tests/app-router.test.ts (+64 -0)
➕ tests/fixtures/app-basic/app/forbidden-loading/loading.tsx (+3 -0)
➕ tests/fixtures/app-basic/app/forbidden-loading/page.tsx (+9 -0)
➕ tests/fixtures/app-basic/app/notfound-loading/loading.tsx (+3 -0)
➕ tests/fixtures/app-basic/app/notfound-loading/page.tsx (+12 -0)
➕ tests/fixtures/app-basic/app/permanent-protected-loading/loading.tsx (+3 -0)
➕ tests/fixtures/app-basic/app/permanent-protected-loading/page.tsx (+9 -0)
➕ tests/fixtures/app-basic/app/protected-loading/loading.tsx (+3 -0)
➕ tests/fixtures/app-basic/app/protected-loading/page.tsx (+14 -0)
➕ tests/fixtures/app-basic/app/unauthorized-loading/loading.tsx (+3 -0)
➕ tests/fixtures/app-basic/app/unauthorized-loading/page.tsx (+9 -0)

📄 Description

Summary

When a page route had both a sibling loading.tsx and an async page.tsx, redirect() / notFound() thrown by the page used to surface as a 200 response with a serialized "Switched to client rendering because the server rendering errored: NEXT_REDIRECT:/" body. Root cause: the probe fire-and-forgot the page promise to preserve loading.tsx streaming, so the rejection was caught by React's route-level Suspense boundary instead of vinext.

Fix mirrors Next.js's app-render.tsx:4293-4346 shape: skip the page probe for hasLoadingBoundary routes, capture NEXT_REDIRECT / NEXT_HTTP_ERROR_FALLBACK digests in rscErrorTracker.onRenderError, and after the SSR shell promise resolves inspect the tracker — if a digest was captured, swap the response to a 307/404 before any bytes are flushed. While here, also closes a few related parity gaps from the audit: basePath prefix on redirect Location, mutable cookies on redirects, and regression tests for permanentRedirect / forbidden / unauthorized under loading.tsx.

Changes

Core (the bug fix)

createAppPageRscErrorTracker (packages/vinext/src/server/app-page-stream.ts) — captures digest-bearing errors in getCapturedSpecialError() instead of dropping them.
probeAppPageBeforeRender (packages/vinext/src/server/app-page-probe.ts) — skips the page probe entirely when hasLoadingBoundary is true. Page now runs once, inside the RSC render.
renderAppPageLifecycle (packages/vinext/src/server/app-page-render.ts) — inspects the tracker once the shell promise resolves and swaps the response to a 307/404 if a digest is present.

Related parity fixes (separate commits)

basePath on redirect Location (packages/vinext/src/server/app-page-execution.ts) — redirect("/about") from a page mounted under basePath: "/blog" now produces Location: /blog/about. Mirrors Next.js's addPathPrefix(getURLFromRedirectError(err), basePath). External targets and already-prefixed paths are passed through unchanged. Layered correctly with the RSC cache-busting transform that landed in main.
cookies().set() on redirects (packages/vinext/src/server/app-page-execution.ts) — auth flows that do cookies().set("session", ...); redirect("/dashboard") now keep the Set-Cookie on the 307. Only applied to redirect responses to match Next.js (the http-access-fallback path leaves cookies to the rendered boundary).

Why this works without a timer

The SSR pipeline naturally runs many microtask awaits between the page Promise rejecting (captured by React's onError) and the lifecycle reading the tracker. Throws that settle in microtasks during shell rendering are deterministically captured — verified across multiple test runs.

Throws that require macrotask boundaries (real I/O, setTimeout, fetch) are not caught and fall through to the streamed body — the same architectural limit Next.js has. The digest survives in the Flight payload for the client router to consume (separate gap in vinext's client-side handling, flagged below).

Alignment with Next.js

	Next.js	This PR
Page invocations per request	1	1
Per-request probe step	none	none (for loading-boundary routes)
Catch mechanism	`try/catch` around shell-ready render	post-shell digest check
Sync / microtask redirects → 307	✓	✓
Macrotask-async redirects	digest in Flight (client router)	digest in Flight (client router handling = gap)
Timer / heuristic	none	none
`permanentRedirect()` (308)	✓	✓
`forbidden()` (403) / `unauthorized()` (401)	✓	✓
`basePath` on redirect Location	✓	✓
`Set-Cookie` from `cookies().set()` on redirect	✓	✓

Test plan

pnpm test tests/app-router.test.ts × 3 runs — 306/306 pass each time (deterministic).
pnpm test:unit — 3626/3626 pass (including new unit coverage for basePath and pending-cookies on buildAppPageSpecialErrorResponse).
npx playwright test tests/e2e/app-router/loading.spec.ts — 3/3 pass (including OpenNext-compat "loading boundary is visible before content resolves").
npx playwright test tests/e2e/app-router/ -g "redirect" — 34/34 pass.

New regression coverage:

tests/fixtures/app-basic/app/protected-loading/ — redirect() (307) under loading.tsx.
tests/fixtures/app-basic/app/permanent-protected-loading/ — permanentRedirect() (308) under loading.tsx, verifies digest statusCode is preserved.
tests/fixtures/app-basic/app/forbidden-loading/ — forbidden() (403) under loading.tsx.
tests/fixtures/app-basic/app/unauthorized-loading/ — unauthorized() (401) under loading.tsx.
tests/app-page-execution.test.ts — five basePath cases (internal / external / already-prefixed / unconfigured / root-target) + three cookie cases (with cookies / without / fallback-doesn't-bleed).

Known gaps (separate PRs)

Macrotask-async redirects (await fetch(); redirect() etc.) leak as streamed bodies; full parity needs client-router handling of Flight-payload digests inside Suspense boundaries.
Action-handler redirects (x-action-redirect header + createRedirectRenderResult) — entirely out of scope.
Nested HTTP error boundary lookup (findPrerenderHTTPErrorBoundaryTree) — Next's nearest-forbidden.tsx / unauthorized.tsx resolution from layout depth.

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/cloudflare/vinext/pull/1057 **Author:** [@james-elicx](https://github.com/james-elicx) **Created:** 5/4/2026 **Status:** ✅ Merged **Merged:** 5/5/2026 **Merged by:** [@james-elicx](https://github.com/james-elicx) **Base:** `main` ← **Head:** `claude/condescending-matsumoto-430ee1` --- ### 📝 Commits (10+) - [`e26849e`](https://github.com/cloudflare/vinext/commit/e26849ea813357a0fb56388bb19b61d05eef5c4e) fix(app-page-probe): always await page promise so redirect()/notFound() under loading.tsx return 307/404 - [`c5fd96c`](https://github.com/cloudflare/vinext/commit/c5fd96c226123356a7493648fefa48faee1b580d) refactor: capture special-error digests post-render instead of awaiting probe - [`baf26a6`](https://github.com/cloudflare/vinext/commit/baf26a682e70604b06627f3e053ed8513ef8c2e3) Revert "refactor: capture special-error digests post-render instead of awaiting probe" - [`04010bd`](https://github.com/cloudflare/vinext/commit/04010bd465bb8af8ba9f1d04f2e7d8c28a57eb93) refactor: align with Next.js — skip page probe + post-shell race for special errors - [`3b6f178`](https://github.com/cloudflare/vinext/commit/3b6f1789e9995d77c45fdcbd89de47469e12b79f) refactor: drop 50ms swap-window — deterministic post-shell digest check - [`ea3be94`](https://github.com/cloudflare/vinext/commit/ea3be947acb7d0dbfcd7fdb029bab31d1fb4eefe) test: regression for permanentRedirect() under loading.tsx returning 308 - [`eb22c16`](https://github.com/cloudflare/vinext/commit/eb22c1640d569ebca4c81d8034d934a79acee1d5) test: regression for forbidden()/unauthorized() under loading.tsx - [`995ef63`](https://github.com/cloudflare/vinext/commit/995ef6304350bca31de2ebeb84bd53c8ce0ece00) fix(app-page): apply basePath to redirect Location header - [`d3798a2`](https://github.com/cloudflare/vinext/commit/d3798a2562e6b3922af91686242f29af1870e35b) fix(app-page): preserve cookies().set() on redirect responses - [`e5dd63b`](https://github.com/cloudflare/vinext/commit/e5dd63b74b0e7d5a4d9fc50c38aed8ae6872353c) test: regression for notFound() under loading.tsx ### 📊 Changes **19 files changed** (+431 additions, -13 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/entries/app-rsc-entry.ts` (+1 -0) 📝 `packages/vinext/src/server/app-page-dispatch.ts` (+7 -0) 📝 `packages/vinext/src/server/app-page-execution.ts` (+65 -2) 📝 `packages/vinext/src/server/app-page-probe.ts` (+16 -1) 📝 `packages/vinext/src/server/app-page-render.ts` (+23 -0) 📝 `packages/vinext/src/server/app-page-stream.ts` (+21 -1) 📝 `tests/app-page-execution.test.ts` (+153 -0) 📝 `tests/app-page-probe.test.ts` (+13 -9) 📝 `tests/app-router.test.ts` (+64 -0) ➕ `tests/fixtures/app-basic/app/forbidden-loading/loading.tsx` (+3 -0) ➕ `tests/fixtures/app-basic/app/forbidden-loading/page.tsx` (+9 -0) ➕ `tests/fixtures/app-basic/app/notfound-loading/loading.tsx` (+3 -0) ➕ `tests/fixtures/app-basic/app/notfound-loading/page.tsx` (+12 -0) ➕ `tests/fixtures/app-basic/app/permanent-protected-loading/loading.tsx` (+3 -0) ➕ `tests/fixtures/app-basic/app/permanent-protected-loading/page.tsx` (+9 -0) ➕ `tests/fixtures/app-basic/app/protected-loading/loading.tsx` (+3 -0) ➕ `tests/fixtures/app-basic/app/protected-loading/page.tsx` (+14 -0) ➕ `tests/fixtures/app-basic/app/unauthorized-loading/loading.tsx` (+3 -0) ➕ `tests/fixtures/app-basic/app/unauthorized-loading/page.tsx` (+9 -0) </details> ### 📄 Description ## Summary When a page route had both a sibling `loading.tsx` and an async `page.tsx`, `redirect()` / `notFound()` thrown by the page used to surface as a `200` response with a serialized *"Switched to client rendering because the server rendering errored: NEXT_REDIRECT:/"* body. Root cause: the probe fire-and-forgot the page promise to preserve `loading.tsx` streaming, so the rejection was caught by React's route-level Suspense boundary instead of vinext. Fix mirrors Next.js's `app-render.tsx:4293-4346` shape: skip the page probe for `hasLoadingBoundary` routes, capture `NEXT_REDIRECT` / `NEXT_HTTP_ERROR_FALLBACK` digests in `rscErrorTracker.onRenderError`, and after the SSR shell promise resolves inspect the tracker — if a digest was captured, swap the response to a 307/404 before any bytes are flushed. While here, also closes a few related parity gaps from the audit: basePath prefix on redirect Location, mutable cookies on redirects, and regression tests for `permanentRedirect` / `forbidden` / `unauthorized` under `loading.tsx`. ## Changes **Core (the bug fix)** - `createAppPageRscErrorTracker` ([packages/vinext/src/server/app-page-stream.ts](packages/vinext/src/server/app-page-stream.ts)) — captures `digest`-bearing errors in `getCapturedSpecialError()` instead of dropping them. - `probeAppPageBeforeRender` ([packages/vinext/src/server/app-page-probe.ts](packages/vinext/src/server/app-page-probe.ts)) — skips the page probe entirely when `hasLoadingBoundary` is true. Page now runs **once**, inside the RSC render. - `renderAppPageLifecycle` ([packages/vinext/src/server/app-page-render.ts](packages/vinext/src/server/app-page-render.ts)) — inspects the tracker once the shell promise resolves and swaps the response to a 307/404 if a digest is present. **Related parity fixes (separate commits)** - **basePath on redirect Location** ([packages/vinext/src/server/app-page-execution.ts](packages/vinext/src/server/app-page-execution.ts)) — `redirect("/about")` from a page mounted under `basePath: "/blog"` now produces `Location: /blog/about`. Mirrors Next.js's `addPathPrefix(getURLFromRedirectError(err), basePath)`. External targets and already-prefixed paths are passed through unchanged. Layered correctly with the RSC cache-busting transform that landed in main. - **`cookies().set()` on redirects** ([packages/vinext/src/server/app-page-execution.ts](packages/vinext/src/server/app-page-execution.ts)) — auth flows that do `cookies().set("session", ...); redirect("/dashboard")` now keep the `Set-Cookie` on the 307. Only applied to redirect responses to match Next.js (the http-access-fallback path leaves cookies to the rendered boundary). ## Why this works without a timer The SSR pipeline naturally runs many microtask awaits between the page Promise rejecting (captured by React's `onError`) and the lifecycle reading the tracker. Throws that settle in microtasks during shell rendering are **deterministically captured** — verified across multiple test runs. Throws that require macrotask boundaries (real I/O, `setTimeout`, `fetch`) are **not** caught and fall through to the streamed body — the same architectural limit Next.js has. The digest survives in the Flight payload for the client router to consume (separate gap in vinext's client-side handling, flagged below). ## Alignment with Next.js | | Next.js | This PR | |---|---|---| | Page invocations per request | 1 | 1 | | Per-request probe step | none | none (for loading-boundary routes) | | Catch mechanism | `try/catch` around shell-ready render | post-shell digest check | | Sync / microtask redirects → 307 | ✓ | ✓ | | Macrotask-async redirects | digest in Flight (client router) | digest in Flight (client router handling = gap) | | Timer / heuristic | none | none | | `permanentRedirect()` (308) | ✓ | ✓ | | `forbidden()` (403) / `unauthorized()` (401) | ✓ | ✓ | | `basePath` on redirect Location | ✓ | ✓ | | `Set-Cookie` from `cookies().set()` on redirect | ✓ | ✓ | ## Test plan - [x] `pnpm test tests/app-router.test.ts` × 3 runs — 306/306 pass each time (deterministic). - [x] `pnpm test:unit` — 3626/3626 pass (including new unit coverage for basePath and pending-cookies on `buildAppPageSpecialErrorResponse`). - [x] `npx playwright test tests/e2e/app-router/loading.spec.ts` — 3/3 pass (including OpenNext-compat *"loading boundary is visible before content resolves"*). - [x] `npx playwright test tests/e2e/app-router/ -g "redirect"` — 34/34 pass. **New regression coverage:** - `tests/fixtures/app-basic/app/protected-loading/` — `redirect()` (307) under `loading.tsx`. - `tests/fixtures/app-basic/app/permanent-protected-loading/` — `permanentRedirect()` (308) under `loading.tsx`, verifies digest statusCode is preserved. - `tests/fixtures/app-basic/app/forbidden-loading/` — `forbidden()` (403) under `loading.tsx`. - `tests/fixtures/app-basic/app/unauthorized-loading/` — `unauthorized()` (401) under `loading.tsx`. - `tests/app-page-execution.test.ts` — five basePath cases (internal / external / already-prefixed / unconfigured / root-target) + three cookie cases (with cookies / without / fallback-doesn't-bleed). ## Known gaps (separate PRs) - **Macrotask-async redirects** (`await fetch(); redirect()` etc.) leak as streamed bodies; full parity needs client-router handling of Flight-payload digests inside Suspense boundaries. - **Action-handler redirects** (`x-action-redirect` header + `createRedirectRenderResult`) — entirely out of scope. - **Nested HTTP error boundary lookup** (`findPrerenderHTTPErrorBoundaryTree`) — Next's nearest-`forbidden.tsx` / `unauthorized.tsx` resolution from layout depth. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>