fix(server): leave unhandled errors to the server hosting the handler

.changeset/unhandled-errors-reach-host.md

@@ -0,0 +1,5 @@
+---
+"@solidjs/start": patch
+---
+
+Let the server hosting the app answer and report an error that an API route or middleware throws, or that a server function request hits before the function runs. The handler answered it inside its own h3 app, so its `fetch` never rejected: with Nitro, the `error` hook and the `errorHandler` never ran for it, and SolidStart's own dev server never showed its error overlay. An h3 `HTTPError` not marked unhandled, or a status thrown as a number, is answered as before. A server that calls the handler's `fetch` itself now gets a rejected promise for such an error and answers it; headers set for the error response with `event.res.errHeaders` are not part of that answer.

apps/tests/src/e2e/api-call.test.ts

@@ -32,6 +32,14 @@ test.describe("api calls", () => {
     expect(head.headers.get("content-type")).toBe(get.headers.get("content-type"));
   });
 
+  test("should pass an error thrown by an API route to Nitro's error hook", async () => {
+    const response = await fetch("http://localhost:3000/api/throws");
+
+    expect(response.status).toBe(500);
+    // Set by the Nitro plugin in `src/nitro-error-hook.ts` from Nitro's `error` hook.
+    expect(response.headers.get("x-nitro-error-hook")).toBe("thrown by an API route");
+  });
+
   test("should preserve multiple Set-Cookie headers on redirect (RFC 6265)", async () => {
     const response = await fetch("http://localhost:3000/api/multi-set-cookie-redirect", {
       redirect: "manual",

apps/tests/src/nitro-error-hook.ts

@@ -0,0 +1,18 @@
+import { definePlugin } from "nitro";
+
+/**
+ * Names the error that Nitro's `error` hook received for a request in a header
+ * of its response, so a test can tell that the error reached the server.
+ */
+export default definePlugin(nitroApp => {
+  const errors = new WeakMap<object, string>();
+
+  nitroApp.hooks.hook("error", (error, { event }) => {
+    if (event) errors.set(event.req, error.message);
+  });
+
+  nitroApp.hooks.hook("response", (response, event) => {
+    const message = errors.get(event.req);
+    if (message) response.headers.set("x-nitro-error-hook", message);
+  });
+});

apps/tests/src/routes/api/throws.ts

@@ -0,0 +1,3 @@
+export const GET = async () => {
+  throw new Error("thrown by an API route");
+};

apps/tests/vite.config.ts

@@ -33,6 +33,7 @@ export default defineConfig({
     }),
     nitro({
       compressPublicAssets: true,
+      plugins: ["./src/nitro-error-hook.ts"],
     }),
   ],
 });

packages/start/src/config/dev-server.spec.ts

@@ -1,19 +1,28 @@
 import { mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from "node:fs";
+import { createServer as createHttpServer } from "node:http";
+import type { AddressInfo } from "node:net";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import {
+  type Connect,
   createServer,
   type DevEnvironment,
   type EnvironmentModuleNode,
   type HotUpdateOptions,
   normalizePath,
   type Plugin,
+  type PreviewServer,
   type ViteDevServer,
 } from "vite";
 import { afterEach, describe, expect, it, vi } from "vitest";
 
 import { VITE_ENVIRONMENTS } from "./constants.ts";
-import { isHtmlResponse, resolvePreviewServerEntry, serverOnlyReload } from "./dev-server.ts";
+import {
+  devServer,
+  isHtmlResponse,
+  resolvePreviewServerEntry,
+  serverOnlyReload,
+} from "./dev-server.ts";
 
 const temporaryDirectories: string[] = [];
 
@@ -55,6 +64,58 @@ describe("resolvePreviewServerEntry", () => {
   });
 });
 
+describe("the preview server", () => {
+  /**
+   * Serves a built server entry whose `fetch` rejects through the preview
+   * middleware, and answers the status of one request to it.
+   */
+  async function preview(error: Error) {
+    const httpServer = createHttpServer();
+    (globalThis as any).PREVIEW_ERROR = error;
+    try {
+      const { root, serverEntry } = createServerEntry("mjs");
+      writeFileSync(
+        serverEntry,
+        `export default { fetch: () => Promise.reject(globalThis.PREVIEW_ERROR) };`,
+      );
+
+      let middleware: Connect.NextHandleFunction | undefined;
+      const previewServer = {
+        config: { root, plugins: [] },
+        middlewares: { use: (fn: Connect.NextHandleFunction) => (middleware = fn) },
+      } as unknown as PreviewServer;
+      const plugin = devServer("./src/entry-server.tsx")[1] as Plugin;
+      const configure = plugin.configurePreviewServer as (server: PreviewServer) => () => void;
+      configure(previewServer)();
+
+      httpServer.on("request", (req, res) => middleware!(req, res, () => {}));
+      await new Promise<void>(resolve => httpServer.listen(0, "127.0.0.1", resolve));
+      const { port } = httpServer.address() as AddressInfo;
+      const response = await fetch(`http://127.0.0.1:${port}/api`, {
+        signal: AbortSignal.timeout(2000),
+      });
+      await response.body?.cancel();
+      return response.status;
+    } finally {
+      delete (globalThis as any).PREVIEW_ERROR;
+      if (httpServer.listening) {
+        httpServer.closeAllConnections();
+        await new Promise(resolve => httpServer.close(resolve));
+      }
+    }
+  }
+
+  it("answers with a 500 when the app leaves an error to it", async () => {
+    const error = new Error("thrown by an API route");
+    const consoleError = vi.spyOn(console, "error").mockImplementation(() => {});
+
+    const status = await preview(error);
+
+    expect(status).toBe(500);
+    expect(consoleError).toHaveBeenCalledWith(error);
+  });
+});
+
 describe("serverOnlyReload", () => {
   /** An app whose middleware is configured without a file extension */
   async function createApp() {

packages/start/src/config/dev-server.ts

@@ -29,14 +29,20 @@ export function devServer(serverEntryPath: string, middlewarePath?: string): Arr
         return () => {
           server.middlewares.use(async (req, res) => {
             const webReq = new NodeRequest({ req, res });
-            const def: {
-              default: { fetch: (req: Request) => Promise<Response> };
-            } = await import(serverEntryUrl);
-            const webRes = await def.default.fetch(webReq);
-            if (isHtmlResponse(webRes)) {
-              res.setHeader("content-encoding", "identity");
+            try {
+              const def: {
+                default: { fetch: (req: Request) => Promise<Response> };
+              } = await import(serverEntryUrl);
+              const webRes = await def.default.fetch(webReq);
+              if (isHtmlResponse(webRes)) {
+                res.setHeader("content-encoding", "identity");
+              }
+              sendNodeResponse(res, webRes);
+            } catch (error) {
+              // The handler leaves an unhandled error to the server hosting it.
+              console.error(error);
+              sendNodeResponse(res, new Response("Internal Server Error", { status: 500 }));
             }
-            sendNodeResponse(res, webRes);
           });
         };
       },