From 1420b86aa7dd31432933d5a38c35dafabc246da9 Mon Sep 17 00:00:00 2001 From: Dotta Date: Sat, 7 Mar 2026 15:19:03 -0600 Subject: [PATCH] fix(server): attach raw Error to res.err and avoid pino err key collision Extract attachErrorContext helper to DRY up the error handler, attach the original Error object to res.err so pino can serialize stack traces, and rename the log context key from err to errorContext so it doesn't clash with pino's built-in err serializer. Co-Authored-By: Claude Opus 4.6 --- server/src/__tests__/error-handler.test.ts | 53 ++++++++++++++++++++++ server/src/middleware/error-handler.ts | 50 +++++++++++++------- server/src/middleware/logger.ts | 2 +- 3 files changed, 87 insertions(+), 18 deletions(-) create mode 100644 server/src/__tests__/error-handler.test.ts diff --git a/server/src/__tests__/error-handler.test.ts b/server/src/__tests__/error-handler.test.ts new file mode 100644 index 00000000..d01a8c3c --- /dev/null +++ b/server/src/__tests__/error-handler.test.ts @@ -0,0 +1,53 @@ +import type { NextFunction, Request, Response } from "express"; +import { describe, expect, it, vi } from "vitest"; +import { HttpError } from "../errors.js"; +import { errorHandler } from "../middleware/error-handler.js"; + +function makeReq(): Request { + return { + method: "GET", + originalUrl: "/api/test", + body: { a: 1 }, + params: { id: "123" }, + query: { q: "x" }, + } as unknown as Request; +} + +function makeRes(): Response { + const res = { + status: vi.fn(), + json: vi.fn(), + } as unknown as Response; + (res.status as unknown as ReturnType).mockReturnValue(res); + return res; +} + +describe("errorHandler", () => { + it("attaches the original Error to res.err for 500s", () => { + const req = makeReq(); + const res = makeRes() as any; + const next = vi.fn() as unknown as NextFunction; + const err = new Error("boom"); + + errorHandler(err, req, res, next); + + expect(res.status).toHaveBeenCalledWith(500); + expect(res.json).toHaveBeenCalledWith({ error: "Internal server error" }); + expect(res.err).toBe(err); + expect(res.__errorContext?.error?.message).toBe("boom"); + }); + + it("attaches HttpError instances for 500 responses", () => { + const req = makeReq(); + const res = makeRes() as any; + const next = vi.fn() as unknown as NextFunction; + const err = new HttpError(500, "db exploded"); + + errorHandler(err, req, res, next); + + expect(res.status).toHaveBeenCalledWith(500); + expect(res.json).toHaveBeenCalledWith({ error: "db exploded" }); + expect(res.err).toBe(err); + expect(res.__errorContext?.error?.message).toBe("db exploded"); + }); +}); diff --git a/server/src/middleware/error-handler.ts b/server/src/middleware/error-handler.ts index 293c42ab..7f86dfd0 100644 --- a/server/src/middleware/error-handler.ts +++ b/server/src/middleware/error-handler.ts @@ -11,6 +11,25 @@ export interface ErrorContext { reqQuery?: unknown; } +function attachErrorContext( + req: Request, + res: Response, + payload: ErrorContext["error"], + rawError?: Error, +) { + (res as any).__errorContext = { + error: payload, + method: req.method, + url: req.originalUrl, + reqBody: req.body, + reqParams: req.params, + reqQuery: req.query, + } satisfies ErrorContext; + if (rawError) { + (res as any).err = rawError; + } +} + export function errorHandler( err: unknown, req: Request, @@ -19,14 +38,12 @@ export function errorHandler( ) { if (err instanceof HttpError) { if (err.status >= 500) { - (res as any).__errorContext = { - error: { message: err.message, stack: err.stack, name: err.name, details: err.details }, - method: req.method, - url: req.originalUrl, - reqBody: req.body, - reqParams: req.params, - reqQuery: req.query, - } satisfies ErrorContext; + attachErrorContext( + req, + res, + { message: err.message, stack: err.stack, name: err.name, details: err.details }, + err, + ); } res.status(err.status).json({ error: err.message, @@ -40,16 +57,15 @@ export function errorHandler( return; } - (res as any).__errorContext = { - error: err instanceof Error + const rootError = err instanceof Error ? err : new Error(String(err)); + attachErrorContext( + req, + res, + err instanceof Error ? { message: err.message, stack: err.stack, name: err.name } - : { message: String(err), raw: err }, - method: req.method, - url: req.originalUrl, - reqBody: req.body, - reqParams: req.params, - reqQuery: req.query, - } satisfies ErrorContext; + : { message: String(err), raw: err, stack: rootError.stack, name: rootError.name }, + rootError, + ); res.status(500).json({ error: "Internal server error" }); } diff --git a/server/src/middleware/logger.ts b/server/src/middleware/logger.ts index dd826c06..be47e3c5 100644 --- a/server/src/middleware/logger.ts +++ b/server/src/middleware/logger.ts @@ -62,7 +62,7 @@ export const httpLogger = pinoHttp({ const ctx = (res as any).__errorContext; if (ctx) { return { - err: ctx.error, + errorContext: ctx.error, reqBody: ctx.reqBody, reqParams: ctx.reqParams, reqQuery: ctx.reqQuery,