diff --git a/packages/express-utils/__tests__/express-utils.test.ts b/packages/express-utils/__tests__/express-utils.test.ts index 791fb46..1816210 100644 --- a/packages/express-utils/__tests__/express-utils.test.ts +++ b/packages/express-utils/__tests__/express-utils.test.ts @@ -1,9 +1,11 @@ import { test, describe, afterEach, beforeEach } from 'node:test' import assert from 'node:assert/strict' +import http from 'node:http' import { testApp } from '@digabi/testing' import express, { Express } from 'express' -import { AppError, DataError, setupDefaultErrorHandlers } from '../src' +import { AppError, DataError, ProxyError, proxyWithOpts, setupDefaultErrorHandlers } from '../src' import _ from 'lodash' +import { Logger } from 'winston' const errorMessages: unknown[] = [] const warnMessages: unknown[] = [] @@ -141,3 +143,96 @@ describe('express-utils', () => { }) }) }) + +describe('proxyWithOpts', () => { + let proxyApp: Express + let upstreamServer: http.Server + let upstreamPort: number + const proxyErrors: ProxyError[] = [] + + let signalRequestReceived: () => void + let signalCanRespond: () => void + let signalErrorLogged: () => void + let requestReceived: Promise + let canRespond: Promise + let errorLogged: Promise + + const proxyLogger = { + error: (...args: ProxyError[]) => { + proxyErrors.push(args[1]) + signalErrorLogged() + }, + warn: (...args: ProxyError[]) => { + proxyErrors.push(args[1]) + signalErrorLogged() + } + } + + beforeEach(async () => { + proxyErrors.length = 0 + + requestReceived = new Promise(resolve => { + signalRequestReceived = resolve + }) + canRespond = new Promise(resolve => { + signalCanRespond = resolve + }) + errorLogged = new Promise(resolve => { + signalErrorLogged = resolve + }) + + // Upstream server waits for signal before responding - eliminates race conditions + upstreamServer = http.createServer((_req, res) => { + signalRequestReceived() // Tell test: request arrived + // Wait for test signal, then respond + const respond = async () => { + await canRespond + res.writeHead(200, { 'content-type': 'text/plain' }) + res.end('data') + } + void respond() + }) + + await new Promise(resolve => { + upstreamServer.listen(0, () => { + upstreamPort = (upstreamServer.address() as { port: number }).port + resolve() + }) + }) + + proxyApp = express() + proxyApp.use('/proxy', proxyWithOpts(`http://localhost:${upstreamPort}`, {})) + + setupDefaultErrorHandlers(proxyApp, false, proxyLogger as unknown as Logger) + + return testApp.initApp(proxyApp)() + }) + + afterEach(async () => { + testApp.closeApp() + await new Promise(resolve => upstreamServer.close(() => resolve())) + }) + + test('client disconnect should not cause 500 error', async () => { + const port = Number(testApp.getServerPrefix().split(':')[2]) + + const req = http.request({ hostname: 'localhost', port, path: '/proxy/data' }) + req.on('error', () => {}) // Ignore client-side errors + req.end() + + await requestReceived // 1. Wait for request to reach upstream + req.destroy() // 2. Destroy client connection + // Small delay to ensure connection closure propagates to proxy's response + await new Promise(resolve => setTimeout(resolve, 10)) + signalCanRespond() // 3. Tell upstream to respond now (to destroyed connection) + + // Wait for error to be logged + await errorLogged + + const has500Error = proxyErrors.some(e => e.status === 500) + const has400Error = proxyErrors.some(e => e.status === 400) + + assert.equal(has500Error, false, 'Client disconnect should not cause 500 error') + assert.equal(has400Error, true, 'Client disconnect should cause 400 error') + }) +}) diff --git a/packages/express-utils/src/index.ts b/packages/express-utils/src/index.ts index ee63078..e347d0d 100644 --- a/packages/express-utils/src/index.ts +++ b/packages/express-utils/src/index.ts @@ -203,14 +203,17 @@ function proxyErrorHandling(next: NextFunction, err: ProxyErrorError, params: Pr // ERR_STREAM_PREMATURE_CLOSE and ECONNRESET happens when the user's browser cancels the request when data is sent to browser if (err && err.code === 'ECONNRESET') { next(new ProxyError(err, params, errorPrefix, 400, 'notice')) - } else if (err && (err.code === 'ECONNABORTED' || err.code === 'ERR_STREAM_PREMATURE_CLOSE')) { + } else if ( + (err && (err.code === 'ECONNABORTED' || err.code === 'ERR_STREAM_PREMATURE_CLOSE')) || + err.code === 'ERR_STREAM_UNABLE_TO_PIPE' // Happens when response stream is already closed (e.g. client disconnects) and pipeline tries to pipe more data to it + ) { next(new ProxyError(err, params, errorPrefix, 400, 'warning')) } else { next(new ProxyError(err, params, errorPrefix, 500, 'error')) } } -class ProxyError extends Error { +export class ProxyError extends Error { url: string | undefined method: string | undefined status: number