[PR #1087] [MERGED] refactor(server): dedupe HTTP error response builders #1085

New issue

Closed

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

BreizhHardware commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: refactor/dedupe-http-error-responses

📝 Commits (2)

3b50586 refactor(server): dedupe HTTP error response builders
241a869 ci: retrigger workflows

📊 Changes

16 files changed (+140 additions, -54 deletions)

View changed files

📝 packages/vinext/src/server/app-middleware.ts (+2 -1)
📝 packages/vinext/src/server/app-page-method.ts (+2 -5)
📝 packages/vinext/src/server/app-page-request.ts (+3 -2)
📝 packages/vinext/src/server/app-prerender-endpoints.ts (+3 -2)
📝 packages/vinext/src/server/app-router-entry.ts (+4 -3)
📝 packages/vinext/src/server/app-rsc-handler.ts (+4 -3)
📝 packages/vinext/src/server/app-rsc-request-normalization.ts (+3 -2)
📝 packages/vinext/src/server/app-server-action-execution.ts (+5 -10)
📝 packages/vinext/src/server/app-ssr-entry.ts (+3 -2)
➕ packages/vinext/src/server/http-error-responses.ts (+90 -0)
📝 packages/vinext/src/server/image-optimization.ts (+3 -1)
📝 packages/vinext/src/server/metadata-route-response.ts (+6 -5)
📝 packages/vinext/src/server/middleware-runtime.ts (+3 -2)
📝 packages/vinext/src/server/pages-api-route.ts (+2 -1)
📝 packages/vinext/src/server/prod-server.ts (+2 -4)
📝 packages/vinext/src/server/request-pipeline.ts (+5 -11)

📄 Description

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

_{🔄 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/1087 **Author:** [@james-elicx](https://github.com/james-elicx) **Created:** 5/5/2026 **Status:** ✅ Merged **Merged:** 5/5/2026 **Merged by:** [@james-elicx](https://github.com/james-elicx) **Base:** `main` ← **Head:** `refactor/dedupe-http-error-responses` --- ### 📝 Commits (2) - [`3b50586`](https://github.com/cloudflare/vinext/commit/3b50586daa67a3742cf0703d93da237d93a5c371) refactor(server): dedupe HTTP error response builders - [`241a869`](https://github.com/cloudflare/vinext/commit/241a86977be24e9cad48e4806b1c03ebb67699b5) ci: retrigger workflows ### 📊 Changes **16 files changed** (+140 additions, -54 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/server/app-middleware.ts` (+2 -1) 📝 `packages/vinext/src/server/app-page-method.ts` (+2 -5) 📝 `packages/vinext/src/server/app-page-request.ts` (+3 -2) 📝 `packages/vinext/src/server/app-prerender-endpoints.ts` (+3 -2) 📝 `packages/vinext/src/server/app-router-entry.ts` (+4 -3) 📝 `packages/vinext/src/server/app-rsc-handler.ts` (+4 -3) 📝 `packages/vinext/src/server/app-rsc-request-normalization.ts` (+3 -2) 📝 `packages/vinext/src/server/app-server-action-execution.ts` (+5 -10) 📝 `packages/vinext/src/server/app-ssr-entry.ts` (+3 -2) ➕ `packages/vinext/src/server/http-error-responses.ts` (+90 -0) 📝 `packages/vinext/src/server/image-optimization.ts` (+3 -1) 📝 `packages/vinext/src/server/metadata-route-response.ts` (+6 -5) 📝 `packages/vinext/src/server/middleware-runtime.ts` (+3 -2) 📝 `packages/vinext/src/server/pages-api-route.ts` (+2 -1) 📝 `packages/vinext/src/server/prod-server.ts` (+2 -4) 📝 `packages/vinext/src/server/request-pipeline.ts` (+5 -11) </details> ### 📄 Description ## 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 - [x] `pnpm vp test run tests/app-router.test.ts` — 308 passed - [x] `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) - [x] `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 - [x] `pnpm fmt --write` - [x] `pnpm knip` — clean 🤖 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>