Skip to content

fix: contain request failures at the Start handler edge - #383

Open
everton-dgn wants to merge 1 commit into
solidjs:nextfrom
everton-dgn:fix/middleware-error-containment
Open

everton-dgn wants to merge 1 commit into
solidjs:nextfrom
everton-dgn:fix/middleware-error-containment

Conversation

@everton-dgn

Copy link
Copy Markdown

Summary

Fixes #382.

  • The generated Start handler now settles whatever escapes the start.middleware chain at the handler edge instead of letting it reject to the host.
  • A thrown Response (except Response.error()), or the Response a thrown respond() envelope carries, becomes the response in dev and production. throw redirect('/login') from a middleware now answers the 302, as the server-function endpoint already does.
  • In a production build any other failure (a middleware throw, a start.setup or start.renderMode module failure, an uncaught in-process server-function call) is reported once to the configureServerErrors hook, or logged with console.error without one, and answered with a bodyless 500. That 500 carries the headers and cookies written to the request event while the response head is still open.
  • Dev keeps rejecting real failures, so the dev server still sees the original error (Vite's overlay with the built-in dev middleware).
  • A client-mode build now fails when prerendering the shell answers a non-2xx status, instead of writing that response to dist/client/index.html.

Cause

handleRequest ran the chain without a catch (src/ssr/index.ts at 52d93eb, L1433-L1453). Each host answered the rejection its own way: Nitro with a JSON 500 that drops the stub cookies, start.node with a plain-text 500, and the example server.js by echoing e.message to the client. The configureServerErrors hook never saw these failures, and a thrown control Response became a 500. In dev a thrown Response reached Vite's error middleware, whose prepareError reads err.message and throws ERR_INVALID_ARG_TYPE, so it answered 500 there too.

Fix

  • handleRequest wraps the chain in try/catch inside the request scope and hands failures to a generated containFailure(error, event). commitEventResponse(response, event) still folds strictly after the chain unwinds.
  • Classification: isResponseEnvelope(error) ? error.response : error, then instanceof Response && status !== 0. It runs in its own try, so a hostile thrown value (a revoked Proxy) is treated as an ordinary failure instead of escaping the catch.
  • Build: reportServerError(error, { kind: 'render', handling: 'failed', event }) from solid-js/internal, then console.error(error) when no hook is registered. This mirrors failRender in @solidjs/web. The answer is new Response(null, { status: 500 }), the endpoint's plain-HTTP answer.
  • Dev: same classification; anything else is rethrown unchanged.
  • handleRequest(request, { renderMode }) validates the option before the chain runs. A host's bad option still rejects the call instead of being contained as a 500. A bad result from the start.renderMode module is a request failure and is contained.
  • Client-mode prerender: if (!response.ok) throw new Error('[@solidjs/vite-plugin] prerendering the client-mode shell failed: the handler answered <status>').
  • README (middleware and renderMode sections) and a patch changeset.

Verification

pnpm run build
cd examples/start-ssr && node test/run.mjs <mode>
cd examples/start-client && node test/run.mjs <mode>
Suite / mode origin/next this branch
start-ssr middleware 75/75 93/93
start-ssr preview 34/34 47/47
start-ssr node 74/74 82/82
start-ssr render-mode not measured 121/121
start-ssr prod not measured 60/60
start-client prod 23/23 26/26
start-client node 21/21 21/21

The dev handler changed too, so I also ran the wider local gate on this branch: start-ssr all modes 666/666, test/http-bridge.mjs 10/10, test/components-warning.mjs 11/11, test/webworker-warning.mjs 12/12, test/dedupe.mjs 8/8, test/host-dispatch.mjs ok, start-client all modes 68/68, start-env 47/47, css-matrix 87/87.

New checks:

  • start-ssr fixtures in src/middleware.ts, all thrown from the outermost middleware outside its error middleware:
    • /mw-throw: an Error, after writing a stub cookie.
    • /mw-throw-late: a throw after next() returned the page.
    • /mw-redirect: a thrown redirect().
    • /mw-envelope: a thrown respond() envelope with a 409 and a header.
    • /mw-response-error: a thrown Response.error().
    • /mw-direct-throw: an uncaught in-process server-function failure.
    • /setup-throw: a start.setup failure.
    • SSR_SERVER_ERRORS=1 registers a recording hook, read back from /api/server-errors.
  • mw-prod, preview, node:
    • The thrown Response and envelope answer 302 and 409.
    • Everything else answers an empty-body 500.
    • The stub cookie arrives exactly once.
  • With the hook (mw-prod, preview):
    • Each failure is heard once as render/failed with the event.
    • The direct server-function failure is heard once, as the runtime's server-function/thrown report.
    • Thrown responses are not reported; Response.error() is.
    • No original error reaches the server log.
  • Without the hook (node): each original error reaches the log through console.error.
  • mw-dev:
    • The thrown redirect and envelope answer 302 and 409.
    • The Error still gets the overlay 500 with its message, and the hook stays silent.
    • A codegen check confirms the dev handler rethrows and imports no reportServerError.
  • render-mode: the built handler still rejects handleRequest(request, { renderMode: 'bogus' }).
  • start-client prod: a build with SOLID_SHELL_FAIL=1 (a start.middleware that throws) exits non-zero with the new message and the original error in the log, and writes no dist/client/index.html.

Regression proof:

  • New tests against the origin/next src/ssr/index.ts: middleware 79/93, preview 35/47, node 75/82, start-client prod 25/26. Render-mode stays 121/121: the renderMode check passes on the old code because it never contained anything.
  • Against a variant that calls the hook through the global slot and drops the renderMode pre-check:
    • middleware 92/93: the direct server-function failure is heard twice.
    • render-mode 120/121: the bogus option resolves to a 500.

Notes

  • Internal API: reportServerError from solid-js/internal. It is the typed export failRender itself calls, and it is present at the ^2.0.0-rc.10 peer floor and in rc.11. The issue suggested calling the hook through the Symbol.for('solid-js/server/errors') slot. Going through the runtime's once-per-error ledger instead avoids a second report for an uncaught in-process server-function call, which the runtime already reports as server-function/thrown before rethrowing. The slot is still read, but only to decide the console.error fallback, as failRender does. A public entry point for request failures in @solidjs/web, with its own kind in ServerErrorSite, would remove this coupling.
  • Site: { kind: 'render', handling: 'failed' } plus event. It is what failRender reports and a valid ServerErrorSite, and the hook's return value is ignored for failed.
  • Deviation from the issue: the thrown-Response branch also runs in dev. Before, dev crashed inside Vite's error middleware for a thrown Response, and the README now recommends throw redirect(). Every other failure in dev is rethrown exactly as before.
  • The start.setup and start.renderMode default-export checks are module top-level throws: they fail at import, not per request, and are not contained.
  • Headers and cookies written to the request event only reach the 500 while the head is open. Once next() has returned a rendered page, the stub is committed with that page, and a later throw drops them (documented). The streamed body of that dropped page is not cancelled.
  • Async render failure before the shell: with no boundary around the app (for example errorBoundary: false), a render that fails asynchronously before the first chunk leaves the response pending forever. The chain is failRender, then abandon, then the pipe's onFailure calling end(). createSSRResponse's end() returns early while there is no stream controller, so its promise never resolves. createSSRResponse is identical in @solidjs/web rc.10 and rc.11. Nothing is thrown, so this containment cannot catch it, and it needs a runtime fix. Probed on rc.10: pending after 3s without a boundary; a 500 with Errored + httpStatus(500); a 200 shell with only Loading.
  • Not covered by a fixture: a start.renderMode module failure, which goes through the same catch as the start.setup one.
  • Interaction with refactor: rename Start mode to app mode #327 (Start to app rename). If this lands first, refactor: rename Start mode to app mode #327 needs to:
    • rename errorBoundary to productionErrorBoundary in the new README paragraph;
    • change start.setup/start.renderMode to app.* in the codegen comment and the changeset;
    • move examples/start-client/src/shell-failure.ts with the app-client rename;
    • switch the SOLID_SHELL_FAIL knob in that example's vite.config.ts from start: to app:.
      If refactor: rename Start mode to app mode #327 lands first, this PR makes the same changes in reverse.

Fixes solidjs#382. handleRequest now settles whatever escapes the start.middleware chain: a thrown Response or response envelope becomes the response, and in production builds any other failure is reported through the configured server error policy and answered with a bodyless 500 instead of rejecting to the host. A client-mode build fails when prerendering the shell answers a non-2xx status.
@changeset-bot

changeset-bot Bot commented Sep 30, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: efdaf56

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@solidjs/vite-plugin Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant