fix(app-router): preserve global error details

[james-elicx]

Summary

• distinguish RSC render errors from SSR client-component errors at the app page boundary
• preserve the correct message, digest, and development stack for custom global error boundaries
• reconstruct the serialized error during client hydration

Next.js parity

Targets the two failures in test/e2e/app-dir/global-error/basic/index.test.ts from Next.js v16.2.6.

Validation

• 106 targeted unit/integration tests passed
• targeted App Router Playwright test passed with one worker
• vp run vinext#build passed
• scoped format/lint/types passed
• independent review found no actionable issues

[james-elicx]

/bigbonk review for issues

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@vinext/cloudflare@2249

npm i https://pkg.pr.new/vinext@2249

commit: 4c9650e

[github-actions]

Example	Preview	Production	Original
app-router-cloudflare	preview	production
pages-router-cloudflare	preview	production
app-router-playground	preview	production	original
realworld-api-rest	preview	production
nextra-docs-template	preview	production
benchmarks	preview	production
hackernews	preview	production	original
workers-cache	preview	production
web	preview	production

[github-actions]

Performance benchmarks

Compared 4c9650e against base cf9d601 using alternating same-runner rounds. Next.js was unchanged and skipped.

0 improved · 0 regressed · 6 within ±1.5%

Scenario	Framework	Baseline	Current	Change
Client bundle size (gzip)	vinext	132.8 KB	132.9 KB	⚫ +0.1%
Client entry size (gzip)	vinext	118.7 KB	118.7 KB	⚫ +0.1%
Dev server cold start	vinext	2.43 s	2.44 s	⚫ +0.2%
Production build time	vinext	3.02 s	3.01 s	⚫ -0.5%
RSC entry closure size (gzip)	vinext	92.2 KB	92.3 KB	⚫ +0.1%
Server bundle size (gzip)	vinext	166.9 KB	167.1 KB	⚫ +0.1%

View detailed results and traces

_{🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head}

[ask-bonk]

Review: fix(app-router): preserve global error details

I verified this change against the authoritative Next.js reference (test/e2e/app-dir/global-error/basic/index.test.ts) and Next.js source (packages/next/src/server/app-render/create-error-handler.tsx). The implementation is correct and matches Next.js behavior in all the cases the PR targets.

Parity verification (confirmed correct)

