diff --git a/.changeset/quiet-errors-wait.md b/.changeset/quiet-errors-wait.md new file mode 100644 index 000000000..594247dce --- /dev/null +++ b/.changeset/quiet-errors-wait.md @@ -0,0 +1,6 @@ +--- +"@solidjs/start": patch +--- + +Await asynchronous `serverFunctions.onError` handlers before serializing server function errors. +Preserve the original error if the handler throws or rejects. diff --git a/apps/tests/src/server-fn-error.ts b/apps/tests/src/server-fn-error.ts index 3d3628dd5..b7ba7ff0d 100644 --- a/apps/tests/src/server-fn-error.ts +++ b/apps/tests/src/server-fn-error.ts @@ -8,7 +8,7 @@ import type { ServerFunctionErrorHandler } from "@solidjs/start/server"; * Returning `undefined` sends whatever was thrown, which is what leaves the * other server-function error tests in this app seeing their own errors. */ -const onServerFunctionError: ServerFunctionErrorHandler = thrown => { +const onServerFunctionError: ServerFunctionErrorHandler = async thrown => { if (thrown instanceof Error && thrown.message === "replace me") { return new Error("replaced by onError"); } diff --git a/packages/start/src/config/index.ts b/packages/start/src/config/index.ts index 34e06da46..ea9a00428 100644 --- a/packages/start/src/config/index.ts +++ b/packages/start/src/config/index.ts @@ -157,14 +157,17 @@ export interface SolidStartOptions { * straight to its caller, and errors from API routes never reach it * either. * - * The export is called synchronously with the thrown value, and what it - * returns decides what the client sees: + * The handler may be asynchronous. SolidStart awaits its return value + * before serializing the response, allowing a monitoring service to flush + * first. The resolved value decides what the client sees: * * - `undefined` (or `null`) sends what was thrown, unchanged. * - A `Response` is passed through unchanged, which keeps a thrown * `redirect()` working. * - Any other value is sent in place of what was thrown, and the client * call rejects with it. + * - If the handler throws or rejects, what the server function threw is + * sent unchanged. * * Control flow reaches the export the same way errors do, so a handler * that replaces everything it sees turns redirects into errors. @@ -173,10 +176,6 @@ export interface SolidStartOptions { * properties, so an error meant to be safe to expose must not carry * internal detail. * - * The return value is not awaited, so make the export a plain function - * rather than an `async` one, and report failures with calls that do not - * need awaiting. - * * Type the export as `ServerFunctionErrorHandler` from * `@solidjs/start/server`. The module is bundled into the server only, so * it may import server-only code such as a monitoring SDK. diff --git a/packages/start/src/fns/error-handler.ts b/packages/start/src/fns/error-handler.ts index 6ba8d2ba8..1164f0baa 100644 --- a/packages/start/src/fns/error-handler.ts +++ b/packages/start/src/fns/error-handler.ts @@ -5,14 +5,15 @@ import onServerFunctionError from "solid-start:server-fn-error-handler"; * it to type the default export of the module named by the * `serverFunctions.onError` option in `vite.config.ts`. * - * Called synchronously, and the value it returns is not awaited, so it cannot - * be an `async` function. + * The handler may be asynchronous. SolidStart awaits its return value before + * sending the error to the client. If the handler throws or rejects, SolidStart + * sends the original thrown value instead. * * @param thrown The value the server function threw. This is a `Response` when * the server function threw control flow such as a `redirect()`. * @returns `undefined` (or `null`) to send `thrown` unchanged, a `Response` to - * pass control flow through untouched, or any other value to send in place of - * `thrown`. + * preserve control flow, any other replacement value, or a promise resolving + * to any of those values. * * @example * ```ts @@ -33,6 +34,11 @@ import onServerFunctionError from "solid-start:server-fn-error-handler"; */ export type ServerFunctionErrorHandler = (thrown: unknown) => unknown; -export function applyServerFunctionErrorHandler(thrown: unknown): unknown { - return onServerFunctionError?.(thrown) ?? thrown; +export async function applyServerFunctionErrorHandler(thrown: unknown): Promise { + try { + const replacement = await onServerFunctionError?.(thrown); + return replacement ?? thrown; + } catch { + return thrown; + } } diff --git a/packages/start/src/fns/handler.spec.ts b/packages/start/src/fns/handler.spec.ts index f4a44324d..58594bb4c 100644 --- a/packages/start/src/fns/handler.spec.ts +++ b/packages/start/src/fns/handler.spec.ts @@ -224,6 +224,14 @@ describe("the configured server function error handler", () => { expect(h3Event.res.headers.get("X-Error")).toBe("replaced"); }); + it("awaits an asynchronous replacement", async () => { + configuredErrorHandler.current = async () => new Error("replaced asynchronously"); + + const h3Event = await callThrowing(new Error("boom")); + + expect(h3Event.res.headers.get("X-Error")).toBe("replaced asynchronously"); + }); + it("treats a Response the handler returns as control flow", async () => { configuredErrorHandler.current = () => new Response(null, { status: 403 }); @@ -233,6 +241,15 @@ describe("the configured server function error handler", () => { expect(h3Event.res.headers.get("X-Error")).toBe("true"); }); + it("treats a Response resolved by an asynchronous handler as control flow", async () => { + configuredErrorHandler.current = async () => new Response(null, { status: 403 }); + + const h3Event = await callThrowing(new Error("boom")); + + expect(h3Event.res.status).toBe(403); + expect(h3Event.res.headers.get("X-Error")).toBe("true"); + }); + it("keeps the original error when the handler returns nothing", async () => { configuredErrorHandler.current = () => undefined; @@ -241,6 +258,24 @@ describe("the configured server function error handler", () => { expect(h3Event.res.headers.get("X-Error")).toBe("boom"); }); + it("keeps the original error when an asynchronous handler returns nothing", async () => { + configuredErrorHandler.current = async () => undefined; + + const h3Event = await callThrowing(new Error("boom")); + + expect(h3Event.res.headers.get("X-Error")).toBe("boom"); + }); + + it("keeps the original error when an asynchronous handler rejects", async () => { + configuredErrorHandler.current = async () => { + throw new Error("handler failed"); + }; + + const h3Event = await callThrowing(new Error("boom")); + + expect(h3Event.res.headers.get("X-Error")).toBe("boom"); + }); + it("leaves the response untouched when no handler is configured", async () => { const h3Event = await callThrowing(new Error("boom")); diff --git a/packages/start/src/fns/handler.ts b/packages/start/src/fns/handler.ts index 135ce497c..1c6d994f4 100644 --- a/packages/start/src/fns/handler.ts +++ b/packages/start/src/fns/handler.ts @@ -127,7 +127,7 @@ export async function handleServerFunction(h3Event: H3Event) { } return serializeToJSONStream(result); } catch (x) { - x = applyServerFunctionErrorHandler(x); + x = await applyServerFunctionErrorHandler(x); if (x instanceof Response) { if (singleFlight && instance) { x = await handleSingleFlight(event, x);