not sure why those errors got removed we'll see
All checks were successful
Deploy to Test Environment / deploy-to-test (push) Successful in 12m30s
All checks were successful
Deploy to Test Environment / deploy-to-test (push) Successful in 12m30s
This commit is contained in:
@@ -112,7 +112,7 @@ describe('errorHandler Middleware', () => {
|
|||||||
// In test/dev, we now expect a stack trace for 5xx errors.
|
// In test/dev, we now expect a stack trace for 5xx errors.
|
||||||
expect(response.body.message).toBe('A generic server error occurred.');
|
expect(response.body.message).toBe('A generic server error occurred.');
|
||||||
expect(response.body.stack).toBeDefined();
|
expect(response.body.stack).toBeDefined();
|
||||||
expect(response.body.stack).toEqual(expect.any(String));
|
expect(response.body.errorId).toEqual(expect.any(String));
|
||||||
expect(mockLogger.error).toHaveBeenCalledWith(
|
expect(mockLogger.error).toHaveBeenCalledWith(
|
||||||
expect.objectContaining({
|
expect.objectContaining({
|
||||||
err: expect.any(Error),
|
err: expect.any(Error),
|
||||||
@@ -136,15 +136,11 @@ describe('errorHandler Middleware', () => {
|
|||||||
expect(mockLogger.warn).toHaveBeenCalledWith(
|
expect(mockLogger.warn).toHaveBeenCalledWith(
|
||||||
{
|
{
|
||||||
err: expect.any(Error),
|
err: expect.any(Error),
|
||||||
validationErrors: undefined,
|
|
||||||
statusCode: 404,
|
statusCode: 404,
|
||||||
},
|
},
|
||||||
'Client Error on GET /http-error-404: Resource not found',
|
'Client Error on GET /http-error-404: Resource not found',
|
||||||
);
|
);
|
||||||
expect(consoleErrorSpy).toHaveBeenCalledWith(
|
expect(consoleErrorSpy).not.toHaveBeenCalled();
|
||||||
expect.stringContaining('--- [TEST] UNHANDLED ERROR ---'),
|
|
||||||
expect.any(Error),
|
|
||||||
);
|
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should handle a NotFoundError with a 404 status', async () => {
|
it('should handle a NotFoundError with a 404 status', async () => {
|
||||||
@@ -156,15 +152,11 @@ describe('errorHandler Middleware', () => {
|
|||||||
expect(mockLogger.warn).toHaveBeenCalledWith(
|
expect(mockLogger.warn).toHaveBeenCalledWith(
|
||||||
{
|
{
|
||||||
err: expect.any(NotFoundError),
|
err: expect.any(NotFoundError),
|
||||||
validationErrors: undefined,
|
|
||||||
statusCode: 404,
|
statusCode: 404,
|
||||||
},
|
},
|
||||||
'Client Error on GET /not-found-error: Specific resource missing',
|
'Client Error on GET /not-found-error: Specific resource missing',
|
||||||
);
|
);
|
||||||
expect(consoleErrorSpy).toHaveBeenCalledWith(
|
expect(consoleErrorSpy).not.toHaveBeenCalled();
|
||||||
expect.stringContaining('--- [TEST] UNHANDLED ERROR ---'),
|
|
||||||
expect.any(NotFoundError),
|
|
||||||
);
|
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should handle a ForeignKeyConstraintError with a 400 status and the specific error message', async () => {
|
it('should handle a ForeignKeyConstraintError with a 400 status and the specific error message', async () => {
|
||||||
@@ -176,15 +168,11 @@ describe('errorHandler Middleware', () => {
|
|||||||
expect(mockLogger.warn).toHaveBeenCalledWith(
|
expect(mockLogger.warn).toHaveBeenCalledWith(
|
||||||
{
|
{
|
||||||
err: expect.any(ForeignKeyConstraintError),
|
err: expect.any(ForeignKeyConstraintError),
|
||||||
validationErrors: undefined,
|
|
||||||
statusCode: 400,
|
statusCode: 400,
|
||||||
},
|
},
|
||||||
'Client Error on GET /fk-error: The referenced item does not exist.',
|
'Client Error on GET /fk-error: The referenced item does not exist.',
|
||||||
);
|
);
|
||||||
expect(consoleErrorSpy).toHaveBeenCalledWith(
|
expect(consoleErrorSpy).not.toHaveBeenCalled();
|
||||||
expect.stringContaining('--- [TEST] UNHANDLED ERROR ---'),
|
|
||||||
expect.any(ForeignKeyConstraintError),
|
|
||||||
);
|
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should handle a UniqueConstraintError with a 409 status and the specific error message', async () => {
|
it('should handle a UniqueConstraintError with a 409 status and the specific error message', async () => {
|
||||||
@@ -196,15 +184,11 @@ describe('errorHandler Middleware', () => {
|
|||||||
expect(mockLogger.warn).toHaveBeenCalledWith(
|
expect(mockLogger.warn).toHaveBeenCalledWith(
|
||||||
{
|
{
|
||||||
err: expect.any(UniqueConstraintError),
|
err: expect.any(UniqueConstraintError),
|
||||||
validationErrors: undefined,
|
|
||||||
statusCode: 409,
|
statusCode: 409,
|
||||||
},
|
},
|
||||||
'Client Error on GET /unique-error: This item already exists.',
|
'Client Error on GET /unique-error: This item already exists.',
|
||||||
);
|
);
|
||||||
expect(consoleErrorSpy).toHaveBeenCalledWith(
|
expect(consoleErrorSpy).not.toHaveBeenCalled();
|
||||||
expect.stringContaining('--- [TEST] UNHANDLED ERROR ---'),
|
|
||||||
expect.any(UniqueConstraintError),
|
|
||||||
);
|
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should handle a ValidationError with a 400 status and include the validation errors array', async () => {
|
it('should handle a ValidationError with a 400 status and include the validation errors array', async () => {
|
||||||
@@ -225,10 +209,7 @@ describe('errorHandler Middleware', () => {
|
|||||||
},
|
},
|
||||||
'Client Error on GET /validation-error: Input validation failed',
|
'Client Error on GET /validation-error: Input validation failed',
|
||||||
);
|
);
|
||||||
expect(consoleErrorSpy).toHaveBeenCalledWith(
|
expect(consoleErrorSpy).not.toHaveBeenCalled();
|
||||||
expect.stringContaining('--- [TEST] UNHANDLED ERROR ---'),
|
|
||||||
expect.any(ValidationError),
|
|
||||||
);
|
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should handle a DatabaseError with a 500 status and a generic message', async () => {
|
it('should handle a DatabaseError with a 500 status and a generic message', async () => {
|
||||||
@@ -238,7 +219,7 @@ describe('errorHandler Middleware', () => {
|
|||||||
// In test/dev, we now expect a stack trace for 5xx errors.
|
// In test/dev, we now expect a stack trace for 5xx errors.
|
||||||
expect(response.body.message).toBe('A database connection issue occurred.');
|
expect(response.body.message).toBe('A database connection issue occurred.');
|
||||||
expect(response.body.stack).toBeDefined();
|
expect(response.body.stack).toBeDefined();
|
||||||
expect(response.body.stack).toEqual(expect.any(String));
|
expect(response.body.errorId).toEqual(expect.any(String));
|
||||||
expect(mockLogger.error).toHaveBeenCalledWith(
|
expect(mockLogger.error).toHaveBeenCalledWith(
|
||||||
expect.objectContaining({
|
expect.objectContaining({
|
||||||
err: expect.any(DatabaseError),
|
err: expect.any(DatabaseError),
|
||||||
@@ -258,8 +239,14 @@ describe('errorHandler Middleware', () => {
|
|||||||
|
|
||||||
expect(response.status).toBe(401);
|
expect(response.status).toBe(401);
|
||||||
expect(response.body).toEqual({ message: 'Invalid Token' });
|
expect(response.body).toEqual({ message: 'Invalid Token' });
|
||||||
// 4xx errors log as warn
|
expect(mockLogger.warn).toHaveBeenCalledWith(
|
||||||
expect(mockLogger.warn).toHaveBeenCalled();
|
{
|
||||||
|
err: expect.any(Error),
|
||||||
|
statusCode: 401,
|
||||||
|
},
|
||||||
|
'Client Error on GET /unauthorized-error-no-status: Invalid Token',
|
||||||
|
);
|
||||||
|
expect(consoleErrorSpy).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should handle an UnauthorizedError with explicit status', async () => {
|
it('should handle an UnauthorizedError with explicit status', async () => {
|
||||||
@@ -267,6 +254,14 @@ describe('errorHandler Middleware', () => {
|
|||||||
|
|
||||||
expect(response.status).toBe(401);
|
expect(response.status).toBe(401);
|
||||||
expect(response.body).toEqual({ message: 'Invalid Token' });
|
expect(response.body).toEqual({ message: 'Invalid Token' });
|
||||||
|
expect(mockLogger.warn).toHaveBeenCalledWith(
|
||||||
|
{
|
||||||
|
err: expect.any(Error),
|
||||||
|
statusCode: 401,
|
||||||
|
},
|
||||||
|
'Client Error on GET /unauthorized-error-with-status: Invalid Token',
|
||||||
|
);
|
||||||
|
expect(consoleErrorSpy).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should call next(err) if headers have already been sent', () => {
|
it('should call next(err) if headers have already been sent', () => {
|
||||||
@@ -311,6 +306,7 @@ describe('errorHandler Middleware', () => {
|
|||||||
expect(response.body.message).toMatch(
|
expect(response.body.message).toMatch(
|
||||||
/An unexpected server error occurred. Please reference error ID: \w+/,
|
/An unexpected server error occurred. Please reference error ID: \w+/,
|
||||||
);
|
);
|
||||||
|
expect(response.body.stack).toBeUndefined();
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should return the actual error message for client errors (4xx) in production', async () => {
|
it('should return the actual error message for client errors (4xx) in production', async () => {
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
// src/middleware/errorHandler.ts
|
// src/middleware/errorHandler.ts
|
||||||
import { Request, Response, NextFunction } from 'express';
|
import { Request, Response, NextFunction } from 'express';
|
||||||
|
import crypto from 'crypto';
|
||||||
import { ZodError } from 'zod';
|
import { ZodError } from 'zod';
|
||||||
import {
|
import {
|
||||||
ForeignKeyConstraintError,
|
ForeignKeyConstraintError,
|
||||||
@@ -24,45 +25,73 @@ export const errorHandler = (err: Error, req: Request, res: Response, next: Next
|
|||||||
// Use the request-scoped logger if available, otherwise fall back to the global logger.
|
// Use the request-scoped logger if available, otherwise fall back to the global logger.
|
||||||
const log = req.log || logger;
|
const log = req.log || logger;
|
||||||
|
|
||||||
// --- Handle Zod Validation Errors ---
|
// --- Handle Zod Validation Errors (from validateRequest middleware) ---
|
||||||
if (err instanceof ZodError) {
|
if (err instanceof ZodError) {
|
||||||
log.warn({ err: err.flatten() }, 'Request validation failed');
|
const statusCode = 400;
|
||||||
return res.status(400).json({
|
const message = 'The request data is invalid.';
|
||||||
message: 'The request data is invalid.',
|
const errors = err.issues.map((e) => ({ path: e.path, message: e.message }));
|
||||||
errors: err.issues.map((e) => ({ path: e.path, message: e.message })),
|
log.warn({ err, validationErrors: errors, statusCode }, `Client Error on ${req.method} ${req.path}: ${message}`);
|
||||||
});
|
return res.status(statusCode).json({ message, errors });
|
||||||
}
|
}
|
||||||
|
|
||||||
// --- Handle Custom Operational Errors ---
|
// --- Handle Custom Operational Errors ---
|
||||||
if (err instanceof NotFoundError) {
|
if (err instanceof NotFoundError) {
|
||||||
log.info({ err }, 'Resource not found');
|
const statusCode = 404;
|
||||||
return res.status(404).json({ message: err.message });
|
log.warn({ err, statusCode }, `Client Error on ${req.method} ${req.path}: ${err.message}`);
|
||||||
|
return res.status(statusCode).json({ message: err.message });
|
||||||
}
|
}
|
||||||
|
|
||||||
if (err instanceof ValidationError) {
|
if (err instanceof ValidationError) {
|
||||||
log.warn({ err }, 'Validation error occurred');
|
const statusCode = 400;
|
||||||
return res.status(400).json({ message: err.message, errors: err.validationErrors });
|
log.warn(
|
||||||
|
{ err, validationErrors: err.validationErrors, statusCode },
|
||||||
|
`Client Error on ${req.method} ${req.path}: ${err.message}`,
|
||||||
|
);
|
||||||
|
return res.status(statusCode).json({ message: err.message, errors: err.validationErrors });
|
||||||
}
|
}
|
||||||
|
|
||||||
if (err instanceof UniqueConstraintError) {
|
if (err instanceof UniqueConstraintError) {
|
||||||
log.warn({ err }, 'Constraint error occurred');
|
const statusCode = 409;
|
||||||
return res.status(409).json({ message: err.message }); // Use 409 Conflict for unique constraints
|
log.warn({ err, statusCode }, `Client Error on ${req.method} ${req.path}: ${err.message}`);
|
||||||
|
return res.status(statusCode).json({ message: err.message }); // Use 409 Conflict for unique constraints
|
||||||
}
|
}
|
||||||
|
|
||||||
if (err instanceof ForeignKeyConstraintError) {
|
if (err instanceof ForeignKeyConstraintError) {
|
||||||
log.warn({ err }, 'Foreign key constraint violation');
|
const statusCode = 400;
|
||||||
return res.status(400).json({ message: err.message });
|
log.warn({ err, statusCode }, `Client Error on ${req.method} ${req.path}: ${err.message}`);
|
||||||
|
return res.status(statusCode).json({ message: err.message });
|
||||||
}
|
}
|
||||||
|
|
||||||
// --- Handle Generic Errors ---
|
// --- Handle Generic Client Errors (e.g., from express-jwt, or manual status setting) ---
|
||||||
// Log the full error object for debugging. The pino logger will handle redaction.
|
const status = (err as any).status || (err as any).statusCode;
|
||||||
log.error({ err }, 'An unhandled error occurred in an Express route');
|
if (status && status >= 400 && status < 500) {
|
||||||
|
log.warn({ err, statusCode: status }, `Client Error on ${req.method} ${req.path}: ${err.message}`);
|
||||||
|
return res.status(status).json({ message: err.message });
|
||||||
|
}
|
||||||
|
|
||||||
|
// --- Handle All Other (500-level) Errors ---
|
||||||
|
const errorId = crypto.randomBytes(4).toString('hex');
|
||||||
|
log.error(
|
||||||
|
{
|
||||||
|
err,
|
||||||
|
errorId,
|
||||||
|
req: { method: req.method, url: req.url, headers: req.headers, body: req.body },
|
||||||
|
},
|
||||||
|
`Unhandled API Error (ID: ${errorId})`,
|
||||||
|
);
|
||||||
|
|
||||||
|
// Also log to console in test environment for visibility in test runners
|
||||||
|
if (process.env.NODE_ENV === 'test') {
|
||||||
|
console.error(`--- [TEST] UNHANDLED ERROR (ID: ${errorId}) ---`, err);
|
||||||
|
}
|
||||||
|
|
||||||
// In production, send a generic message to avoid leaking implementation details.
|
// In production, send a generic message to avoid leaking implementation details.
|
||||||
if (process.env.NODE_ENV === 'production') {
|
if (process.env.NODE_ENV === 'production') {
|
||||||
return res.status(500).json({ message: 'An internal server error occurred.' });
|
return res.status(500).json({
|
||||||
|
message: `An unexpected server error occurred. Please reference error ID: ${errorId}`,
|
||||||
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
// In non-production environments (dev, test, etc.), send more details for easier debugging.
|
// In non-production environments (dev, test, etc.), send more details for easier debugging.
|
||||||
return res.status(500).json({ message: err.message, stack: err.stack });
|
return res.status(500).json({ message: err.message, stack: err.stack, errorId });
|
||||||
};
|
};
|
||||||
Reference in New Issue
Block a user