diff --git a/.changeset/ssr-response-settles-pre-shell.md b/.changeset/ssr-response-settles-pre-shell.md new file mode 100644 index 000000000..1e397a1ac --- /dev/null +++ b/.changeset/ssr-response-settles-pre-shell.md @@ -0,0 +1,5 @@ +--- +"@solidjs/web": patch +--- + +`createSSRResponse` no longer hangs when a streamed render ends before its shell flushes (#3719). A render that fails pre-shell (`onError` hears `handling: "failed"`) or is aborted through its `signal` now resolves with a bodyless 500, or a redirect when a `Location` is already on the response stub, and the stub is committed. A render that succeeds with an empty document resolves with an empty 200. The promise still never rejects. diff --git a/documentation/solid-2.0/12-ssr-http.md b/documentation/solid-2.0/12-ssr-http.md index 93d282f71..cfb7f5a8c 100644 --- a/documentation/solid-2.0/12-ssr-http.md +++ b/documentation/solid-2.0/12-ssr-http.md @@ -327,6 +327,7 @@ export function handleRequest(request: Request): Promise { - **At shell flush** — the moment the head freezes — the stub is `committed` and its status/headers are merged over `options.responseInit` (`Set-Cookie` values survive as separate entries; `Server-Timing` folds entry by entry; `content-type` defaults to `text/html; charset=utf-8`). The commit is also where the request's trace reaches the head (`Server-Timing: traceparent;desc="…"` — see `getTraceContext()`). - **A `Location` present before the flush** becomes a real redirect instead of an HTML response: bodyless, carrying the stub’s cookies, with the status from `getExpectedRedirectStatus` (also exported — the stub’s own status when it is a redirect status, `302` otherwise, because a status set for the page render doesn’t describe the redirect that preempts it). - **A `Location` set after the flush** can only be honored client-side: stream completion appends `` before closing, carrying `options.nonce` so a strict `script-src` CSP doesn’t block it. + - **A render that ends before the flush** (it failed, which `onError` hears as `handling: "failed"`, or its `signal` aborted) produced no page: the promise resolves with a bodyless `500` (a `Location` already on the stub still redirects) and the stub is committed. It never rejects. - `options.transformChunk(chunk)` rewrites each outgoing HTML chunk — the seam handlers use for entry-script injection and doctype prefixes. String results return a `Response` synchronously; stream results return a promise that resolves at shell flush, so returning it from a fetch handler sends the head at the right moment by construction. diff --git a/packages/web/src/server.ts b/packages/web/src/server.ts index b89bba0d5..c6bafd67f 100644 --- a/packages/web/src/server.ts +++ b/packages/web/src/server.ts @@ -1849,6 +1849,13 @@ export function renderToString(code, options = {}) { if (dispose) dispose(); } } + +// `createSSRResponse`'s sink method for a render that ended before its shell +// reached the sink — failed or aborted. A bare `end()` with nothing written +// is a successful empty document, so the two need separate roads; every +// other `pipe` sink keeps the plain `end()` (failure) or silence (abort). +const SHELL_ABANDONED = Symbol(); + export function renderToStream( fn: () => T, options?: { @@ -1986,6 +1993,9 @@ export function renderToStream(code, options = {}) { let disconnected = false; let failed = false; let onFailed; + // A disconnect leaves the sink alone, but `createSSRResponse`'s promise + // still waits on a shell that will never come (see `SHELL_ABANDONED`). + let onDisconnectedBeforeShell; // What the render still hands the serializer once it is torn down: a // rejection with nobody to hear it is owned here; an iterable is never // started, so there is nothing to return. @@ -2074,7 +2084,7 @@ export function renderToStream(code, options = {}) { if (!disconnect) { failed = true; onFailed && onFailed(sink); - } + } else if (!sink && onDisconnectedBeforeShell) onDisconnectedBeforeShell(); }; // A retry pass that throws a REAL error (not NotReady) can have nothing on // the stack to catch it: the initial render pass throws synchronously out @@ -3270,12 +3280,18 @@ export function renderToStream(code, options = {}) { // A render failure (see `abandon`) ends the sink: it is still alive — // the RENDER died — and leaving it open would hang the response. Post- // shell through the live wrapper (coalesced bytes flush first); pre- - // shell nothing was written, end the raw sink. + // shell nothing was written: `createSSRResponse`'s sink answers it + // (`SHELL_ABANDONED`), any other raw sink is ended. + const abandoned = w[SHELL_ABANDONED]; onFailure(sink => { try { - sink ? sink.end() : w.end(); + sink ? sink.end() : abandoned ? abandoned() : w.end(); } catch (_) {} }); + if (abandoned) { + if (disconnected) abandoned(); + else onDisconnectedBeforeShell = abandoned; + } function flush() { allSettled(blockingPromises).then(awaited => { scheduleFlush(() => { @@ -6735,6 +6751,10 @@ export function createSSRResponse( * can only be honored client-side, so stream completion appends * `` for relative or HTTP(S) targets * (carrying `options.nonce` for strict `script-src` CSPs) before closing. + * A render that fails (`onError` hears `handling: "failed"`) or is + * aborted through its `signal` before the shell flushes has no page: the + * promise resolves with a bodyless 500 (a pre-flush `Location` still + * redirects) and the stub is committed. The promise never rejects. * * `options.transformChunk(chunk)` rewrites each outgoing HTML chunk (entry * script injection, doctype prefixes, ...). The default `content-type` is @@ -6776,7 +6796,7 @@ export function createSSRResponse(result, event, options = {}) { closed = true; } }; - result.pipe({ + const sink = { write(chunk) { if (!flushed) { flushed = true; @@ -6812,6 +6832,8 @@ export function createSSRResponse(result, event, options = {}) { enqueue(transformChunk ? transformChunk(chunk) : chunk); }, end() { + // An empty document's shell reaches the sink as no write at all. + if (!flushed) sink.write(""); if (closed || !controller) return; // A Location that appears here was written after the head went out // (a pre-flush one short-circuited above) — client-side is the only @@ -6830,8 +6852,19 @@ export function createSSRResponse(result, event, options = {}) { try { controller.close(); } catch {} + }, + [SHELL_ABANDONED]() { + if (flushed) return; + flushed = closed = true; + if (stub) commitResponseStub(stub, { event }); + const head = deriveHead(stub, responseInit); + // No page was produced: a pre-flush Location still preempts it, and + // otherwise a status set before the failure does not describe it. + const status = stub && stub.headers.get("Location") ? getExpectedRedirectStatus(stub) : 500; + resolve(new Response(null, { status, headers: head.headers })); } - }); + }; + result.pipe(sink); }); } /** * Composes fetch-style middleware into one function of the same shape; diff --git a/packages/web/test/server/ssr-async-rejection-3569.spec.tsx b/packages/web/test/server/ssr-async-rejection-3569.spec.tsx index 0537c4ecb..8ef68b7f2 100644 --- a/packages/web/test/server/ssr-async-rejection-3569.spec.tsx +++ b/packages/web/test/server/ssr-async-rejection-3569.spec.tsx @@ -38,8 +38,16 @@ * renderer's own retry failures to exactly that; a boundary-originated * failure gets the same treatment here. */ +import { getEventListeners } from "node:events"; import { describe, expect, test } from "vitest"; -import { renderToStream, Loading, Errored, type ServerErrorContext } from "@solidjs/web"; +import { + renderToStream, + createRequestEvent, + createSSRResponse, + Loading, + Errored, + type ServerErrorContext +} from "@solidjs/web"; import { NotReadyError, createMemo } from "solid-js"; import type { JSX } from "@solidjs/web"; import { hydrationRecordKeys } from "../harness/hydration-records.js"; @@ -335,46 +343,46 @@ describe("#3569 (a) a bare child's rejection routes like a template hole's", () }); }); -describe("#3569 (b) a render failure completes the consumer", () => { - // A failure that reaches `failRender` regardless of (a): a `` - // whose child plants a fresh pending source on every pass trips the - // boundary's convergence budget (#3003). That error is the boundary - // machinery's own — no hole, no bare child — so it takes `finalizeError`'s - // uncontained path: pre-flush, no parent handler → the request fails. - const NeverConverges = () => { - throw new NotReadyError(Promise.resolve() as any); - }; +// A failure that reaches `failRender` regardless of (a): a `` +// whose child plants a fresh pending source on every pass trips the +// boundary's convergence budget (#3003). That error is the boundary +// machinery's own — no hole, no bare child — so it takes `finalizeError`'s +// uncontained path: pre-flush, no parent handler → the request fails. +const NeverConverges = () => { + throw new NotReadyError(Promise.resolve() as any); +}; - /** - * The failing boundary, plus a root read that holds the shell until the - * failure has been reported — so for the piped forms the failure lands - * strictly pre-flush (post-flush a boundary's `done()` answers true and - * the fragment channel takes over; that shape already worked). - */ - function failingPreShell() { - let releaseShell!: () => void; - const gate = new Promise(r => (releaseShell = () => r("shell"))); - const { heard, onError } = recorder(); - const App = () => { - const held = createMemo(() => gate); - return ( -
- {held()} - loading}> - - -
- ); - }; - const options = { - onError(error: unknown, context: ServerErrorContext) { - onError(error, context); - setTimeout(releaseShell, 0); - } - }; - return { App, options, heard }; - } +/** + * The failing boundary, plus a root read that holds the shell until the + * failure has been reported — so for the piped forms the failure lands + * strictly pre-flush (post-flush a boundary's `done()` answers true and + * the fragment channel takes over; that shape already worked). + */ +function failingPreShell() { + let releaseShell!: () => void; + const gate = new Promise(r => (releaseShell = () => r("shell"))); + const { heard, onError } = recorder(); + const App = () => { + const held = createMemo(() => gate); + return ( +
+ {held()} + loading}> + + +
+ ); + }; + const options = { + onError(error: unknown, context: ServerErrorContext) { + onError(error, context); + setTimeout(releaseShell, 0); + } + }; + return { App, options, heard }; +} +describe("#3569 (b) a render failure completes the consumer", () => { test("await: resolves (with the HTML produced — none, pre-shell), one `failed` call, nothing leaks", async () => { const { App, options, heard } = failingPreShell(); const { escaped, value } = await watchRejections(() => @@ -414,3 +422,173 @@ describe("#3569 (b) a render failure completes the consumer", () => { expect(heard.map(h => h.context.handling)).toEqual(["failed"]); }, 10_000); }); + +// solidjs/solid#3719 — `createSSRResponse` over the same pre-shell failures: +// its promise resolves at shell flush, and a render that fails (or is +// aborted) before the shell has none. It settles anyway, the way every +// other consumer above completes: it resolves, never rejects (`onError` +// already heard the failure), with a bodyless 500 — no page was produced, +// and an empty 200 would be cached as one. +describe("#3719 createSSRResponse settles when the render ends before the shell", () => { + function respond( + code: () => any, + options: any, + event = createRequestEvent(new Request("http://localhost/")) + ) { + return settleOrHang(createSSRResponse(renderToStream(code, options), event), 2000); + } + async function settled(value: Settled) { + expect(value.settled).toBe(true); + const response = (value as { value: Response }).value; + return { response, body: await response.text() }; + } + + test("a failure the render reaches through a boundary (non-converging ) resolves a bodyless 500", async () => { + const { App, options, heard } = failingPreShell(); + const event = createRequestEvent(new Request("http://localhost/")); + const { escaped, value } = await watchRejections(() => respond(() => , options, event)); + expect(messages(escaped)).toEqual([]); + const { response, body } = await settled(value); + expect(response.status).toBe(500); + expect(body).toBe(""); + expect(response.headers.has("content-type")).toBe(false); + // The page exit: the stub comes back committed, so the handler edge's + // `commitEventResponse` passes it through instead of folding it again. + expect(event.response.committed).toBe(true); + expect(heard.map(h => h.context.handling)).toEqual(["failed"]); + }, 10_000); + + test("a sync throw on a root hole's retry pass resolves a bodyless 500", async () => { + const { heard, onError } = recorder(); + let first = true; + const App = () => ( +
+ { + (() => { + if (first) { + first = false; + throw new NotReadyError(Promise.resolve() as any); + } + throw new Error("boom on retry"); + }) as unknown as JSX.Element + } +
+ ); + const { escaped, value } = await watchRejections(() => respond(() => , { onError })); + expect(messages(escaped)).toEqual([]); + const { response, body } = await settled(value); + expect(response.status).toBe(500); + expect(body).toBe(""); + expect(heard.map(h => [h.context.handling, (h.error as Error).message])).toEqual([ + ["failed", "boom on retry"] + ]); + }); + + test("an async read that rejects with no boundary above it resolves a bodyless 500", async () => { + const { heard, onError } = recorder(); + const App = () => { + const data = useLateReject(); + return ( +
+

{data()}

+
+ ); + }; + const { escaped, value } = await watchRejections(() => respond(() => , { onError })); + expect(messages(escaped)).toEqual([]); + const { response, body } = await settled(value); + expect(response.status).toBe(500); + expect(body).toBe(""); + expect(heard.map(h => [h.context.handling, (h.error as Error).message])).toEqual([ + ["failed", "fetch failed"] + ]); + }); + + test("a Location already on the stub (set outside the render) still redirects", async () => { + // The pre-flush rule: a Location present before the head freezes + // preempts the page, and a failed page is no exception. + const { heard, onError } = recorder(); + const event = createRequestEvent(new Request("http://localhost/")); + event.response.headers.set("Location", "/login"); + event.response.headers.append("Set-Cookie", "a=1"); + const App = () => { + const data = useLateReject(); + return

{data()}

; + }; + const { value } = await watchRejections(() => respond(() => , { onError }, event)); + const { response, body } = await settled(value); + expect(response.status).toBe(302); + expect(response.headers.get("location")).toBe("/login"); + expect(response.headers.getSetCookie()).toEqual(["a=1"]); + expect(body).toBe(""); + expect(heard.map(h => h.context.handling)).toEqual(["failed"]); + }); + + test("an abort before the shell resolves a bodyless 500 without reporting a failure", async () => { + const { heard, onError } = recorder(); + const controller = new AbortController(); + const App = () => { + const held = createMemo(() => new Promise(() => {})); + return
{held()}
; + }; + const { escaped, value } = await watchRejections(() => { + const pending = respond(() => , { onError, signal: controller.signal }); + setTimeout(() => controller.abort(), 10); + return pending; + }); + expect(messages(escaped)).toEqual([]); + const { response, body } = await settled(value); + expect(response.status).toBe(500); + expect(body).toBe(""); + // A disconnect, not a render failure: the hook hears nothing. + expect(heard).toEqual([]); + expect(getEventListeners(controller.signal, "abort")).toHaveLength(0); + }); + + test("a signal already aborted when the render starts resolves too", async () => { + const { heard, onError } = recorder(); + const controller = new AbortController(); + controller.abort(); + const App = () => { + const held = createMemo(() => new Promise(() => {})); + return
{held()}
; + }; + const { value } = await watchRejections(() => + respond(() => , { onError, signal: controller.signal }) + ); + const { response } = await settled(value); + expect(response.status).toBe(500); + expect(heard).toEqual([]); + expect(getEventListeners(controller.signal, "abort")).toHaveLength(0); + }); + + test("a render that succeeds with nothing to write resolves an empty 200", async () => { + // The shell of an empty page reaches the sink as no write at all: `end()` + // alone is a success, not a failure — it answers like + // `createSSRResponse("")`. + const { heard, onError } = recorder(); + const { value } = await watchRejections(() => respond(() => null, { onError })); + const { response, body } = await settled(value); + expect(response.status).toBe(200); + expect(response.headers.get("content-type")).toBe("text/html; charset=utf-8"); + expect(body).toBe(""); + expect(heard).toEqual([]); + }); + + test("a rejection after the shell is unchanged: 200, the fallback streams, then the rejected fragment", async () => { + const { heard, onError } = recorder(); + const App = () => { + const data = useLateReject(); + return loading}>{data()}; + }; + const { escaped, value } = await watchRejections(() => respond(() => , { onError })); + expect(messages(escaped)).toEqual([]); + const { response, body } = await settled(value); + expect(response.status).toBe(200); + expect(response.headers.get("content-type")).toBe("text/html; charset=utf-8"); + expect(bare(body)).toContain("loading"); + expect(body).toContain("fetch failed"); + expect(hydrationRecordKeys(body).filter(k => k.endsWith("_fr"))).toHaveLength(1); + expect(heard.map(h => h.context.handling)).toEqual(["client"]); + }); +});