Repository: unjs/h3
Pull request #1008 (2025-04-02 12:16:12)

refactor: improve response and hooks handling
This PR rewrites response handling logic to increase safety and consistency. 

`onError` and `onBeforeResponse` config hooks are guaranteed to always be called for 404 and other situations.


Patch:
diff --git a/src/h3.ts b/src/h3.ts
index 18b2afd..6ad289e 100644
--- a/src/h3.ts
+++ b/src/h3.ts
@@ -19,8 +19,7 @@ import {
 import { serve as srvxServe, type ServerOptions } from "srvx";
 import { getPathname, joinURL } from "./utils/internal/path";
 import { _H3Event } from "./event";
-import { kNotFound, prepareResponse } from "./response";
-import { createError } from "./error";
+import { kNotFound, handleResponse } from "./response";
 
 /**
  * Serve the h3 app, automatically handles current runtime behavior.
@@ -106,32 +105,7 @@ class _H3 implements H3 {
     }
 
     // Prepare response
-    const config = this.config;
-    if (!(handlerRes instanceof Promise)) {
-      const response = prepareResponse(handlerRes, event, config);
-      return config.onBeforeResponse
-        ? Promise.resolve(config.onBeforeResponse(event, response)).then(
-            () => response,
-          )
-        : response;
-    }
-    return handlerRes
-      .catch((error) => {
-        const h3Error = createError(error);
-        return config.onError
-          ? Promise.resolve(config.onError(h3Error, event)).then(
-              (res) => res ?? h3Error,
-            )
-          : h3Error;
-      })
-      .then((resolvedRes) => {
-        const response = prepareResponse(resolvedRes, event, config);
-        return config.onBeforeResponse
-          ? Promise.resolve(config.onBeforeResponse(event, response)).then(
-              () => response,
-            )
-          : response;
-      });
+    return handleResponse(handlerRes, event, this.config);
   }
 
   _handler(event: H3Event) {
diff --git a/src/response.ts b/src/response.ts
index b70ae90..cc19f9e 100644
--- a/src/response.ts
+++ b/src/response.ts
@@ -1,5 +1,5 @@
 import type { H3Config, H3Event } from "./types";
-import type { H3Error, PreparedResponse } from "./types/h3";
+import type { H3Error } from "./types/h3";
 import { Response as SrvxResponse } from "srvx";
 import { createError } from "./error";
 import { isJSONSerializable } from "./utils/internal/object";
@@ -7,64 +7,91 @@ import { isJSONSerializable } from "./utils/internal/object";
 export const kNotFound = /* @__PURE__ */ Symbol.for("h3.notFound");
 export const kHandled = /* @__PURE__ */ Symbol.for("h3.handled");
 
-export function prepareResponse(
+export function handleResponse(
   val: unknown,
   event: H3Event,
   config: H3Config,
-): Response {
+): Response | Promise<Response> {
+  if (val && val instanceof Promise) {
+    return val
+      .catch((error) => error)
+      .then((resolvedVal) => handleResponse(resolvedVal, event, config));
+  }
+
+  const response = prepareResponse(val, event, config);
+  if (response instanceof Promise) {
+    return handleResponse(response, event, config);
+  }
+
+  const { onBeforeResponse } = config;
+  return onBeforeResponse
+    ? Promise.resolve(onBeforeResponse(event, response)).then(() => response)
+    : response;
+}
+
+function prepareResponse(
+  val: unknown,
+  event: H3Event,
+  config: H3Config,
+  nested?: boolean,
+): Response | Promise<Response> {
   if (val === kHandled) {
-    return new Response(null);
+    return new SrvxResponse(null);
   }
 
-  if (val instanceof Response) {
-    // Note: preparted status and statusText are discarded in favor of response values
-    const preparedHeaders = (event.res as { _headers?: Headers })._headers;
-    if (!preparedHeaders) {
-      return val;
-    }
-    // Slow path: merge headers
-    const noBody = event.req.method === "HEAD" || isNullStatus(val.status);
-    const mergedHeaders = new Headers(preparedHeaders);
-    for (const [name, value] of val.headers) {
-      if (name === "set-cookie") {
-        mergedHeaders.append(name, value);
-      } else {
-        mergedHeaders.set(name, value);
-      }
-    }
-    return new SrvxResponse(noBody ? null : val.body, {
-      status: val.status,
-      statusText: val.statusText,
-      headers: mergedHeaders,
-    }) as Response;
+  if (val === kNotFound) {
+    val = createError({
+      statusCode: 404,
+      statusMessage: `Cannot find any route matching [${event.req.method}] ${event.url}`,
+    });
+  }
+
+  if (val && val instanceof Error) {
+    const error = createError(val); // todo: flag unhandled
+    const { onError } = config;
+    return onError && !nested
+      ? Promise.resolve(onError(error, event))
+          .catch((error) => error)
+          .then((newVal) => prepareResponse(newVal ?? val, event, config, true))
+      : errorResponse(error, config.debug);
   }
 
-  // We always prepare response body to resolve status and headers
-  const body = prepareResponseBody(val, event, config);
-  const status = event.res.status;
-  const responseInit: PreparedResponse = {
-    body: event.req.method === "HEAD" || isNullStatus(status) ? null : body,
-    status,
-    statusText: event.res.statusText,
-    headers: (event.res as { _headers?: Headers })._headers,
-  };
+  if (!(val instanceof Response)) {
+    const body = prepareResponseBody(val, event, config); // side effect: might set headers
+    const status = event.res.status;
+    return new SrvxResponse(nullBody(event.req.method, status) ? null : body, {
+      status,
+      statusText: event.res.statusText,
+      headers: (event.res as { _headers?: Headers })._headers,
+    });
+  }
 
-  return new SrvxResponse(responseInit.body, responseInit) as Response;
-}
+  // Note: Only check _headers. res.status/statusText are not used as we use them from the response
+  const preparedHeaders = (event.res as { _headers?: Headers })._headers;
+  if (!preparedHeaders) {
+    return val;
+  }
 
-function isNullStatus(status?: number) {
-  return (
-    status &&
-    (status === 100 ||
-      status === 101 ||
-      status === 102 ||
-      status === 204 ||
-      status === 205 ||
-      status === 304)
-  );
+  // Slow path: merge headers
+  const mergedHeaders = new Headers(preparedHeaders);
+  for (const [name, value] of val.headers) {
+    if (name === "set-cookie") {
+      mergedHeaders.append(name, value);
+    } else {
+      mergedHeaders.set(name, value);
+    }
+  }
+  return new SrvxResponse(
+    nullBody(event.req.method, val.status) ? null : val.body,
+    {
+      status: val.status,
+      statusText: val.statusText,
+      headers: mergedHeaders,
+    },
+  ) as Response;
 }
 
-export function prepareResponseBody(
+function prepareResponseBody(
   val: unknown,
   event: H3Event,
   config: H3Config,
@@ -74,18 +101,6 @@ export function prepareResponseBody(
     return "";
   }
 
-  // Not found
-  if (val === kNotFound) {
-    return prepareErrorResponseBody(
-      {
-        statusCode: 404,
-        statusMessage: `Cannot find any route matching [${event.req.method}] ${event.url}`,
-      },
-      event,
-      config,
-    );
-  }
-
   const valType = typeof val;
 
   // Text
@@ -99,11 +114,6 @@ export function prepareResponseBody(
     return val;
   }
 
-  // Error (should be before JSON)
-  if (val instanceof Error) {
-    return prepareErrorResponseBody(val, event, config);
-  }
-
   // JSON
   if (isJSONSerializable(val, valType)) {
     event.res.headers.set("content-type", "application/json; charset=utf-8");
@@ -133,37 +143,49 @@ export function prepareResponseBody(
     return val.stream();
   }
 
-  // Symbol or Function is not supported
-  if (valType === "symbol" || valType === "function") {
-    return prepareErrorResponseBody(
-      {
-        statusCode: 500,
-        statusMessage: `[h3] Cannot send ${valType} as response.`,
-      },
-      event,
-      config,
-    );
+  // Symbol or Function
+  if (valType === "symbol") {
+    return val.toString();
+  }
+  if (valType === "function") {
+    return `${(val as () => unknown).name}()`;
   }
 
   return val as BodyInit;
 }
 
-export function prepareErrorResponseBody(
-  val: Partial<H3Error> | Error,
-  event: H3Event,
-  config: H3Config,
-): string {
-  const error = createError(val as H3Error);
-  event.res.status = error.statusCode;
-  event.res.statusText = error.statusMessage;
-  event.res.headers.set("content-type", "application/json; charset=utf-8");
-  return JSON.stringify({
-    statusCode: error.statusCode,
-    statusMessage: error.statusMessage,
-    data: error.data,
-    stack:
-      config.debug && error.stack
-        ? error.stack.split("\n").map((l) => l.trim())
-        : undefined,
-  });
+function nullBody(
+  method: string,
+  status: number | undefined,
+): boolean | 0 | undefined {
+  // prettier-ignore
+  return (method === "HEAD" ||
+    status === 100 || status === 101 || status === 102 ||
+    status === 204 || status === 205 || status === 304
+  )
+}
+
+function errorResponse(error: H3Error, debug?: boolean): Response {
+  return new SrvxResponse(
+    JSON.stringify(
+      {
+        statusCode: error.statusCode,
+        statusMessage: error.statusMessage,
+        data: error.data,
+        stack:
+          debug && error.stack
+            ? error.stack.split("\n").map((l) => l.trim())
+            : undefined,
+      },
+      null,
+      2,
+    ),
+    {
+      status: error.statusCode,
+      statusText: error.statusMessage,
+      headers: {
+        "content-type": "application/json; charset=utf-8",
+      },
+    },
+  );
 }


Test patch:
diff --git a/test/app.test.ts b/test/app.test.ts
index 3d5b999..a02196a 100644
--- a/test/app.test.ts
+++ b/test/app.test.ts
@@ -17,25 +17,21 @@ describeMatrix("app", (t, { it, expect }) => {
     expect(await res.text()).toBe("9007199254740991");
   });
 
-  it("throws error when returning symbol or function", async () => {
+  it("returning symbol or function", async () => {
     t.app.get("/fn", () => {
-      return function foo() {};
+      return function test() {};
     });
     t.app.get("/symbol", () => {
-      return Symbol.for("foo");
+      return Symbol.for("test");
     });
 
     const resFn = await t.fetch("/fn");
-    expect(resFn.status).toBe(500);
-    expect((await resFn.json()).statusMessage).toBe(
-      "[h3] Cannot send function as response.",
-    );
+    expect(resFn.status).toBe(200);
+    expect(await resFn.text()).toMatch("test()");
 
     const resSymbol = await t.fetch("/symbol");
-    expect(resSymbol.status).toBe(500);
-    expect((await resSymbol.json()).statusMessage).toBe(
-      "[h3] Cannot send symbol as response.",
-    );
+    expect(resSymbol.status).toBe(200);
+    expect(await resSymbol.text()).toMatch("Symbol(test)");
   });
 
   it("can return Response directly", async () => {
diff --git a/test/bench/bundle.test.ts b/test/bench/bundle.test.ts
index fa4d5b1..645aa05 100644
--- a/test/bench/bundle.test.ts
+++ b/test/bench/bundle.test.ts
@@ -21,7 +21,7 @@ describe("benchmark", () => {
     const denoBundle = await getBundleSize(code, ["deno"]);
     // console.log(`Bundle size: (deno) ${denoBundle.bytes} (gzip: ${denoBundle.gzipSize})` );
     expect(denoBundle.bytes).toBeLessThanOrEqual(12_000); // <12kb
-    expect(denoBundle.gzipSize).toBeLessThanOrEqual(4100); // <4.1kb
+    expect(denoBundle.gzipSize).toBeLessThanOrEqual(4200); // <4.2kb
   });
 });