fix: recover from SSR errors in dev without a manual refresh

.changeset/cuddly-pots-repeat.md

@@ -0,0 +1,5 @@
+---
+"@solidjs/start": patch
+---
+
+Recover from SSR errors in dev without a manual refresh. A syntax error in `app.tsx` left the 500 page on screen for good, because the error page has no modules registered for HMR and so ignored the update that fixed the file. The dev server now adds a small script to SSR error pages that reloads them once the server renders again.

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

@@ -1,9 +1,19 @@
 import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
-import { afterEach, describe, expect, it } from "vitest";
+import { afterEach, describe, expect, it, vi } from "vitest";
 
-import { isHtmlResponse, resolvePreviewServerEntry } from "./dev-server.ts";
+import type { ServerResponse } from "node:http";
+import type { Connect } from "vite";
+
+import {
+  HMR_RECOVERY_SCRIPT,
+  inspectWriteHead,
+  hmrRecoveryMiddleware,
+  isHtmlErrorResponse,
+  isHtmlResponse,
+  resolvePreviewServerEntry,
+} from "./dev-server.ts";
 
 const temporaryDirectories: string[] = [];
 
@@ -64,3 +74,211 @@ describe("isHtmlResponse", () => {
     expect(isHtmlResponse(new Response())).toBe(false);
   });
 });
+
+describe("isHtmlErrorResponse", () => {
+  it("recognizes an HTML server error", () => {
+    expect(isHtmlErrorResponse(500, "text/html; charset=utf-8")).toBe(true);
+  });
+
+  it.each([
+    [200, "text/html"],
+    [404, "text/html"],
+    [500, "application/json"],
+    [500, undefined],
+  ])("ignores %i %s", (status, contentType) => {
+    expect(isHtmlErrorResponse(status, contentType)).toBe(false);
+  });
+});
+
+describe("inspectWriteHead", () => {
+  it("reads the content type from headers given as an object", () => {
+    const args = [500, "Internal Server Error", { "Content-Type": "text/html" }];
+
+    expect(inspectWriteHead(args).contentType).toBe("text/html");
+  });
+
+  it("reads the content type from headers given as a flat array", () => {
+    const args = [500, ["content-length", "12", "content-type", "text/html"]];
+
+    expect(inspectWriteHead(args).contentType).toBe("text/html");
+  });
+
+  it("does not mistake the status message for headers", () => {
+    expect(inspectWriteHead([500, "content-type"]).contentType).toBeUndefined();
+  });
+
+  it("removes the length from an object without touching the rest", () => {
+    const args = [500, "OK", { "Content-Length": "12", "Content-Type": "text/html" }];
+
+    expect(inspectWriteHead(args).withoutContentLength).toEqual([
+      500,
+      "OK",
+      { "Content-Type": "text/html" },
+    ]);
+  });
+
+  it("removes the length and its value from a flat array", () => {
+    const args = [500, ["content-length", "12", "content-type", "text/html"]];
+
+    expect(inspectWriteHead(args).withoutContentLength).toEqual([
+      500,
+      ["content-type", "text/html"],
+    ]);
+  });
+
+  it("leaves arguments that carry no headers alone", () => {
+    expect(inspectWriteHead([500]).withoutContentLength).toEqual([500]);
+  });
+});
+
+describe("hmrRecoveryMiddleware", () => {
+  function collect(chunk: string | ArrayBufferView) {
+    return typeof chunk === "string"
+      ? Buffer.from(chunk, "utf8")
+      : Buffer.from(chunk.buffer, chunk.byteOffset, chunk.byteLength);
+  }
+
+  function createResponse() {
+    const headers = new Map<string, unknown>();
+    const chunks: Array<Buffer> = [];
+    const response = {
+      statusCode: 200,
+      writeHeadArgs: undefined as Array<unknown> | undefined,
+      getHeader: (name: string) => headers.get(name.toLowerCase()),
+      setHeader(name: string, value: unknown) {
+        headers.set(name.toLowerCase(), value);
+        return response;
+      },
+      headersSent: false,
+      removeHeader(name: string) {
+        headers.delete(name.toLowerCase());
+      },
+      writeHead(...args: Array<unknown>) {
+        response.writeHeadArgs = args;
+        response.headersSent = true;
+        return response;
+      },
+      write(chunk: string | ArrayBufferView) {
+        chunks.push(collect(chunk));
+        response.headersSent = true;
+        return true;
+      },
+      end(chunk?: unknown) {
+        // Deliberately does not go through `write`: the real `end` writes to the socket directly,
+        // and delegating would feed the chunk straight back into the middleware's wrapper.
+        if (typeof chunk === "string" || ArrayBuffer.isView(chunk)) chunks.push(collect(chunk));
+        response.headersSent = true;
+        return response;
+      },
+      get body() {
+        return Buffer.concat(chunks).toString("utf8");
+      },
+    };
+
+    return response;
+  }
+
+  function run(response: ReturnType<typeof createResponse>, accept = "text/html") {
+    const next = vi.fn();
+    hmrRecoveryMiddleware(
+      { headers: { accept } } as Connect.IncomingMessage,
+      response as unknown as ServerResponse,
+      next as unknown as Connect.NextFunction,
+    );
+
+    return next;
+  }
+
+  it("appends the recovery script to an HTML server error", () => {
+    const response = createResponse();
+    run(response);
+
+    const page = "<html><head></head><body>boom</body></html>";
+    response.writeHead(500, "Internal Server Error", [
+      "content-length",
+      String(page.length),
+      "content-type",
+      "text/html; charset=utf-8",
+    ]);
+    response.end(new TextEncoder().encode(page));
+
+    expect(response.body).toBe(page + HMR_RECOVERY_SCRIPT);
+  });
+
+  it("drops the announced length so the longer body can go out chunked", () => {
+    const response = createResponse();
+    run(response);
+
+    response.writeHead(500, "Internal Server Error", [
+      "content-length",
+      "42",
+      "content-type",
+      "text/html; charset=utf-8",
+    ]);
+    response.end("<html></html>");
+
+    expect(response.writeHeadArgs).toEqual([
+      500,
+      "Internal Server Error",
+      ["content-type", "text/html; charset=utf-8"],
+    ]);
+  });
+
+  it("removes a length set without writeHead before the headers go out", () => {
+    const response = createResponse();
+    run(response);
+
+    response.statusCode = 500;
+    response.setHeader("content-type", "text/html");
+    response.setHeader("content-length", "13");
+    response.end("<html></html>");
+
+    expect(response.getHeader("content-length")).toBeUndefined();
+    expect(response.body).toBe("<html></html>" + HMR_RECOVERY_SCRIPT);
+  });
+
+  it("streams a body written across several chunks straight through", () => {
+    const response = createResponse();
+    run(response);
+
+    response.writeHead(500, { "content-type": "text/html" });
+    response.write("<html><head></head><body>");
+    response.write("boom");
+    response.end("</body></html>");
+
+    expect(response.body).toBe("<html><head></head><body>boom</body></html>" + HMR_RECOVERY_SCRIPT);
+  });
+
+  it("leaves a successful page untouched", () => {
+    const response = createResponse();
+    run(response);
+
+    response.writeHead(200, { "content-type": "text/html" });
+    response.end("<html><head></head><body>fine</body></html>");
+
+    expect(response.body).toBe("<html><head></head><body>fine</body></html>");
+    expect(response.getHeader("content-length")).toBeUndefined();
+  });
+
+  it("leaves a non-HTML server error untouched", () => {
+    const response = createResponse();
+    run(response);
+
+    response.writeHead(500, { "content-type": "application/json" });
+    response.end(`{"error":true}`);
+
+    expect(response.body).toBe(`{"error":true}`);
+  });
+
+  it("skips requests that do not accept HTML", () => {
+    const response = createResponse();
+    const next = run(response, "application/json");
+
+    expect(next).toHaveBeenCalled();
+
+    response.writeHead(500, { "content-type": "text/html" });
+    response.end("<html><head></head></html>");
+
+    expect(response.body).toBe("<html><head></head></html>");
+  });
+});

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

