fix(ClientRequest): remove passthrough socket listeners on close

[Stanzilla]

Summary

This PR fixes a memory leak that occurs when using passthrough mode in Node.js 20+.

The Problem

When MockHttpSocket.passthrough() is called, event listeners are attached to the original socket (lookup, connect, secureConnect, secure, session, ready, drain, data, error, resume, timeout, prefinish, finish, close, end) but they are never removed when the socket closes.

This causes memory leaks in long-running processes, as reported in mswjs/msw#2537.

The Fix

Call socket.removeAllListeners() in the close event handler before emitting the close event to the MockHttpSocket. This ensures all listeners attached to the original socket are properly cleaned up.

.on('close', (hadError) => {
  // Remove all listeners from the original socket to prevent memory leaks.
  // @see https://github.com/mswjs/msw/issues/2537
  socket.removeAllListeners()
  this.emit('close', hadError)
})

Test

Added a regression test that makes many passthrough requests and verifies no MaxListenersExceededWarning is emitted.

Fixes mswjs/msw#2537

[kettanaito]

Thanks for working on this, @Stanzilla. This looks great. One issue here is that the test you've added passes even if your fix is reverted. I'm trying to add a direct test on the socket listeners instead, but that proves problematic since request's socket is our MockHttpSocket. The underlying passthrough socket instance is never exposed.

[kettanaito]

Discovery

After some digging, I discovered that we were incorrectly handling closure forwarding between the mock and the passthrough sockets. We always forward passthrough closures as client-facing closures (in the close listener on the passthrough socket), which makes it much easier to clean up the passthrough socket on the this.on('close') listener on the mock socket.

We also had a bug where we were listening to the this.on('error') but that only gets fired for non-graceful socket closures. When the socket is destroyed, it's close then error (if the socket was destroyed with an error).

I've fixed that, removed your test, added a new test that asserts directly on the passthrough socket, and things seems to pass now.

[kettanaito]

Now, the only thing left to do is to get rid of kPassthroughSocket because it's needless and I don't want to keep it around just for testing purposes.

Update: Turned out, we already had this.originalSocket to store the passthrough socket. Nice! Getting rid of the symbol.

[Stanzilla]

Awesome, thank you for getting it over the finish line!

[kettanaito]

Sadly, the release is currently blocked by #759. I will get to it at the beginning of the year.

[kettanaito]

Released: v0.41.0 🎉

This has been released in v0.41.0.

• 📄 Release notes
• 📦 View on npm

Get these changes by running the following command:

npm i @mswjs/interceptors@latest

---

Predictable release automation by Release.