diff --git a/.changeset/frame-error-not-cached.md b/.changeset/frame-error-not-cached.md new file mode 100644 index 000000000..294204693 --- /dev/null +++ b/.changeset/frame-error-not-cached.md @@ -0,0 +1,7 @@ +--- +"@solidjs/web": patch +--- + +A server component that throws during its synchronous render is no longer cacheable. `respond(View, { headers })` puts those headers on the frame response, and the response is a 200 whose body carries the error, so an author's `cache-control: public, max-age` stored the failure. When the render fails before the head is built, the frame response sends `Cache-Control: no-store` and drops the CDN-targeted fields that would override it at a CDN (`CDN-Cache-Control`, `-CDN-Cache-Control`, `Surrogate-Control`). The status stays 200 and the body still carries the error record. A render that succeeds keeps the author's headers, and so does a returned `{ error }` value: the author chose that value and the headers that travel with it. A failure that lands after a suspended shell has already left can no longer change the head. + +`@solidjs/web/frames/server` no longer exports `renderServerComponent`, `renderToFrameStream`, `serverComponentResponse` or `createFrameSink`. They were internal building blocks: a server component reaches the client by being returned from a server function (directly or wrapped in `respond()`), and the transform hooks (`frameTransformResult`, `frameTransformFlightResult`, `frameTransformDirectResult`) render and frame it. The entry is experimental, and the hooks and document-shell exports are unchanged. diff --git a/packages/diagnostics/README.md b/packages/diagnostics/README.md index a94c6d7ad..5695c2acc 100644 --- a/packages/diagnostics/README.md +++ b/packages/diagnostics/README.md @@ -57,7 +57,7 @@ artifact.records.invocation; // every server-function execution: id, durationMs, artifact.records.frame; // every frame stream produced (side: "server"): id, shellMs, durationMs, outcome, the chunk census ``` -`boundary` is one row per `` boundary that **waited** (a boundary that rendered on its first pass has nothing to attribute): how long it held its content up (`durationMs`), how long finished content then sat behind `` siblings (`heldMs`), how many render passes it took (`passes` — `2` is one round of async, more is a sequential chain) and how it ended (`outcome`: settled, the `renderToString` fallback, a client-only handoff, an error). `invocation` is one row per server-function execution; a direct call made during a boundary's pass carries that boundary's `id` in `boundary`, so a boundary's wait reads as the calls it consisted of. `frame` is one row per frame stream a server component rendered to (`renderServerComponent`, or a server-function response through `frameTransformResult`): time to the shell (`shellMs`) and to `complete` (`durationMs`), how it ended, and a census of what the stream carried (`fragments`, `slots`, `regions`, `errors`); a server-function response that is a frame stream has an invocation row and a frame row with the same `id`. The runtime derives two dev checks from the same facts — `SSR_BOUNDARY_WATERFALL` for a sequential chain of waits and `SSR_CLIENT_CONTENT_MASKED` for client-only content that surfaced only after a wait — so `expectNoDiagnostics` catches them without reading the tables. +`boundary` is one row per `` boundary that **waited** (a boundary that rendered on its first pass has nothing to attribute): how long it held its content up (`durationMs`), how long finished content then sat behind `` siblings (`heldMs`), how many render passes it took (`passes` — `2` is one round of async, more is a sequential chain) and how it ended (`outcome`: settled, the `renderToString` fallback, a client-only handoff, an error). `invocation` is one row per server-function execution; a direct call made during a boundary's pass carries that boundary's `id` in `boundary`, so a boundary's wait reads as the calls it consisted of. `frame` is one row per frame stream a server component rendered to (a server-function response through `frameTransformResult`): time to the shell (`shellMs`) and to `complete` (`durationMs`), how it ended, and a census of what the stream carried (`fragments`, `slots`, `regions`, `errors`); a server-function response that is a frame stream has an invocation row and a frame row with the same `id`. The runtime derives two dev checks from the same facts — `SSR_BOUNDARY_WATERFALL` for a sequential chain of waits and `SSR_CLIENT_CONTENT_MASKED` for client-only content that surfaced only after a wait — so `expectNoDiagnostics` catches them without reading the tables. In the browser (the in-process capture in a jsdom test, or the bridge under Playwright) the same tables hold the page's **requests**: `artifact.records.call` is one row per server-function call the client made — `id`, `method`, `durationMs` (the whole wait the caller saw), `outcome`, `status` — and `artifact.records.frame` rows with `side: "client"` are the frame streams it applied (`address` is the local boundary the stream was remapped onto; `outcome` adds `truncated` for a body that ended early). A `call` and the server's `invocation` of the same `id` differ by the wire; a `frame` seen from both sides joins by `id` and `version`. `artifact.records.recovery` is the client's side of a server hand-off: one row per `` boundary that rendered its children as fresh client DOM because the server could not produce the fragment (`outcome: "client"` on the server's `boundary` row of the same `id`) — `waitedMs` is the fallback the person looked at while the server was still trying, `renderMs` the fresh render. The tables are always present; one is empty when nothing of its kind happened. Their row types are the runtimes' own — `BoundaryEvent`/`RecoveryEvent` from `solid-js`, `InvocationEvent`/`FrameEvent`/`CallEvent` from `@solidjs/web` — which is why those two packages are (type-only) peers of this one. JSONL egress adds one line per record, `type` naming its table. diff --git a/packages/web/frames/src/frame-sink.ts b/packages/web/frames/src/frame-sink.ts index 406f0c550..9a2add4b1 100644 --- a/packages/web/frames/src/frame-sink.ts +++ b/packages/web/frames/src/frame-sink.ts @@ -2822,6 +2822,10 @@ export function serverComponentResponse(component, options = {}, init = {}) { // The live body's heartbeat and dev chaos (see armLiveBody); disarmed on // every road the body ends by. let stopLive = null; + // The frame itself failed as one value (an unkeyed `error` record; a keyed + // one is a fragment's diagnostic). Seen during the sync pass, it decides + // the head below. + let failed = false; const body = new ReadableStream({ start(controller) { const end = () => { @@ -2849,6 +2853,7 @@ export function serverComponentResponse(component, options = {}, init = {}) { }); stream.pipe({ write(chunk) { + if (chunk.type === "error" && !chunk.key) failed = true; if (closed) return; try { controller.enqueue(frame(JSON.stringify(chunk))); @@ -2877,6 +2882,20 @@ export function serverComponentResponse(component, options = {}, init = {}) { teardown.abort({ [DISCONNECTED]: true }); } }); + // The author's cache headers arrived with the head (`respond()`) and + // describe a success. A sync render failure is a 200 whose body is the + // `:error` record, seen above before this head exists: declare it + // uncacheable, for every cache. A CDN-targeted field (`CDN-Cache-Control`, + // `-CDN-Cache-Control`, RFC 9213) or `Surrogate-Control` overrides + // `Cache-Control` at the CDN, so those go too. Status stays 200: the client + // reads the frame stream, and a 4xx/5xx without the body-format header is + // a transport failure instead of the component's error. A failure after + // this, once a suspended shell has to leave, can no longer change the head. + if (failed) { + headers.set("Cache-Control", "no-store"); + for (const name of [...headers.keys()]) + if (name === "surrogate-control" || name.endsWith("cdn-cache-control")) headers.delete(name); + } return new Response(body, { status: init.status || 200, headers }); } diff --git a/packages/web/frames/src/server.ts b/packages/web/frames/src/server.ts index 57fcf08ce..56638815e 100644 --- a/packages/web/frames/src/server.ts +++ b/packages/web/frames/src/server.ts @@ -138,14 +138,10 @@ export function asyncArg(value: PromiseLike | AsyncIterable): T { } export { - renderToFrameStream, - renderServerComponent, - serverComponentResponse, frameTransformResult, // Single-flight: mutations whose invalidated payload includes markup // stream it as regions in one response (install as transformFlightResult) frameTransformFlightResult, - createFrameSink, // Document SSR (t=0): inline rendering + the hydration reference frameTransformDirectResult, ServerComponentPlugin, diff --git a/packages/web/test/dist-frames-server-instance.spec.ts b/packages/web/test/dist-frames-server-instance.spec.ts index f447b6379..53bd8dc65 100644 --- a/packages/web/test/dist-frames-server-instance.spec.ts +++ b/packages/web/test/dist-frames-server-instance.spec.ts @@ -172,12 +172,16 @@ describe("a single-flight server-component call carries its invocation id (#3641 FRAME_STREAM_HEADER, frameTransformDirectResult, frameTransformFlightResult, - frameTransformResult, - renderServerComponent + frameTransformResult } from "@solidjs/web/frames/server"; import { provideRequestEvent } from "@solidjs/web/storage"; createServerReference(registerServerReference("view-1", async () => () => "view markup")); + createServerReference( + registerServerReference("view-select", async () => () => + ssr('') + ) + ); let handlerSawInvocation; configureServerFunctionsServer({ @@ -193,32 +197,33 @@ describe("a single-flight server-component call carries its invocation id (#3641 collectFlightData: async () => ({ "route-data[]": { title: "x" } }) }); - const res = await handleServerFunctionRequest( - new Request("http://localhost/_server/data/view-1", { - method: "POST", - headers: { - "content-type": "application/json", - origin: "http://localhost", - "sec-fetch-site": "same-origin", - "x-server-function-format": "3", - "x-single-flight": "true" - }, - body: "[]" - }) - ); - const text = await res.text(); + const call = (id, singleFlight) => + handleServerFunctionRequest( + new Request("http://localhost/_server/data/" + id, { + method: "POST", + headers: { + "content-type": "application/json", + origin: "http://localhost", + "sec-fetch-site": "same-origin", + "x-server-function-format": "3", + ...(singleFlight ? { "x-single-flight": "true" } : {}) + }, + body: "[]" + }) + ); // Length-prefixed frames: \`;0x<8 hex>;\` + JSON. - const chunks = text - .split(/;0x[0-9a-f]{8};/) - .filter(Boolean) - .map(s => JSON.parse(s)) - .map(({ type, id, html }) => ({ type, id, html })); + const chunksOf = async res => + (await res.text()) + .split(/;0x[0-9a-f]{8};/) + .filter(Boolean) + .map(s => JSON.parse(s)) + .map(({ type, id, html }) => ({ type, id, html })); + + const res = await call("view-1", true); + const chunks = await chunksOf(res); ssrSelectValues(); - const frame = await renderServerComponent( - () => ssr(''), - { frame: { id: "f", version: 1 } } - ); + const frame = await chunksOf(await call("view-select", false)); process.stdout.write( JSON.stringify({ diff --git a/packages/web/test/server/frame-error-not-cached.spec.tsx b/packages/web/test/server/frame-error-not-cached.spec.tsx new file mode 100644 index 000000000..4b019d1a3 --- /dev/null +++ b/packages/web/test/server/frame-error-not-cached.spec.tsx @@ -0,0 +1,115 @@ +/** + * @jsxImportSource @solidjs/web + * + * A server component wrapped in `respond(View, { headers })` contributes + * those headers to the frame response. The component renders while the + * response is being built, so a throw in that pass is known before the head + * leaves, and the frame response is declared uncacheable: + * `no-store`, with the CDN-targeted fields that would override it at a CDN + * removed. The status stays 200 and the body still carries the `:error` + * record — that is how the client surfaces it. A render that succeeds keeps + * the author's headers, and so does a returned `{ error }` value: the author + * chose that value and the headers that travel with it. + */ +import { AsyncLocalStorage } from "node:async_hooks"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { respond } from "@solidjs/web"; +import { frameTransformResult } from "../../frames/src/frame-sink.js"; +import { + GET, + createServerReference, + handleServerFunctionRequest, + registerServerReference +} from "../../server-functions/src/server.js"; +import { ChunkReader } from "../../server-functions/src/shared.js"; + +const RequestContext = Symbol.for("solid.RequestContext"); +beforeAll(() => { + (globalThis as any)[RequestContext] = new AsyncLocalStorage(); +}); +afterAll(() => { + delete (globalThis as any)[RequestContext]; +}); + +const handle = (request: Request) => + handleServerFunctionRequest(request, { transformResult: frameTransformResult }); + +const cacheable = { headers: { "cache-control": "public, max-age=60" } }; +const cdnCacheable = { + headers: { + "cache-control": "public, max-age=60", + "cdn-cache-control": "max-age=600", + "cloudflare-cdn-cache-control": "max-age=600", + "surrogate-control": "max-age=600", + vary: "Accept-Language" + } +}; +const cdnFields = ["cdn-cache-control", "cloudflare-cdn-cache-control", "surrogate-control"]; + +function declare(id: string, fn: () => unknown) { + return GET(createServerReference(registerServerReference(id, fn))); +} + +const get = (id: string) => new Request(`http://localhost/_server/data/${id}`, { method: "GET" }); + +async function chunksOf(response: Response) { + const reader: any = new ChunkReader(response.body!); + const out: any[] = []; + await reader.drain((data: string) => out.push(JSON.parse(data))); + return out; +} + +describe("a thrown server component is not cached behind respond() headers", () => { + it("a sync throw answers 200, no-store, with the error record still in the body", async () => { + declare("frame-error-cache-throw", async () => { + const user: { id: string } | null = null; + return respond(() =>

