diff --git a/src/middleware/errorHandler.test.ts b/src/middleware/errorHandler.test.ts index b980689..6409e1a 100644 --- a/src/middleware/errorHandler.test.ts +++ b/src/middleware/errorHandler.test.ts @@ -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, @@ -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', () => ({ @@ -23,7 +21,7 @@ jest.mock('../logger.js', () => ({ describe('Error Handler', () => { let mockReq: Partial & { id?: string }; - let mockRes: Partial; + let mockRes: Partial & { destroy?: jest.Mock }; let mockNext: NextFunction; beforeEach(() => { @@ -33,6 +31,7 @@ describe('Error Handler', () => { mockRes = { status: jest.fn().mockReturnThis(), json: jest.fn(), + destroy: jest.fn(), headersSent: false }; mockNext = jest.fn(); @@ -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]', @@ -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, + 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, + 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'); diff --git a/src/middleware/errorHandler.ts b/src/middleware/errorHandler.ts index ebe5a38..b1a4130 100644 --- a/src/middleware/errorHandler.ts +++ b/src/middleware/errorHandler.ts @@ -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"; @@ -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, @@ -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 = { diff --git a/src/routes/proxyRoutes.ts b/src/routes/proxyRoutes.ts index 95aa201..8ea2627 100644 --- a/src/routes/proxyRoutes.ts +++ b/src/routes/proxyRoutes.ts @@ -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;