Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 39 additions & 6 deletions src/middleware/errorHandler.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { Request, Response, NextFunction } from 'express';
import { errorHandler } from '../middleware/errorHandler.js';
import { Request, Response, NextFunction } from 'express';import { errorHandler } from '../middleware/errorHandler.js';
import {
BadRequestError,
UnauthorizedError,
Expand All @@ -9,8 +8,7 @@ import {
TooManyRequestsError,
AppError,
} from '../errors/index.js';
import { ValidationError } from '../middleware/validate.js';
import { logger } from '../logger.js';
import { ValidationError } from '../middleware/validate.js';import { logger } from '../logger.js';
import type { ErrorEnvelope } from '../types/ResponseEnvelope.js';

jest.mock('../logger.js', () => ({
Expand All @@ -23,7 +21,7 @@ jest.mock('../logger.js', () => ({

describe('Error Handler', () => {
let mockReq: Partial<Request> & { id?: string };
let mockRes: Partial<Response>;
let mockRes: Partial<Response> & { destroy?: jest.Mock };
let mockNext: NextFunction;

beforeEach(() => {
Expand All @@ -33,6 +31,7 @@ describe('Error Handler', () => {
mockRes = {
status: jest.fn().mockReturnThis(),
json: jest.fn(),
destroy: jest.fn(),
headersSent: false
};
mockNext = jest.fn();
Expand Down Expand Up @@ -63,7 +62,7 @@ describe('Error Handler', () => {
code: 'BAD_REQUEST',
message: 'Test bad request',
});
expect(typeof call.timestamp).toBe('string');
expect(typeof call.timestamp).toBe(typeof 'string');

expect(logger.error).toHaveBeenCalledWith(
'[errorHandler]',
Expand Down Expand Up @@ -141,6 +140,40 @@ describe('Error Handler', () => {
expect(mockRes.json).not.toHaveBeenCalled();
});

it('should destroy the socket when headers are already sent', () => {
mockRes.headersSent = true;
const error = new Error('mid-stream failure');

errorHandler(
error,
mockReq as Request,
mockRes as Response<ErrorEnvelope>,
mockNext
);

expect(mockRes.status).not.toHaveBeenCalled();
expect(mockRes.json).not.toHaveBeenCalled();
expect(mockRes.destroy).toHaveBeenCalledWith(error);
});

it('logs the error once with requestId when headers are already sent', () => {
mockRes.headersSent = true;
const error = new Error('mid-stream failure');

errorHandler(
error,
mockReq as Request,
mockRes as Response<ErrorEnvelope>,
mockNext
);

expect(logger.error).toHaveBeenCalledTimes(1);
expect(logger.error).toHaveBeenCalledWith(
'[errorHandler]',
expect.objectContaining({ requestId: 'test-request-id' })
);
});

it('should include explicit catalog code when provided', () => {
const error = new AppError('Custom error', 422, 'UNPROCESSABLE_ENTITY');

Expand Down
16 changes: 10 additions & 6 deletions src/middleware/errorHandler.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
import type { Request, Response, NextFunction } from 'express';
import { isAppError } from '../errors/index.js';
import { logger } from '../logger.js';
import { isAppError } from '../errors/index.js';import { logger } from '../logger.js';
import type { ValidationErrorDetail } from './validate.js';
import { ValidationError } from './validate.js';
import { buildErrorEnvelope } from './envelope.js';
import type { ErrorEnvelope } from '../types/ResponseEnvelope.js';
import { normalizeError } from '../errors/errorEnvelopePolicy.js';
import { ValidationError } from './validate.js';import { buildErrorEnvelope } from './envelope.js';import type { ErrorEnvelope } from '../types/ResponseEnvelope.js';import { normalizeError } from '../errors/errorEnvelopePolicy.js';

const isProduction = process.env.NODE_ENV === "production";

Expand All @@ -32,6 +28,7 @@ function extractValidationDetails(err: unknown): ValidationErrorDetail[] | undef
* - Returns consistent JSON envelope: { success: false, error: { code, message }, requestId, timestamp }
* - Never sends stack traces to the client in production
* - Logs full error server-side
* - When headers are already sent, destroys the socket so the client sees a terminated stream
*/
export function errorHandler(
err: unknown,
Expand Down Expand Up @@ -66,6 +63,13 @@ export function errorHandler(

if (!res.headersSent) {
res.status(statusCode).json(body);
} else {
// Headers already flushed: we cannot write a JSON envelope.
// Terminate the socket so the client observes a truncated stream
// instead of hanging until its own timeout.
if (typeof res.destroy === 'function') {
res.destroy(err instanceof Error ? err : undefined);
}
}

const logData = {
Expand Down
20 changes: 20 additions & 0 deletions src/routes/proxyRoutes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,26 @@ export function createProxyRouter(deps: ProxyDeps): Router {
} catch (err: unknown) {
let outcome: UpstreamOutcome = 'error';

// If headers have already been flushed to the client, we cannot send a
// structured error response. Destroy the socket so the client sees a
// terminated stream instead of hanging on a truncated body. Log once
// with the requestId for observability.
if (res.headersSent) {
logger.error(
{
err,
requestId,
apiId: String(apiEntry.id),
endpointId: endpoint.endpointId,
upstreamStatus,
},
'Proxy error after headers sent; destroying response socket',
);
timer.stop(upstreamStatus, outcome);
res.destroy(err instanceof Error ? err : undefined);
return;
}

if (err instanceof CircuitBreakerOpenError) {
// Circuit breaker open — don't bill the caller
upstreamStatus = 502;
Expand Down