From 0abfefecd743ac9b86e77474fe686d4895c362ba Mon Sep 17 00:00:00 2001 From: Vladimir Kutepov Date: Sun, 30 Aug 2026 14:39:44 +0700 Subject: [PATCH 1/3] test(web): mark three open server-function gaps as expected failures Each is verified against `next` and stated as the behaviour that is wanted, so the suite stays green while the gap is open and turns red the day it closes. - A result the codec cannot encode is delivered as `undefined`. The function already ran; only the encoding failed, and it failed after the head was committed, so the status is spent and no error tag can be added. The truncated body decodes to the same answer a void function gives, so a write that succeeded is indistinguishable from one that returned nothing. - A streamed result has no backpressure: the stream is built with no `pull` and no queuing strategy, and every codec node is enqueued as soon as it is parsed. A consumer reading three chunks over 60ms left the producer 3695 items ahead; on a large or infinite stream one slow client buffers the whole result in server memory. - The decode depth cap guards the seroval path only, and the body format is chosen by the caller, so selecting the JSON format opts out of it: depth 5000 decodes where the capped path answers 400. `.fails` rather than the repo's `test.skip` idiom because the point is to notice the fix. Tests only; no runtime change, so no changeset. --- .../server-functions-open-gaps.spec.tsx | 176 ++++++++++++++++++ 1 file changed, 176 insertions(+) create mode 100644 packages/web/test/server/server-functions-open-gaps.spec.tsx diff --git a/packages/web/test/server/server-functions-open-gaps.spec.tsx b/packages/web/test/server/server-functions-open-gaps.spec.tsx new file mode 100644 index 000000000..7f87e8ef3 --- /dev/null +++ b/packages/web/test/server/server-functions-open-gaps.spec.tsx @@ -0,0 +1,176 @@ +/** + * Gaps that are open on `next` today, written as `it.fails` so the suite + * stays green while they are open and turns RED the day each is fixed — + * at which point the marker comes off and the test becomes an ordinary + * guard. (The repo's existing idiom for "intended, not yet held" is + * `test.skip`; these use `.fails` instead because the point is to notice + * the fix, which a skipped test cannot do.) + * + * Each assertion states the behaviour that is wanted, not the behaviour + * that happens. + * + * Like the other server-function specs, these run against the built + * bundles (server-functions/dist/*, wired up in vite.config.server.mjs). + */ +import { AsyncLocalStorage } from "node:async_hooks"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { + BODY_FORMAT_HEADER, + ERROR_HEADER, + handleServerFunctionRequest, + registerServerFunction +} from "@solidjs/web/server-functions/server"; +import { createServerReference } from "@solidjs/web/server-functions/client"; + +const RequestContext = Symbol.for("solid.RequestContext"); + +beforeAll(() => { + (globalThis as any)[RequestContext] = new AsyncLocalStorage(); +}); + +afterAll(() => { + delete (globalThis as any)[RequestContext]; +}); + +function scriptedPost(id: string) { + return new Request(`https://app.example/_server/data/${id}`, { + method: "POST", + body: "[]", + headers: { + "Sec-Fetch-Site": "same-origin", + "X-Server-Function-Instance": "server-function:test" + } + }); +} + +/** + * Routes the client stub through the handler the way a socket does: the + * body is drained into a buffer first, so a stream that errors mid-flight + * reaches the client as a TRUNCATED body rather than as a live exception. + * That is the difference between an in-process call and a deployed one. + */ +function connectWire() { + const original = globalThis.fetch; + globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => { + const address = input instanceof Request ? input.url : input.toString(); + const request = new Request(new URL(address, "https://app.example"), init); + request.headers.set("Sec-Fetch-Site", "same-origin"); + const response = await handleServerFunctionRequest(request); + + const chunks: Uint8Array[] = []; + try { + for await (const chunk of response.body ?? []) chunks.push(chunk as Uint8Array); + } catch { + /* the wire cut here; whatever arrived is what the client sees */ + } + return new Response(chunks.length ? Buffer.concat(chunks) : null, { + status: response.status, + headers: response.headers + }); + }) as typeof fetch; + return () => { + globalThis.fetch = original; + }; +} + +describe("a result the codec cannot encode", () => { + // The function already ran and committed its side effects; only the + // ENCODING failed, and it failed after the head was committed, so the + // status is spent and no error tag can be added. The truncated body + // then decodes to `undefined` — the same answer a void function gives. + // A caller cannot tell "this mutation returned nothing" from "this + // mutation's result was lost", which is the worst possible reading of a + // write that succeeded. + it.fails("reaches the caller as a failure rather than as undefined", async () => { + let ran = 0; + registerServerFunction("gap-encode-failure", async () => { + ran++; + return { + ok: true, + get unencodable() { + throw new Error("cannot encode"); + } + }; + }); + + const disconnect = connectWire(); + try { + await expect(createServerReference("gap-encode-failure")()).rejects.toThrow(); + expect(ran).toBe(1); + } finally { + disconnect(); + } + }); +}); + +describe("a streamed result with a consumer that reads slowly", () => { + // The response stream is built with no `pull` and no queuing strategy, + // and every codec node is enqueued the moment it is parsed, so the + // producer runs as fast as it can resolve regardless of whether anyone + // is reading. One slow client on a large or infinite stream therefore + // buffers the whole result in server memory — invisible to application + // code, and unbounded. + it.fails("does not let the producer run unboundedly ahead", async () => { + let produced = 0; + registerServerFunction("gap-backpressure", async function* () { + while (produced < 100_000) { + produced++; + yield { n: produced }; + await new Promise(resolve => setImmediate(resolve)); + } + }); + + const response = await handleServerFunctionRequest(scriptedPost("gap-backpressure")); + const reader = response.body!.getReader(); + for (let i = 0; i < 3; i++) { + await reader.read(); + await new Promise(resolve => setTimeout(resolve, 20)); + } + await reader.cancel(); + + // A generous ceiling: a bounded producer stays near the queue size, + // an unbounded one reaches five figures in this window. + expect(produced).toBeLessThan(500); + }); +}); + +describe("the decode depth cap", () => { + // The codec's `depthLimit: 64` exists "because payloads may come from an + // untrusted peer", and it guards the seroval path only. The body format + // is chosen by the CALLER, so selecting the JSON format opts out of the + // cap entirely: `extractBody` hands the payload to a bare JSON.parse. + it.fails("holds whichever body format the caller selects", async () => { + registerServerFunction("gap-depth", async (value: unknown) => { + let depth = 0; + let cursor: any = value; + while (cursor && typeof cursor === "object" && "a" in cursor) { + depth++; + cursor = cursor.a; + } + return { depth }; + }); + + const root: any = {}; + let cursor = root; + for (let i = 0; i < 5_000; i++) { + cursor.a = {}; + cursor = cursor.a; + } + + const response = await handleServerFunctionRequest( + new Request("https://app.example/_server/data/gap-depth", { + method: "POST", + body: JSON.stringify([root]), + headers: { + "Sec-Fetch-Site": "same-origin", + "X-Server-Function-Instance": "server-function:test", + [BODY_FORMAT_HEADER]: "8" + } + }) + ); + + // the seroval path answers 400 for a payload past the cap + expect(response.status).toBe(400); + expect(response.headers.has(ERROR_HEADER)).toBe(false); + }); +}); From c9cb240daeb774207a7836fb60a1af3531c6130e Mon Sep 17 00:00:00 2001 From: Vladimir Kutepov Date: Sun, 30 Aug 2026 15:06:04 +0700 Subject: [PATCH 2/3] test(web): fix the depth gap's header, and make two assertions honest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of the first commit turned up three things, one of which made a test worthless: - BODY_FORMAT_HEADER is not exported from the server entry, so the import was `undefined` and the request carried a header literally named "undefined". The JSON format was never selected and the function received no argument at all: the handler answered {"depth":0} where the gap needs {"depth":500}. The test failed, but not for its own reason — exactly the failure mode `.fails` cannot show you. Local const, as `server-functions-failure-signal.spec.tsx` already does. - Depth 5000 sat on the repo's own cliff (shared.ts notes ~5900 nested objects overflow V8's default stack on CI). 500 is comfortably past the 64-level cap and nowhere near it. - The backpressure ceiling was wall-clock, and under `.fails` a starved CI that produced fewer than 500 would have turned red for no reason. Counted in event-loop turns instead: bounded stays near the queue size on any machine, unbounded tracks the turn count. Also adopts MATRIX.md's spelling (`test.fails` with a `// GAP:` comment), which is the repo's documented idiom for this and which the first commit wrongly described as absent, and drops a dead assertion after rejects.toThrow(). --- .../server-functions-open-gaps.spec.tsx | 102 +++++++++--------- 1 file changed, 53 insertions(+), 49 deletions(-) diff --git a/packages/web/test/server/server-functions-open-gaps.spec.tsx b/packages/web/test/server/server-functions-open-gaps.spec.tsx index 7f87e8ef3..43b5ee2d7 100644 --- a/packages/web/test/server/server-functions-open-gaps.spec.tsx +++ b/packages/web/test/server/server-functions-open-gaps.spec.tsx @@ -1,28 +1,23 @@ /** - * Gaps that are open on `next` today, written as `it.fails` so the suite - * stays green while they are open and turns RED the day each is fixed — - * at which point the marker comes off and the test becomes an ordinary - * guard. (The repo's existing idiom for "intended, not yet held" is - * `test.skip`; these use `.fails` instead because the point is to notice - * the fix, which a skipped test cannot do.) - * - * Each assertion states the behaviour that is wanted, not the behaviour - * that happens. + * Gaps that are open on `next` today. Each test states the behaviour that + * is wanted and is marked `test.fails`, per the convention in + * `test/lifecycle-matrix/MATRIX.md`: the marker is the point — the suite + * stays green while the gap is open and turns red the day it closes, at + * which point the marker comes off and the test becomes an ordinary guard. * * Like the other server-function specs, these run against the built * bundles (server-functions/dist/*, wired up in vite.config.server.mjs). */ import { AsyncLocalStorage } from "node:async_hooks"; -import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { afterAll, beforeAll, describe, expect, test } from "vitest"; import { - BODY_FORMAT_HEADER, - ERROR_HEADER, handleServerFunctionRequest, registerServerFunction } from "@solidjs/web/server-functions/server"; import { createServerReference } from "@solidjs/web/server-functions/client"; const RequestContext = Symbol.for("solid.RequestContext"); +const BODY_FORMAT_HEADER = "X-Server-Function-Format"; beforeAll(() => { (globalThis as any)[RequestContext] = new AsyncLocalStorage(); @@ -46,10 +41,11 @@ function scriptedPost(id: string) { /** * Routes the client stub through the handler the way a socket does: the * body is drained into a buffer first, so a stream that errors mid-flight - * reaches the client as a TRUNCATED body rather than as a live exception. - * That is the difference between an in-process call and a deployed one. + * arrives as a TRUNCATED body rather than as a live exception. That is the + * difference between an in-process call and a deployed one, and it is the + * difference this gap hides behind. */ -function connectWire() { +function connectBufferedTransport() { const original = globalThis.fetch; globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => { const address = input instanceof Request ? input.url : input.toString(); @@ -74,14 +70,13 @@ function connectWire() { } describe("a result the codec cannot encode", () => { - // The function already ran and committed its side effects; only the - // ENCODING failed, and it failed after the head was committed, so the - // status is spent and no error tag can be added. The truncated body - // then decodes to `undefined` — the same answer a void function gives. - // A caller cannot tell "this mutation returned nothing" from "this - // mutation's result was lost", which is the worst possible reading of a - // write that succeeded. - it.fails("reaches the caller as a failure rather than as undefined", async () => { + // GAP: the caller receives `undefined`. The function already ran and + // committed its side effects; only the ENCODING failed, and it failed + // after the head was committed, so the status is spent and no error tag + // can be added. The truncated body decodes to the answer a void function + // gives, so a write that succeeded is indistinguishable from one that + // returned nothing — and a data layer may retry it. + test.fails("reaches the caller as a failure rather than as undefined", async () => { let ran = 0; registerServerFunction("gap-encode-failure", async () => { ran++; @@ -93,24 +88,34 @@ describe("a result the codec cannot encode", () => { }; }); - const disconnect = connectWire(); + const restore = connectBufferedTransport(); + let outcome: { resolved: true; value: unknown } | { resolved: false; error: unknown }; try { - await expect(createServerReference("gap-encode-failure")()).rejects.toThrow(); - expect(ran).toBe(1); + outcome = { resolved: true, value: await createServerReference("gap-encode-failure")() }; + } catch (error) { + outcome = { resolved: false, error }; } finally { - disconnect(); + restore(); } + + expect(ran).toBe(1); + expect(outcome.resolved).toBe(false); + expect((outcome as { error: unknown }).error).toBeInstanceOf(Error); }); }); -describe("a streamed result with a consumer that reads slowly", () => { - // The response stream is built with no `pull` and no queuing strategy, - // and every codec node is enqueued the moment it is parsed, so the - // producer runs as fast as it can resolve regardless of whether anyone - // is reading. One slow client on a large or infinite stream therefore - // buffers the whole result in server memory — invisible to application - // code, and unbounded. - it.fails("does not let the producer run unboundedly ahead", async () => { +describe("a streamed result nobody is reading", () => { + // GAP: the producer runs unboundedly ahead. The response stream is built + // with no `pull` and no queuing strategy, and every codec node is + // enqueued the moment it is parsed, so the producer runs as fast as it + // can resolve whether or not anyone reads. On a large or infinite stream + // one slow client buffers the whole result in server memory, invisibly + // to application code. + // + // Counted in event-loop turns rather than wall-clock: a bounded producer + // stays near the queue size whatever the machine, an unbounded one + // tracks the turn count. + test.fails("does not let the producer run ahead of the consumer", async () => { let produced = 0; registerServerFunction("gap-backpressure", async function* () { while (produced < 100_000) { @@ -122,24 +127,22 @@ describe("a streamed result with a consumer that reads slowly", () => { const response = await handleServerFunctionRequest(scriptedPost("gap-backpressure")); const reader = response.body!.getReader(); - for (let i = 0; i < 3; i++) { - await reader.read(); - await new Promise(resolve => setTimeout(resolve, 20)); + await reader.read(); + for (let turn = 0; turn < 200; turn++) { + await new Promise(resolve => setImmediate(resolve)); } await reader.cancel(); - // A generous ceiling: a bounded producer stays near the queue size, - // an unbounded one reaches five figures in this window. - expect(produced).toBeLessThan(500); + expect(produced).toBeLessThan(50); }); }); describe("the decode depth cap", () => { - // The codec's `depthLimit: 64` exists "because payloads may come from an - // untrusted peer", and it guards the seroval path only. The body format - // is chosen by the CALLER, so selecting the JSON format opts out of the - // cap entirely: `extractBody` hands the payload to a bare JSON.parse. - it.fails("holds whichever body format the caller selects", async () => { + // GAP: the cap is opt-out. `depthLimit: 64` exists "because payloads may + // come from an untrusted peer" and guards the seroval path only, while + // the body format is chosen by the CALLER — selecting the JSON format + // hands the payload to a bare JSON.parse and skips the cap entirely. + test.fails("holds whichever body format the caller selects", async () => { registerServerFunction("gap-depth", async (value: unknown) => { let depth = 0; let cursor: any = value; @@ -150,9 +153,11 @@ describe("the decode depth cap", () => { return { depth }; }); + // comfortably past the 64-level cap, and well short of the ~5900 + // nested objects that overflow V8's default stack in JSON.stringify const root: any = {}; let cursor = root; - for (let i = 0; i < 5_000; i++) { + for (let i = 0; i < 500; i++) { cursor.a = {}; cursor = cursor.a; } @@ -169,8 +174,7 @@ describe("the decode depth cap", () => { }) ); - // the seroval path answers 400 for a payload past the cap + // the capped path answers 400 for a payload past the limit expect(response.status).toBe(400); - expect(response.headers.has(ERROR_HEADER)).toBe(false); }); }); From c391375aff6b3a7e672097d240cadd7cd01cd1bf Mon Sep 17 00:00:00 2001 From: Vladimir Kutepov Date: Sun, 30 Aug 2026 15:55:58 +0700 Subject: [PATCH 3/3] test(web): point each gap marker at the issue tracking it MATRIX.md's convention is a `// GAP:` comment; naming the issue in it means whoever closes one finds the test that turns red. --- .../web/test/server/server-functions-open-gaps.spec.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/web/test/server/server-functions-open-gaps.spec.tsx b/packages/web/test/server/server-functions-open-gaps.spec.tsx index 43b5ee2d7..8e202b4a2 100644 --- a/packages/web/test/server/server-functions-open-gaps.spec.tsx +++ b/packages/web/test/server/server-functions-open-gaps.spec.tsx @@ -4,6 +4,7 @@ * `test/lifecycle-matrix/MATRIX.md`: the marker is the point — the suite * stays green while the gap is open and turns red the day it closes, at * which point the marker comes off and the test becomes an ordinary guard. + * Each carries the issue that tracks it: #3117, #3118, #3119. * * Like the other server-function specs, these run against the built * bundles (server-functions/dist/*, wired up in vite.config.server.mjs). @@ -70,7 +71,7 @@ function connectBufferedTransport() { } describe("a result the codec cannot encode", () => { - // GAP: the caller receives `undefined`. The function already ran and + // GAP (#3117): the caller receives `undefined`. The function already ran and // committed its side effects; only the ENCODING failed, and it failed // after the head was committed, so the status is spent and no error tag // can be added. The truncated body decodes to the answer a void function @@ -105,7 +106,7 @@ describe("a result the codec cannot encode", () => { }); describe("a streamed result nobody is reading", () => { - // GAP: the producer runs unboundedly ahead. The response stream is built + // GAP (#3118): the producer runs unboundedly ahead. The response stream is built // with no `pull` and no queuing strategy, and every codec node is // enqueued the moment it is parsed, so the producer runs as fast as it // can resolve whether or not anyone reads. On a large or infinite stream @@ -138,7 +139,7 @@ describe("a streamed result nobody is reading", () => { }); describe("the decode depth cap", () => { - // GAP: the cap is opt-out. `depthLimit: 64` exists "because payloads may + // GAP (#3119): the cap is opt-out. `depthLimit: 64` exists "because payloads may // come from an untrusted peer" and guards the seroval path only, while // the body format is chosen by the CALLER — selecting the JSON format // hands the payload to a bare JSON.parse and skips the cap entirely.