From ef0ea300eee4dc5f5668d63ce0347ffaba542ba6 Mon Sep 17 00:00:00 2001
From: Mike Mackenzie <veb@users.noreply.github.com>
Date: Sat, 22 Aug 2026 09:20:26 +1200
Subject: [PATCH] fix(backend): keep 4xx statuses from body-parser errors

The catch-all mapped every non-ClientError to 500, so malformed JSON and
oversize bodies read as server faults: monitoring counts them against
5xx rates and webhook senders retry them forever. http-errors carries a
numeric status; honour it when it is a 4xx.
---
 .../src/__tests__/server-abort-crash.test.ts  | 101 ++++++++++++++++++
 packages/backend/src/server.ts                |  17 ++-
 2 files changed, 116 insertions(+), 2 deletions(-)
 create mode 100644 packages/backend/src/__tests__/server-abort-crash.test.ts

diff --git a/packages/backend/src/__tests__/server-abort-crash.test.ts b/packages/backend/src/__tests__/server-abort-crash.test.ts
new file mode 100644
index 0000000..dc57013
--- /dev/null
+++ b/packages/backend/src/__tests__/server-abort-crash.test.ts
@@ -0,0 +1,101 @@
+/**
+ * Regression test: a request that fails inside body parsing must produce an
+ * HTTP error response, not kill the process.
+ *
+ * Polka 0.5.2's default catch-all does `res.end(err.length && err || ...)`.
+ * body-parser's errors ("request aborted", "request entity too large") carry
+ * a numeric `length` property, so the Error object itself is written to the
+ * response, `res.end` throws ERR_INVALID_ARG_TYPE, and on the abort path
+ * the throw surfaces as an unhandled 'error' event on the IncomingMessage,
+ * which exits the process. In production, webhook senders that hang up
+ * mid-upload were killing the API on every retry.
+ *
+ * These tests boot the real server factory, so they pin the custom onError
+ * handler that replaces Polka's default.
+ */
+import { connect } from "node:net";
+import { request } from "node:http";
+import type { AddressInfo } from "node:net";
+import { afterAll, beforeAll, describe, expect, it } from "vitest";
+import type { Config } from "../config";
+import { createServer_, type ServerContext } from "../server";
+
+let ctx: ServerContext;
+let port: number;
+const uncaught: unknown[] = [];
+const onUncaught = (err: unknown) => {
+  uncaught.push(err);
+};
+
+beforeAll(async () => {
+  process.on("uncaughtException", onUncaught);
+  ctx = createServer_({ config: {} as Config, version: "v1" });
+  ctx.polka.post("/hook", (_req: any, res: any) => {
+    res.statusCode = 200;
+    res.end("ok");
+  });
+  await new Promise<void>((resolve) => ctx.httpServer.listen(0, resolve));
+  port = (ctx.httpServer.address() as AddressInfo).port;
+});
+
+afterAll(async () => {
+  process.off("uncaughtException", onUncaught);
+  ctx.io.close();
+  await new Promise((resolve) => ctx.httpServer.close(resolve));
+});
+
+function post(body: string): Promise<{ status: number; text: string }> {
+  return new Promise((resolve, reject) => {
+    const req = request(
+      {
+        port,
+        method: "POST",
+        path: "/hook",
+        headers: { "content-type": "application/json" },
+      },
+      (res) => {
+        let text = "";
+        res.on("data", (c) => {
+          text += c;
+        });
+        res.on("end", () => resolve({ status: res.statusCode ?? 0, text }));
+      },
+    );
+    req.on("error", reject);
+    req.end(body);
+  });
+}
+
+describe("server survives body-parser errors", () => {
+  it("answers malformed JSON with a 4xx, not a 5xx", async () => {
+    // body-parser's parse failure is an http-error with status 400. A 500
+    // here would make client garbage look like a server fault to monitoring
+    // and invite webhook senders to retry it forever.
+    const res = await post("{not json");
+    expect(res.status).toBe(400);
+    expect(JSON.parse(res.text)).toMatchObject({ success: false });
+    expect(uncaught).toEqual([]);
+  });
+
+  it("stays alive when the client hangs up mid-body", async () => {
+    // The production shape: a webhook sender times out and resets the
+    // connection while the JSON body is still uploading.
+    const socket = connect(port);
+    await new Promise<void>((resolve) => socket.on("connect", resolve));
+    socket.write(
+      "POST /hook HTTP/1.1\r\n" +
+        `Host: 127.0.0.1:${port}\r\n` +
+        "Content-Type: application/json\r\n" +
+        "Content-Length: 1000\r\n" +
+        "\r\n" +
+        '{"partial":',
+    );
+    await new Promise((resolve) => setTimeout(resolve, 50));
+    socket.destroy();
+    await new Promise((resolve) => setTimeout(resolve, 100));
+
+    expect(uncaught).toEqual([]);
+    const after = await post('{"ok":true}');
+    expect(after.status).toBe(200);
+  });
+});
diff --git a/packages/backend/src/server.ts b/packages/backend/src/server.ts
index 27bb435..cec0d35 100644
--- a/packages/backend/src/server.ts
+++ b/packages/backend/src/server.ts
@@ -64,11 +64,24 @@ export function createServer_(options: ServerOptions): ServerContext {
   const app = polka({
     onError: (err: unknown, req: any, res: any) => {
       if (res.writableEnded || res.finished) return;
-      const status = err instanceof ClientError ? err.status : 500;
+      // http-errors (body-parser: malformed JSON, oversize body, abort)
+      // carries a numeric 4xx status; collapsing those to 500 would make
+      // client garbage retryable and page whoever watches 5xx rates.
+      const httpStatus = (err as { status?: unknown })?.status;
+      const status =
+        err instanceof ClientError
+          ? err.status
+          : typeof httpStatus === "number" &&
+              httpStatus >= 400 &&
+              httpStatus < 500
+            ? httpStatus
+            : 500;
       const message =
         err instanceof ClientError
           ? err.message
-          : "An error occurred while processing your request";
+          : status < 500
+            ? "Bad request"
+            : "An error occurred while processing your request";
       log.error("[http] request failed:", req.method, req.url, err);
       error(res, status, message);
     },