• RSC server-component error (production): sanitized generic message + digest present. Matches Next.js (Minified React error #441 + #digest). ✓
• Client-component SSR throw (production): original client page error message shown, no digest. This is exactly what Next.js does ('Global error: Error: client page error', hasElementByCssSelector('#digest') is falsy). The errorOrigin === "ssr" bypass of sanitizeErrorForClient is the correct way to model this — Next.js does not sanitize SSR client-component errors. ✓
• Unconditional digest stamping (app-rsc-errors.ts): Next.js stamps err.digest = stringHash(err.message + (err.stack || '')) unconditionally, not gated on NODE_ENV. The previous vinext behavior (production-only) was the divergence; this PR fixes it. The frozen-error try/catch guard is a sound defensive addition. ✓

Correctness checks (all passed)

• getCapturedError() === null is an exact-null check; capturedError is initialized to null and only set to a real captured error, so the "ssr"/"rsc" discrimination is reliable.
• onRenderError (which stamps the digest) runs during RSC render, before renderErrorBoundaryResponse reads the captured error, so RSC errors carry a digest in dev too — consistent with the new dev test.
• The other renderErrorBoundaryPage(error) call sites (app-page-request.ts:506 build-element errors, app-page-dispatch.ts:979 build errors) correctly fall through to the "rsc" default — those are RSC-origin and should remain sanitized in production.
• The digest-stamping change is isolated to the RSC onError path; the hasDigest dedup in wrapRscResponseForDevErrorReporting consumes a separate consumeInvalidDynamicUsageError store and is unaffected.
• The global-error→default-fallback path (renderWith(DEFAULT_GLOBAL_ERROR_COMPONENT)) still works: the default component is wrapped in SerializedErrorBoundary and receives a reconstructed error, which it accepts.

I ran the targeted unit tests (app-rsc-errors, app-page-boundary-render, app-page-render — 82 passed) and vp check on the changed files (format/lint/types clean).

Minor observations (non-blocking)

1. reset semantics differ from Next.js. SerializedErrorBoundary passes reset={() => globalThis.location?.reload()}. Next.js's error/global-error reset is a soft React state reset (this.setState({ error: null })), not a hard page reload. This is arguably an improvement over the prior behavior (the global-error path previously passed no reset at all, so the button was a no-op), but it's a deliberate divergence worth noting. If exact parity matters, consider routing through the existing ErrorBoundaryInner.reset instead of a full reload.

2. "digest" in errorObject ? String(errorObject.digest) : undefined would coerce a present-but-undefined digest to the string "undefined". In practice the sanitized error always sets a defined digest and SSR-origin errors have no digest key, so this can't trigger today — but a typeof errorObject.digest === "string" guard would be more defensive.

Neither is blocking. This is solid, well-researched parity work with fixtures ported directly from the Next.js test suite. LGTM.

[ask-bonk]

Review posted to PR #2249.

Summary

I reviewed the global-error parity fix against the authoritative Next.js reference test (test/e2e/app-dir/global-error/basic/index.test.ts) and Next.js source. The change is correct and matches Next.js behavior in all targeted cases.

Key findings (all verified correct):

• The errorOrigin === "ssr" bypass of sanitizeErrorForClient is the right model — Next.js shows the raw message (no digest) for client-component SSR throws, even in production.
• Unconditional digest stamping in app-rsc-errors.ts:156-168 matches Next.js (create-error-handler.tsx stamps err.digest regardless of NODE_ENV). The prior production-only behavior was the divergence; this fixes it. The frozen-error try/catch guard is sound.
• The getCapturedError() === null SSR/RSC discrimination is reliable, and the other renderErrorBoundaryPage call sites correctly default to "rsc".
• Confirmed the digest-stamping change doesn't regress the separate hasDigest dedup path.

Validation: ran the 3 targeted unit test files (82 passed) and vp check on changed files (clean).

Two minor non-blocking observations:

1. SerializedErrorBoundary uses location.reload() for reset, which diverges from Next.js's soft React state reset (though it's better than the prior no-op).
2. "digest" in errorObject ? String(...) could theoretically coerce to "undefined"; can't trigger today but a typeof guard would be more defensive.

No actionable issues — solid parity work with fixtures ported directly from the Next.js suite.

github run

[james-elicx]

/bigbonk review for issues

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: fix(app-router): preserve global error details

Re-reviewed the full PR (both commits — 1232f14 and the follow-up 4c9650e "preserve default global error semantics", which the prior automated review did not cover) against the authoritative Next.js reference (test/e2e/app-dir/global-error/basic/index.test.ts) and Next.js source (packages/next/src/server/app-render/create-error-handler.tsx). The implementation is correct and matches observable Next.js behavior in all targeted cases. No actionable issues.

Parity verification (confirmed against Next.js source/tests)

• SSR client-component throw → no digest, original message. The reference test asserts #error = 'Global error: Error: client page error' and hasElementByCssSelector('#digest') is falsy. The errorOrigin === "ssr" bypass of sanitizeErrorForClient (app-page-boundary-render.ts:435) reproduces this exactly. Note: Next.js's createHTMLErrorHandler does stamp a digest internally on SSR errors, but the global-error boundary never surfaces it — so "no digest displayed" is the correct observable behavior, and vinext gets there without stamping. ✓
• RSC server-component throw → digest present. Reference asserts #digest matches /\w+/, dev shows the original message, prod shows the minified React error. Matches. ✓
• Unconditional digest stamping (app-rsc-errors.ts:156). Confirmed against Next.js createReactServerErrorHandler: err.digest is stamped unconditionally (not gated on NODE_ENV). The prior production-only gate was the divergence; this fixes it. The frozen-error try/catch is a sound defensive guard and is covered by a test (returns a digest without masking frozen RSC errors). ✓