User : {user!.id}

, cacheable); + }); + const response = await handle(get("frame-error-cache-throw")); + expect(response.status).toBe(200); + expect(response.headers.get("cache-control")).toBe("no-store"); + expect(response.headers.get("X-Frame-Stream")).toBe("frame-error-cache-throw"); + const chunks = await chunksOf(response); + const error = chunks.find(c => c.type === "error" && !c.key); + expect(error).toBeTruthy(); + expect(String(error.error)).toContain("id"); + }); + + it("a render that succeeds keeps the author's Cache-Control", async () => { + declare("frame-error-cache-ok", async () => respond(() =>

User : ada

, cacheable)); + const response = await handle(get("frame-error-cache-ok")); + expect(response.status).toBe(200); + expect(response.headers.get("cache-control")).toBe("public, max-age=60"); + const chunks = await chunksOf(response); + expect(chunks.map(c => c.type)).toEqual(["start", "html", "complete"]); + }); + + it("a returned { error } value keeps the author's Cache-Control", async () => { + declare("frame-error-cache-value", async () => respond({ error: "nope" }, cacheable)); + const response = await handle(get("frame-error-cache-value")); + expect(response.status).toBe(200); + expect(response.headers.get("cache-control")).toBe("public, max-age=60"); + expect(response.headers.get("X-Frame-Stream")).toBeNull(); + expect(await response.json()).toEqual({ error: "nope" }); + }); + + it("a sync throw also drops the CDN-targeted fields that would override no-store", async () => { + declare("frame-error-cache-cdn-throw", async () => { + const user: { id: string } | null = null; + return respond(() =>

User : {user!.id}

, cdnCacheable); + }); + const response = await handle(get("frame-error-cache-cdn-throw")); + expect(response.headers.get("cache-control")).toBe("no-store"); + for (const name of cdnFields) expect(response.headers.get(name), name).toBeNull(); + expect(response.headers.get("vary")).toBe("Accept-Language"); + }); + + it("a render that succeeds keeps the CDN-targeted fields", async () => { + declare("frame-error-cache-cdn-ok", async () => + respond(() =>

User : ada

, cdnCacheable) + ); + const response = await handle(get("frame-error-cache-cdn-ok")); + expect(response.headers.get("cache-control")).toBe("public, max-age=60"); + for (const name of cdnFields) expect(response.headers.get(name), name).toBe("max-age=600"); + }); +}); diff --git a/packages/web/test/server/ssr-error-sanitization.fixture.mjs b/packages/web/test/server/ssr-error-sanitization.fixture.mjs index 5fd39ccde..b9341edba 100644 --- a/packages/web/test/server/ssr-error-sanitization.fixture.mjs +++ b/packages/web/test/server/ssr-error-sanitization.fixture.mjs @@ -16,7 +16,14 @@ import { renderToString, ssr } from "@solidjs/web"; -import { renderServerComponent } from "@solidjs/web/frames/server"; +import { frameTransformResult } from "@solidjs/web/frames/server"; +import { + configureServerFunctionsServer, + createServerReference, + handleServerFunctionRequest, + registerServerReference +} from "@solidjs/web/server-functions/server"; +import { provideRequestEvent } from "@solidjs/web/storage"; import { OBSERVE, createMemo } from "solid-js"; const delay = ms => new Promise(r => setTimeout(r, ms)); @@ -169,25 +176,50 @@ results.value = await withFindings(() => }) ); -// 6. Frame streams: a synchronous root failure's error chunk, and a failing -// fragment's keyed chunks — the fragment's, and the live hole's: a thunk -// hole (`() => escape(...)`, the compiled shape of a dynamic expression in -// a server component) whose re-evaluation throws. +// 6. Frame streams, served as an app serves them (a server function returning +// a view, through the handler's frame transform): a synchronous root +// failure's error chunk, and a failing fragment's keyed chunks — the +// fragment's, and the live hole's: a thunk hole (`() => escape(...)`, the +// compiled shape of a dynamic expression in a server component) whose +// re-evaluation throws. { - const boom = databaseError(); - results.frameRoot = await withFindings(async () => { - const chunks = await renderServerComponent( - () => { - throw boom; - }, - { frame: { id: "f-err" } } - ); - return chunks; + configureServerFunctionsServer({ + provideEvent: provideRequestEvent, + transformResult: frameTransformResult }); + const frameChunks = async id => { + const res = await handleServerFunctionRequest( + new Request(`http://localhost/_server/data/${id}`, { + method: "POST", + headers: { + "content-type": "application/json", + origin: "http://localhost", + "sec-fetch-site": "same-origin", + "x-server-function-format": "3" + }, + body: "[]" + }) + ); + // Length-prefixed frames: `;0x<8 hex>;` + JSON. + return (await res.text()) + .split(/;0x[0-9a-f]{8};/) + .filter(Boolean) + .map(s => JSON.parse(s)); + }; + + const boom = databaseError(); + createServerReference( + registerServerReference("f-err", async () => () => { + throw boom; + }) + ); + results.frameRoot = await withFindings(() => frameChunks("f-err")); + const boom2 = databaseError(); - results.frameFragment = await withFindings(async () => { - const chunks = await renderServerComponent( - () => + createServerReference( + registerServerReference( + "f-frag", + async () => () => ssr( ["
", "
"], loading(() => { @@ -197,11 +229,10 @@ results.value = await withFindings(() => }); return ssr(["

", "

"], () => escape(data())); }) - ), - { frame: { id: "f-frag" } } - ); - return chunks; - }); + ) + ) + ); + results.frameFragment = await withFindings(() => frameChunks("f-frag")); } // 7. The server error hook (C6): the ambient hook hears the 's