refactor(server): dedupe HTTP error response builders

[james-elicx]

Summary

Follow-up to #1058 / #1071 / #1078. Extracts a fresh batch of new Response(...) HTTP error builders into a shared module, packages/vinext/src/server/http-error-responses.ts. Now home to:

• badRequestResponse(init?) — 400 "Bad Request"
• forbiddenResponse() — 403 "Forbidden" (moved from request-pipeline.ts)
• notFoundResponse(init?) — 404 "Not Found"
• methodNotAllowedResponse(allowedMethods, init?) — 405 "Method Not Allowed" with Allow header
• payloadTooLargeResponse() — 413 "Payload Too Large" (moved from app-server-action-execution.ts)
• internalServerErrorResponse(message?, init?) — 500 "Internal Server Error"

Each helper accepts an optional headers init so callers (e.g. app-rsc-handler, app-page-method, prod-server) can merge middleware response headers without re-implementing the new Response(...) boilerplate.

Standardized status codes / sites touched

• 404 → "Not Found"
  • app-page-request.ts (×2) — validateAppPageDynamicParams
  • app-prerender-endpoints.ts (×2) — disabled prerender endpoints
  • app-rsc-handler.ts — plain 404 fallback (with merged middleware headers)
  • app-rsc-request-normalization.ts — basePath miss
  • app-router-entry.ts (×2) — protocol-relative guard, null result
  • app-ssr-entry.ts (×2) — protocol-relative guard, null result
  • metadata-route-response.ts (×5) — sitemap/image/dynamic 404s
  • prod-server.ts — Node static-served 404 (with merged static headers)
  • request-pipeline.ts (×2) — guardProtocolRelativeUrl, normalizeTrailingSlash
• 400 → "Bad Request"
  • app-router-entry.ts, app-rsc-request-normalization.ts, image-optimization.ts, middleware-runtime.ts
• 405 → "Method Not Allowed" (with Allow: GET, HEAD)
  • app-page-method.ts
• 500 → "Internal Server Error"
  • app-middleware.ts, pages-api-route.ts
  • middleware-runtime.ts (dev-mode dynamic message via internalServerErrorResponse(message))
  • app-server-action-execution.ts (×2; prod = canonical, dev = "Server action failed: ..." via internalServerErrorResponse(message))
• 413 → "Payload Too Large" (payloadTooLargeResponse is now the shared symbol)

Body string changes (intentional, called out)

Five sites that previously returned the body "404 Not Found" (the protocol-relative open-redirect guards in request-pipeline.ts ×2, app-router-entry.ts, app-ssr-entry.ts, plus the trailing-slash defense-in-depth check) now return the canonical "Not Found" body. Status code (404) and absence of Content-Type are unchanged. The sibling generated-worker template strings in deploy.ts keep the old body to avoid touching codegen output.

Sites left inline (intentional)

• Template-string sites (cannot import runtime helpers without restructuring codegen): deploy.ts, entries/pages-server-entry.ts, server/dev-origin-check.ts.
• Test-asserted custom bodies: pages-api-route.ts:47 and prod-server.ts:1593 keep "404 - API route not found" (asserted in tests/pages-api-route.test.ts:148); pages-api-route.ts:53 keeps "API route does not export a default function".
• Single-occurrence custom messages: app-page-dispatch.ts:303 ("Page has no default export"); pages-page-data.ts:139 ("404"); image-optimization.ts (×4: "Image not found", "The requested resource is not an allowed image type"); app-prerender-endpoints.ts (×2: "missing pattern"); request-pipeline.ts (image-URL validation custom bodies).
• Null-body status responses: app-route-handler-dispatch.ts (new Response(null, { status: 400 / 405 })), app-route-handler-execution.ts (status: 500 with null body) — different shape from text-body helpers.
• Single-use 502 / 504: config-matchers.ts (proxy gateway responses).

Test plan

• pnpm vp test run tests/app-router.test.ts — 308 passed
• pnpm vp test run tests/pages-router.test.ts — 200 tests pass (pre-existing afterAll cleanup hook timeout, unrelated to this change; verified by stashing the diff and reproducing on main)
• pnpm vp test run tests/app-rsc-handler.test.ts tests/app-prerender-endpoints.test.ts tests/app-page-request.test.ts tests/pages-api-route.test.ts tests/api-handler.test.ts tests/app-page-dispatch.test.ts tests/metadata-route-response.test.ts tests/app-page-route-wiring.test.ts tests/app-page-execution.test.ts tests/app-route-handler-policy.test.ts tests/image-optimization-parity.test.ts tests/image-config.test.ts tests/app-post-middleware-context.test.ts — 230 passed
• pnpm fmt --write
• pnpm knip — clean

🤖 Generated with Claude Code

[pkg-pr-new]

Open in StackBlitz

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

commit: 241a869

[james-elicx]

/bigbonk review

[ask-bonk]

Clean refactor. Reviewed all 16 files — the extraction is mechanically correct and well-scoped.

What I checked:

• forbiddenResponse move from request-pipeline.ts to http-error-responses.ts — no external consumers imported it from the old location (index.ts and deploy.ts only import other symbols from request-pipeline). The import in request-pipeline.ts itself is updated correctly.
• methodNotAllowedResponse in app-page-method.ts — the old code created Headers, merged middleware headers, set Allow, then passed to new Response. The new code creates Headers, merges middleware headers, passes to methodNotAllowedResponse which wraps in new Headers(init.headers) and sets Allow. One extra Headers copy but semantically identical.
• notFoundResponse variable rename to renderedNotFoundResponse in app-rsc-handler.ts — necessary to avoid shadowing the imported helper. Clean.
• internalServerErrorResponse(message) usage in app-server-action-execution.ts — the undefined for production correctly falls back to the ?? default. Both call sites are consistent.
• Body string changes ("404 Not Found" → "Not Found") — confirmed no tests assert on the old body strings. Template-string sites in deploy.ts are left unchanged as documented.
• dev-origin-check.ts keeps its inline new Response("Forbidden", ...) — correctly listed as intentionally-inline.
• No remaining canonical-body new Response(...) calls outside http-error-responses.ts and dev-origin-check.ts.

Nothing to flag. Good to merge.

[ask-bonk]

Review submitted — approved with detailed verification notes. The refactor is mechanically correct: all 16 files are consistent, the forbiddenResponse move has no broken external consumers, header semantics are preserved, the variable rename avoids shadowing, and the body string normalization ("404 Not Found" → "Not Found") has no test impact.

github run