Answer a non-JSON body to the approvals API with 400, not 500

CHANGELOG.md

@@ -8,6 +8,12 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged.
 
 ## Unreleased
 
+### A request to the approvals API that is not JSON answers 400
+
+A body that could not be parsed as JSON, sent to any approvals route that reads one, such as
+`PATCH /api/approvals/preferences` or `POST /api/approvals/rules`, answered 500 with the parser's own
+message. It now answers 400 "Supply a valid request.", as the delivery routes do.
+
 ### Syncing a memory source a policy refuses says why
 
 When a connected app's policy refused the read behind a memory source's sync, `POST

server/src/approvals/routes.ts

@@ -42,14 +42,19 @@ export function createApprovalRoutes(
   const routes = new Hono<{ Variables: AppVariables }>();
   routes.use("*", requireUser);
   routes.onError((error, context) =>
-    context.json(
-      { error: error.message },
-      error instanceof ApprovalNotFoundError
-        ? 404
-        : error instanceof ApprovalRefusedError || error instanceof z.ZodError
-          ? 400
-          : 500,
-    ),
+    // A body that is not JSON is the caller's mistake, as the delivery routes answer it, not a
+    // 500 carrying the parser's message.
+    error instanceof SyntaxError
+      ? context.json({ error: "Supply a valid request." }, 400)
+      : context.json(
+          { error: error.message },
+          error instanceof ApprovalNotFoundError
+            ? 404
+            : error instanceof ApprovalRefusedError ||
+                error instanceof z.ZodError
+              ? 400
+              : 500,
+        ),
   );
   routes.get("/", async (context) =>
     context.json(await service.inbox(context.var.actor.id)),

server/tests/approvals.test.ts

@@ -716,3 +716,25 @@ test.each([
   expect(continued).toHaveLength(1);
   expect(continued[0]?.result.error).toContain("Not done");
 });
+
+test("a request body that is not JSON answers 400, not a 500 with the parser's message", async () => {
+  const routes = createApprovalRoutes(
+    {} as serviceModule.ApprovalService,
+    async (ctx, next) => {
+      ctx.set("actor", {
+        id: "owner",
+        email: "owner@example.com",
+        role: "user",
+      });
+      await next();
+    },
+  );
+  for (const [method, path] of [
+    ["PATCH", "/preferences"],
+    ["POST", "/rules"],
+  ]) {
+    const response = await routes.request(path, { method, body: "{oops" });
+    expect(response.status).toBe(400);
+    expect(await response.json()).toEqual({ error: "Supply a valid request." });
+  }
+});