fix(forward): destroy outbound socket when client disconnects early

[bliuchak]

forward.ts (and forward_socks.ts) never destroyed the outbound request when the client-facing response closed first while the upstream target had accepted the connection but never responded - e.g. during server.close(true) shutdown, or the browser disconnecting. chain.ts already had this symmetry (sourceSocket.on('close', () => targetSocket .destroy())); forward.ts/forward_socks.ts didn't, leaving the outbound socket open indefinitely.

The originally reported diagnosis (missing client.destroy() in client.on('error')) turned out to be a no-op - Node already destroys the socket there internally - so the fix targets the actual gap instead.

Fixes #670

[Copilot]

Pull request overview

Fixes an outbound socket leak in the HTTP forwarders by ensuring upstream requests are torn down when the client-facing side closes before the upstream responds (e.g., during server.close(true) shutdown or early client disconnects).

Changes:

• Destroy upstream http.ClientRequest when the downstream ServerResponse closes in forward.ts and forward_socks.ts.
• Add an e2e regression test that reproduces the “upstream accepts but never responds” socket-leak scenario.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/forward.ts	Adds client-response close handling to destroy the upstream request/socket to prevent hangs/leaks.
src/forward_socks.ts	Mirrors the same upstream-destroy-on-downstream-close behavior for SOCKS-forwarded HTTP.
test/e2e/forward-socket-cleanup.js	New e2e test validating the outbound socket is destroyed when the client side disappears mid-request.

Comments suppressed due to low confidence (2)

src/forward.ts:154

• After adding the response 'close' handler that can destroy the upstream request, the upstream ClientRequest can emit 'error' after the client-facing response has already been closed/destroyed. In that case response.headersSent may still be false, and the handler below will try to set headers/end on a destroyed ServerResponse, which can throw (e.g. ERR_STREAM_DESTROYED / write after end) and potentially crash the process. Guard the error handler against already-closed responses (destroyed/writableEnded) before attempting to write an error response.

    client.on('error', (error: NodeJS.ErrnoException) => {
        if (response.headersSent) {
            resolve();
            return;
        }

src/forward_socks.ts:107

• Same pattern as forward.ts: if the client-facing response is already closed/destroyed (including cases where the new response 'close' handler destroys the upstream request), the ClientRequest 'error' handler can still run with headersSent === false and then attempts to write headers/body to a destroyed ServerResponse. Add a destroyed/writableEnded guard before writing the error response to avoid ERR_STREAM_DESTROYED/write-after-end failures.

    client.on('error', (error: NodeJS.ErrnoException) => {
        if (response.headersSent) {
            resolve();
            return;
        }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.