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 <noreply@anthropic.com>
This commit is contained in:
53
server/src/__tests__/error-handler.test.ts
Normal file
53
server/src/__tests__/error-handler.test.ts
Normal file
@@ -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<typeof vi.fn>).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");
|
||||
});
|
||||
});
|
||||
@@ -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" });
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user