@@ -1,4 +1,5 @@
 import { existsSync } from "node:fs";
+import type { ServerResponse } from "node:http";
 import { join } from "node:path";
 import { pathToFileURL } from "node:url";
 import { NodeRequest, sendNodeResponse } from "srvx/node";
@@ -33,6 +34,12 @@ export function devServer(serverEntryPath: string): Array<PluginOption> {
       },
       configureServer(viteDevServer) {
         (globalThis as any).VITE_DEV_SERVER = viteDevServer;
+
+        // Runs before the middleware that actually renders the document (ours below, or the one
+        // installed by whichever plugin owns the server environment) so that it can rewrite the
+        // response it produces.
+        viteDevServer.middlewares.use(hmrRecoveryMiddleware);
+
         return async () => {
           if (viteDevServer.config.server.middlewareMode) return;
 
@@ -165,6 +172,132 @@ function removeHtmlMiddlewares(server: ViteDevServer) {
   }
 }
 
+/**
+ * Reloads the page once an HMR update makes the server render it again.
+ *
+ * Vite's client only reloads by itself when the page either has a `vite-error-overlay` mounted or
+ * receives an explicit `full-reload`. An SSR error page rendered by another plugin (Nitro's, for
+ * instance) has neither, and none of its modules are registered for HMR, so a plain `update` for
+ * the fixed file would be dropped and the error page would stay on screen forever.
+ *
+ * The update is broadcast to the browser before the server environment has necessarily picked the
+ * change up, so reloading straight away tends to render the same error again. Retrying briefly is
+ * what makes the recovery reliable; failing quietly leaves the next update to try again.
+ */
+export const HMR_RECOVERY_SCRIPT = `<script type="module">
+  import { createHotContext } from "/@vite/client";
+
+  let pending = false;
+  async function reloadWhenServerRecovers() {
+    if (pending) return;
+    pending = true;
+    for (const delay of [0, 100, 200, 400, 800]) {
+      await new Promise(resolve => setTimeout(resolve, delay));
+      const response = await fetch(location.href, {
+        cache: "no-store",
+        headers: { accept: "text/html" },
+      }).catch(() => null);
+      if (response && response.status < 500) return location.reload();
+    }
+    pending = false;
+  }
+
+  const hot = createHotContext("/@solid-start/dev-error-page");
+  hot.on("vite:beforeUpdate", reloadWhenServerRecovers);
+  hot.on("vite:error", reloadWhenServerRecovers);
+</script>`;
+
+/**
+ * Reads the content type out of `writeHead` arguments and drops `content-length` from them.
+ *
+ * `writeHead` sends the headers immediately, and it may carry them as an object or as a flat
+ * `[name, value, ...]` array that never reaches `getHeader`, so this is the only chance to see or
+ * change them. Dropping the length rather than recomputing it lets Node fall back to chunked
+ * encoding, so the body can stream through untouched instead of being buffered up to measure it.
+ *
+ * @param args the arguments `writeHead` was called with
+ * @returns the content type it announced, and a copy of the arguments without the length
+ */
+export function inspectWriteHead(args: Array<unknown>) {
+  let contentType: string | undefined;
+
+  const withoutContentLength = args.map((argument, index) => {
+    const entries = index === 0 ? undefined : headerEntries(argument);
+    if (!entries) return argument;
+
+    const kept = entries.filter(([name, value]) => {
+      const header = String(name).toLowerCase();
+      if (header === "content-type") contentType ??= String(value);
+      return header !== "content-length";
+    });
+
+    return Array.isArray(argument) ? kept.flat() : Object.fromEntries(kept);
+  });
+
+  return { contentType, withoutContentLength };
+}
+
+/** Both header forms `writeHead` accepts, as pairs. */
+function headerEntries(argument: unknown): Array<[unknown, unknown]> | undefined {
+  if (Array.isArray(argument)) {
+    const pairs: Array<[unknown, unknown]> = [];
+    for (let i = 0; i < argument.length - 1; i += 2) pairs.push([argument[i], argument[i + 1]]);
+    return pairs;
+  }
+
+  return argument && typeof argument === "object" ? Object.entries(argument) : undefined;
+}
+
+export function isHtmlErrorResponse(status: number, contentType: unknown): boolean {
+  return (
+    status >= 500 &&
+    typeof contentType === "string" &&
+    contentType.toLowerCase().startsWith("text/html")
+  );
+}
+
+/**
+ * Appends {@link HMR_RECOVERY_SCRIPT} to SSR error pages. Every other response is left alone.
+ */
+export function hmrRecoveryMiddleware(
+  req: Connect.IncomingMessage,
+  res: ServerResponse,
+  next: Connect.NextFunction,
+) {
+  // Only documents can host the recovery script.
+  if (!req.headers.accept?.includes("text/html")) return next();
+
+  const originalWriteHead = res.writeHead.bind(res);
+  const originalEnd = res.end.bind(res);
+  let injecting: boolean | undefined;
+
+  res.writeHead = function writeHead(...args: Array<any>) {
+    const { contentType, withoutContentLength } = inspectWriteHead(args);
+    injecting = isHtmlErrorResponse(
+      typeof args[0] === "number" ? args[0] : res.statusCode,
+      contentType ?? res.getHeader("content-type"),
+    );
+
+    return originalWriteHead(...((injecting ? withoutContentLength : args) as [number]));
+  } as ServerResponse["writeHead"];
+
+  res.end = function end(chunk: any, ...rest: Array<any>) {
+    // A response that never called `writeHead` still has its status and headers on `res`.
+    injecting ??= isHtmlErrorResponse(res.statusCode, res.getHeader("content-type"));
+    if (!injecting) return originalEnd(chunk, ...rest);
+
+    if (!res.headersSent) res.removeHeader("content-length");
+    if (typeof chunk === "string" || ArrayBuffer.isView(chunk)) {
+      res.write(chunk, ...(rest.filter(argument => typeof argument === "string") as []));
+    }
+
+    const callback = [chunk, ...rest].find(argument => typeof argument === "function");
+    return originalEnd(HMR_RECOVERY_SCRIPT, callback as () => void);
+  } as ServerResponse["end"];
+
+  next();
+}
+
 /**
  * Formats error for SSR message in error overlay
  * @param req