fix(web): fail a call on a response the runtime did not write (#3087)

Only the protocol's error header and a 5xx counted as failure, so every other
non-2xx was decoded as a result — and decoding a login page, or an empty 405,
yields nothing. The call resolved to `undefined`, reading as "the function
returned nothing" where the request had not reached it.

A response at 400 or above carrying neither the error header nor a body format
now fails the call with the status on the error. Undecoded, because its body
is someone else's; and before the passthrough that control flow uses, because
a foreign refusal carries a `Location` of its own often enough — an SSO
interstitial is exactly that — that the passthrough would have swallowed it.

`BodyFormat.Void` marks the one response the runtime encodes without a format
to carry, so a void result with a status stays a result, matching what the
equivalent bare `Response` already did.

Author: frenzzy

.changeset/transport-failure-on-foreign-response.md

@@ -0,0 +1,9 @@
+---
+"@solidjs/web": patch
+---
+
+Fail a server function call on a response the runtime did not write, instead of resolving it to `undefined` (#3087).
+
+Only the protocol's error header and a 5xx counted as failure, so every other non-2xx was decoded as a result — and decoding a login page, or an empty 405, yields nothing. A response at 400 or above carrying neither the error header nor a body format now fails the call with the status on the error, undecoded, and before the passthrough control flow uses: a foreign refusal carries a `Location` of its own often enough that the passthrough would have swallowed it.
+
+One runtime-produced shape is caught with them: a verbatim `X-Content-Raw` response at a non-2xx status, whose body a call site without an integration could not read anyway — an integration's `responseHandler` claims the response before the check. Otherwise the runtime's own responses are unaffected. `BodyFormat.Void` marks the one it encoded without a format to carry, so `respond(undefined, { status: 400 })`, `new Response(null, { status: 404 })` and `respond(value, { status: 400 })` all stay results. A 2xx is not judged at all: a login page served at 200 is indistinguishable from a void result by header alone.

packages/web/server-functions/src/client.ts

@@ -544,6 +544,24 @@ async function fetchServerFunction(base, id, options, args, meta, callArgs = arg
     if (handled !== undefined) return handled;
   }
 
+  // A refusal the runtime did not write is not a result, and decoding one
+  // yields nothing — which used to resolve the call to `undefined`, reading
+  // as "the function returned nothing" where the request never reached it.
+  // Every response the runtime encodes carries the body format (a void one
+  // included), so the two markers below identify its own; anything else at
+  // 400 and up is the peer refusing: the handler's own gates, a route it is
+  // not mounted on, an auth interstitial, a CDN block. Answered before the
+  // passthrough beneath, because a foreign refusal carries a `Location` of
+  // its own — an SSO interstitial is exactly that — and undecoded, because
+  // its body is someone else's, not a payload to hand the caller.
+  if (
+    response.status >= 400 &&
+    !response.headers.has(ERROR_HEADER) &&
+    !response.headers.has(BODY_FORMAT_HEADER)
+  ) {
+    throw serverFunctionFailure(response, undefined);
+  }
+
   // Proxies may omit the protocol error header on 5xx responses.
   const failed = response.headers.has(ERROR_HEADER) || response.status >= 500;
 

packages/web/server-functions/src/server.ts

@@ -1376,6 +1376,7 @@ function encodeResult(value, headers, status, codec, signal) {
   // client load its decode half (see shared.js loadSerializer). Negotiated
   // per response: mixed pages simply carry both formats.
   if (value === undefined) {
+    headers.set(BODY_FORMAT_HEADER, BodyFormat.Void);
     return new Response(null, { status, headers });
   }
   // By the time a result is being encoded the function has already run —

packages/web/server-functions/src/shared.ts

@@ -539,7 +539,15 @@ export const BodyFormat = {
    * legs: argument lists on the request, results (single-flight envelopes
    * included) on the response.
    */
-  Json: "8"
+  Json: "8",
+  /**
+   * No body at all — a function that returned nothing. The format carries no
+   * payload; it marks the response as one the runtime encoded, which is what
+   * separates a void result with a status on it from a refusal answered by
+   * something else. Decoding falls through to `undefined`, so a peer that
+   * predates the tag reads it the same way.
+   */
+  Void: "9"
 };
 
 // Nesting deeper than this is not JSON-safe. The guard itself walks an

packages/web/test/server/server-functions-transport-failure.spec.tsx

@@ -0,0 +1,238 @@
+/**
+ * What the transport does with a response the runtime did not write (#3087).
+ *
+ * Its own responses are recognisable: they carry the error header, or the
+ * body format every encoding path stamps — a void result included. Anything
+ * else at 400 and up is the peer refusing, and decoding one yields nothing,
+ * which used to resolve the call to `undefined`. These pin the refusals that
+ * now fail, the value-shaped statuses that must not, and the limit of what a
+ * status can tell you.
+ *
+ * One runtime-produced shape is caught along with them: a verbatim
+ * `X-Content-Raw` response at a non-2xx status, whose body no call site
+ * without an integration could read anyway. An integration is unaffected —
+ * its `responseHandler` claims the response before any of this.
+ *
+ * Like the extension 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 { redirect, respond } from "@solidjs/web";
+import {
+  handleServerFunctionRequest,
+  registerServerFunction
+} from "@solidjs/web/server-functions/server";
+import {
+  configureServerFunctionsClient,
+  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];
+});
+
+/**
+ * The transport's fetch. `answer` replaces the handler with a response from
+ * somewhere else; `rewrite` sends the call to a different address.
+ */
+function connectTransport({
+  answer,
+  rewrite
+}: { answer?: () => Response; rewrite?: (address: string) => string } = {}) {
+  const original = globalThis.fetch;
+  globalThis.fetch = ((input: RequestInfo | URL, init?: RequestInit) => {
+    if (answer) return Promise.resolve(answer());
+    const address = input instanceof Request ? input.url : input.toString();
+    const request = new Request(
+      new URL(rewrite ? rewrite(address) : address, "http://localhost"),
+      input instanceof Request ? input : init
+    );
+    request.headers.set("Sec-Fetch-Site", "same-origin");
+    return handleServerFunctionRequest(request);
+  }) as typeof fetch;
+  return () => {
+    globalThis.fetch = original;
+  };
+}
+
+const foreign = (status: number, body: BodyInit | null, type?: string) => () =>
+  new Response(body, { status, headers: type ? { "content-type": type } : undefined });
+
+describe("server-function transport failures (#3087)", () => {
+  it("fails the call when the handler refuses it", async () => {
+    const restore = connectTransport();
+    try {
+      // a client that outlived the build registering its function
+      await expect(createServerReference("fail-never-registered")()).rejects.toMatchObject({
+        status: 404
+      });
+    } finally {
+      restore();
+    }
+  });
+
+  it("fails the call when the handler rejects the request itself", async () => {
+    registerServerFunction("fail-args", async () => "ok");
+    // something under `args` that is not an argument array: a 400 for every
+    // caller of that url
+    const restore = connectTransport({ rewrite: address => `${address}?args=nope` });
+    try {
+      await expect(createServerReference("fail-args")()).rejects.toMatchObject({ status: 400 });
+    } finally {
+      restore();
+    }
+  });
+
+  it("fails the call on a response nothing in the runtime wrote", async () => {
+    registerServerFunction("fail-foreign", async () => "ok");
+    for (const answer of [
+      foreign(404, "<h1>Not found</h1>", "text/html"),
+      foreign(401, "<html>login</html>", "text/html"),
+      foreign(403, null)
+    ]) {
+      const restore = connectTransport({ answer });
+      try {
+        await expect(createServerReference("fail-foreign")()).rejects.toMatchObject({
+          status: answer().status
+        });
+      } finally {
+        restore();
+      }
+    }
+  });
+
+  it("fails one carrying integration metadata of its own", async () => {
+    registerServerFunction("fail-interstitial", async () => "ok");
+    // an SSO interstitial answers 403 with a Location, and a gateway can
+    // answer 404 with anything — neither is the runtime's control flow, so
+    // neither may take the passthrough that control flow uses
+    for (const [status, headers] of [
+      [403, { "content-type": "text/html", Location: "https://sso.example/login" }],
+      [404, { "X-Revalidate": "stories" }]
+    ] as const) {
+      const restore = connectTransport({ answer: () => new Response(null, { status, headers }) });
+      try {
+        await expect(createServerReference("fail-interstitial")()).rejects.toMatchObject({
+          status
+        });
+      } finally {
+        restore();
+      }
+    }
+  });
+
+  it("fails with an Error even when the foreign body sniffs as a known encoding", async () => {
+    registerServerFunction("fail-sniffed", async () => "ok");
+    // content-type sniffing would decode this as URLSearchParams; a refusal's
+    // body is not a payload, and a caller reads `.status`, not a form
+    const restore = connectTransport({
+      answer: foreign(403, "reason=blocked", "application/x-www-form-urlencoded")
+    });
+    try {
+      await expect(createServerReference("fail-sniffed")()).rejects.toMatchObject({ status: 403 });
+      await expect(createServerReference("fail-sniffed")()).rejects.toBeInstanceOf(Error);
+    } finally {
+      restore();
+    }
+  });
+
+  it("resolves a value-shaped status the function itself produced", async () => {
+    registerServerFunction("fail-validated", async () =>
+      respond({ field: "required" }, { status: 400 })
+    );
+    registerServerFunction("fail-void", async () => respond(undefined, { status: 400 }));
+    registerServerFunction("fail-empty", async () => new Response(null, { status: 404 }));
+    const restore = connectTransport();
+    try {
+      expect(await createServerReference("fail-validated")()).toEqual({ field: "required" });
+      // nothing, with a status on it — both spellings answer the same way
+      expect(await createServerReference("fail-void")()).toBeUndefined();
+      expect(await createServerReference("fail-empty")()).toBeNull();
+    } finally {
+      restore();
+    }
+  });
+
+  it("leaves the runtime's own control flow on its passthrough", async () => {
+    registerServerFunction("fail-redirect", async () => {
+      throw redirect("/login");
+    });
+    const restore = connectTransport();
+    try {
+      const response = await createServerReference("fail-redirect")();
+      expect(response).toBeInstanceOf(Response);
+      expect((response as Response).headers.get("Location")).toBe("/login");
+    } finally {
+      restore();
+    }
+  });
+
+  it("cannot judge a 2xx, and does not try", async () => {
+    registerServerFunction("fail-spa", async () => "ok");
+    // a login page or an SPA index served at 200 is indistinguishable from a
+    // void result by header alone; the status is all this rule reads
+    const restore = connectTransport({ answer: foreign(200, "<html>login</html>", "text/html") });
+    try {
+      expect(await createServerReference("fail-spa")()).toBeUndefined();
+    } finally {
+      restore();
+    }
+  });
+
+  it("fails a verbatim passthrough that carries a refusal's status", async () => {
+    registerServerFunction(
+      "fail-raw",
+      async () => new Response("nope", { status: 404, headers: { "X-Content-Raw": "1" } })
+    );
+    registerServerFunction(
+      "fail-raw-ok",
+      async () => new Response("here", { status: 200, headers: { "X-Content-Raw": "1" } })
+    );
+    const restore = connectTransport();
+    try {
+      // a raw body is for whoever claims the response; a call site with no
+      // integration could not read it at either status, and a status beats
+      // minting `undefined`
+      await expect(createServerReference("fail-raw")()).rejects.toMatchObject({ status: 404 });
+      expect(await createServerReference("fail-raw-ok")()).toBeUndefined();
+    } finally {
+      restore();
+    }
+  });
+
+  it("leaves the response to an integration that claims it", async () => {
+    registerServerFunction(
+      "fail-claimed",
+      async () => new Response("frame", { status: 404, headers: { "X-Content-Raw": "1" } })
+    );
+    const restore = connectTransport();
+    configureServerFunctionsClient({
+      responseHandler: { handle: (response: Response) => response.status }
+    });
+    try {
+      // the seam runs before the check, which is what keeps server
+      // components free to answer with a status of their own
+      expect(await createServerReference("fail-claimed")()).toBe(404);
+    } finally {
+      configureServerFunctionsClient({ responseHandler: null as any });
+      restore();
+    }
+  });
+
+  it("leaves an ordinary call alone", async () => {
+    registerServerFunction("fail-plain", async (word: string) => word.toUpperCase());
+    const restore = connectTransport();
+    try {
+      expect(await createServerReference("fail-plain")("solid")).toBe("SOLID");
+    } finally {
+      restore();
+    }
+  });
+});