forward.ts: Socket leak when client.on('error') fires — missing client.destroy()

[Jobs-Chen]

Description
When forward() encounters an error on the upstream http.ClientRequest (e.g. ECONNRESET, ECONNREFUSED, socket hang-up), the client.on('error') handler calls resolve() but never calls client.destroy(). This leaves the underlying TCP socket open even after the Agent has removed it from its socket pool.
Root cause
In src/forward.ts (line ~91):
client.on('error', (error: NodeJS.ErrnoException) => {
if (response.headersSent) {
resolve();
return;
}

const statusCode = errorCodeToStatusCode[error.code!] ?? badGatewayStatusCodes.GENERIC_ERROR;

response.statusCode = !proxy && error.code === 'ENOTFOUND' ? 404 : statusCode;
response.setHeader('content-type', 'text/plain; charset=utf-8');
response.end(http.STATUS_CODES[response.statusCode]);

resolve();

});
When http.ClientRequest emits 'error', Node.js internally emits 'agentRemove' on the socket, which removes it from the Agent's sockets map. However, the socket itself is not destroyed. Since agent.destroy() only destroys sockets still tracked by the Agent, these "orphaned" sockets are unreachable and remain open indefinitely.
This is asymmetric with chain.ts, which correctly does:
sourceSocket.on('close', () => client.destroy());
Impact

• After server.close(true) + agent.destroy(), orphaned sockets keep the Node.js process alive, causing a delayed exit (observed ~3s in production with 40 concurrent browser instances).
• The issue is intermittent — it only manifests when the error occurs during an active forward() request (i.e., the createSocket async window overlaps with server.close()).
  Reproduction

1. Start a ProxyServer with a custom httpAgent (keepAlive: true) via prepareRequestFunction.

2. Have a client (e.g. Puppeteer with --proxy-server) make an HTTP (non-CONNECT) request through the proxy.

3. While the request is in flight, call server.close(true) and agent.destroy().

4. Observe that process._getActiveHandles() still contains Socket objects whose connect stack traces back to forward.ts.
   Suggested fix
   Add client.destroy() in the error handler:
   client.on('error', (error: NodeJS.ErrnoException) => {
   client.destroy();

   if (response.headersSent) {
   resolve();
   return;
   }

   const statusCode = errorCodeToStatusCode[error.code!] ?? badGatewayStatusCodes.GENERIC_ERROR;

   response.statusCode = !proxy && error.code === 'ENOTFOUND' ? 404 : statusCode;
   response.setHeader('content-type', 'text/plain; charset=utf-8');
   response.end(http.STATUS_CODES[response.statusCode]);

   resolve();
   });
   Environment

• proxy-chain: 3.0.0
• Node.js: v22.18.0
• OS: Ubuntu 22.04 (Docker)

[bliuchak]

@Jobs-Chen hey, thank you for the issue. I'll take a look into it.

[bliuchak]

Thanks again for the detailed report and clear repro steps - this pointed at a real bug, though after digging in the actual mechanism turned out to be a bit different from what's described above.

On the specific diagnosis: I checked Node's _http_client.js internals and reproduced it directly - when the upstream http.ClientRequest errors (ECONNRESET, ECONNREFUSED, etc.), Node itself already calls socket.destroy() synchronously as part of handling that 'error' event, before forward.ts's own handler even finishes running. I confirmed this holds across Node 20/22/24. So adding client.destroy() inside client.on('error', ...) turns out to be a no-op there - I verified this by patching only that change onto the code and the leak still reproduced.

The actual bug: forward.ts (and forward_socks.ts, same pattern) never destroyed the outbound request to the target when the client-facing side disappeared first - e.g. server.close(true) during shutdown, or the browser disconnecting - while the target had accepted the connection but was still hanging (no response, no error yet). chain.ts already has this symmetry (sourceSocket.on('close', () => targetSocket.destroy())); forward.ts didn't. That asymmetry is what leaves the socket open indefinitely.

One nuance: this fix reliably closes the leak when the custom httpAgent isn't separately destroyed (the common case). Calling agent.destroy() right alongside close(true), as in your repro's step 3, didn't reproduce a leak for me even pre-fix - Node's Agent.destroy() already handles an already-connected socket fine there. If you can retest against the fix under your original 40-instance load, that'd help confirm it's fully resolved.

Fix: added response.on('close', () => client.destroy()) to both forward.ts and forward_socks.ts, plus a regression test that fails against the old code and passes with the fix. PR: #671

Thanks again for the clear write-up and reproduction steps - they're what made it possible to track down the actual mechanism.

[bliuchak]

@Jobs-Chen Btw, could you please try the fix from #671 against your original 40-concurrent-instance setup and confirm whether the delayed exit is resolved? Also, in your actual shutdown path, are you calling agent.destroy() alongside server.close(true), or just close(true) alone? That distinction changed the outcome in my testing, so knowing which matches your real code would help confirm we've closed the right gap.

[Jobs-Chen]

Thanks for the fix in #671! I upgraded to 3.0.1-beta.4 and the forward.ts leak is resolved. To answer your question: in our shutdown path we only call server.close(true), we do not separately call agent.destroy() — so the fix applies to our case.

However, we're still seeing a delayed exit (~3s). After adding debug logging to our fallback process.exit(0) timer, process._getActiveHandles() shows 4 dangling sockets:

active handles: 4, active requests: 0
  handle[0] Socket: destroyed=false, remote=undefined:undefined, local=undefined:undefined, bytesRead=0, bytesWritten=0
  handle[1] Socket: destroyed=false, remote=undefined:undefined, local=undefined:undefined, bytesRead=0, bytesWritten=78808
  handle[2] Socket: destroyed=false, remote=undefined:undefined, local=undefined:undefined, bytesRead=0, bytesWritten=0
  handle[3] Socket: destroyed=false, remote=upstream-proxy-ip:upstream-proxy-port, local=local-ip:local-port, bytesRead=14, bytesWritten=59

handle[3] is an outbound socket to the upstream proxy (via CONNECT tunnel) — bytesRead=14, bytesWritten=59 indicates a CONNECT request/response was exchanged. The other three have remote=undefined — their TCP connections have closed, but destroyed=false.

I traced this to chain.ts and chain_socks.ts: when sourceSocket is destroyed by closeConnections(), the sourceSocket.on('close', ...) handler calls targetSocket.end() (graceful FIN) instead of targetSocket.destroy(). If the upstream proxy doesn't respond to FIN, the socket stays open. This is the same class of bug as #670, just in chain.ts / chain_socks.ts instead of forward.ts.

The fix would be to replace end() with destroy() in the close handlers of both files:

// chain.ts (lines ~107-119) and chain_socks.ts (lines ~71-83)
targetSocket.on('close', () => {
    sourceSocket.resume();
    if (sourceSocket.writable) {
        sourceSocket.destroy();  // was: sourceSocket.end()
    }
});
sourceSocket.on('close', () => {
    targetSocket.resume();
    if (targetSocket.writable) {
        targetSocket.destroy();  // was: targetSocket.end()
    }
});

The end() calls in the error/CONNECT-failure paths (e.g., sourceSocket.end(createCustomStatusHttpResponse(...))) should remain as-is since those send HTTP error responses before closing.