Second commit (4c9650e) — correct and necessary

Adding BoundaryComponent !== DEFAULT_GLOBAL_ERROR_COMPONENT to the SerializedErrorBoundary wrapping condition is the right call. When the user's global-error.tsx throws and the renderer falls back to renderWith(DEFAULT_GLOBAL_ERROR_COMPONENT) (app-page-boundary-render.ts:570), the built-in default must render directly (mirroring Next.js's direct errorComponent={DefaultGlobalError}) rather than being re-wrapped and routed through a serialization round-trip + location.reload() reset. The self-throw fallback path stays covered by tests/nextjs-compat/global-error.test.ts:393 (single <html>/<body>, 500 status).

Correctness checks (all pass)

• getCapturedError() === null discrimination is reliable: capturedError initializes to null (app-page-stream.ts:344) and is only set for non-navigation-signal RSC errors (:367). SSR throws happen in the separate SSR environment and never touch this tracker, so null ⇒ SSR-origin is sound.
• All other renderErrorBoundaryPage/renderErrorBoundaryResponse call sites (app-page-request.ts:506 build-element errors, app-page-dispatch.ts:979 build errors) correctly default to "rsc" — those are server-origin and should stay sanitized in production.
• app-page-stream.ts:326 invokes the discriminating closure from app-page-render.ts:871, so the two-arg origin propagation is consistent end-to-end.

Validation

• vp test run tests/app-page-boundary-render.test.ts tests/app-page-render.test.ts tests/app-rsc-errors.test.ts → 82 passed
• vp test run tests/nextjs-compat/global-error.test.ts → 24 passed
• vp check on all 7 changed source files → format/lint/types clean

Minor observations (non-blocking, same as noted before)

1. SerializedErrorBoundary reset is () => globalThis.location?.reload() — a hard reload vs Next.js's soft React state reset. Better than the prior no-op for the global-error path, but a deliberate divergence worth tracking if exact reset parity is ever required.
2. "digest" in errorObject ? String(errorObject.digest) : undefined could coerce a present-but-undefined digest to "undefined". Not triggerable today (sanitized errors always set a defined digest; SSR-origin errors have no digest key), but a typeof errorObject.digest === "string" guard would be more defensive.

LGTM — solid parity work, fixtures ported directly from the Next.js suite, and the follow-up commit correctly preserves the built-in default fallback semantics.

[ask-bonk]

Review posted to PR #2249.

Summary

I reviewed the full PR — both commits, including the follow-up 4c9650e "preserve default global error semantics" that the earlier automated review did not cover — against the authoritative Next.js reference test and Next.js source. The implementation is correct with no actionable issues.

Key verifications (confirmed against Next.js source/tests):

• SSR client-component throw → original message, no digest displayed. The errorOrigin === "ssr" bypass of sanitizeErrorForClient matches the reference test (#digest falsy, message 'client page error'). I confirmed via Next.js source that even though createHTMLErrorHandler stamps a digest internally, the global-error boundary never surfaces it for SSR errors — so vinext's "no digest" behavior is the correct observable parity.
• Unconditional digest stamping matches createReactServerErrorHandler (stamps err.digest regardless of NODE_ENV); the prior production-only gate was the divergence. The frozen-error try/catch is sound and tested.
• Second commit correctly excludes DEFAULT_GLOBAL_ERROR_COMPONENT from SerializedErrorBoundary wrapping, so the self-throw fallback renders the built-in default directly (mirroring Next.js), not through a serialization round-trip.

Correctness checks passed: getCapturedError() === null discrimination is reliable; all other call sites correctly default to "rsc"; two-arg origin propagation is consistent end-to-end.

Validation I ran: 82 unit tests + 24 nextjs-compat tests passed; vp check clean on all 7 source files.

Two minor non-blocking observations carried over (hard location.reload() reset vs Next.js soft reset; potential "undefined" digest coercion that can't trigger today). LGTM.

github run