Recreate and fix error when client is closed during proxied request

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<void>
+  let canRespond: Promise<void>
+  let errorLogged: Promise<void>
+
+  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<void>(resolve => {
+      signalRequestReceived = resolve
+    })
+    canRespond = new Promise<void>(resolve => {
+      signalCanRespond = resolve
+    })
+    errorLogged = new Promise<void>(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<void>(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<void>(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')
+  })
+})

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