From 362c689226ac29c15c4572c42d525d3f5f348d2a Mon Sep 17 00:00:00 2001 From: George Waters Date: Tue, 30 Dec 2025 12:21:18 +0000 Subject: [PATCH 1/4] fix: early return from passthrough if no event listeners are registered --- .../ClientRequest/MockHttpSocket.ts | 60 +++++++++++-------- src/utils/handleRequest.ts | 10 ++++ .../http-passthrough-early-return.test.ts | 58 ++++++++++++++++++ 3 files changed, 104 insertions(+), 24 deletions(-) create mode 100644 test/modules/http/compliance/http-passthrough-early-return.test.ts diff --git a/src/interceptors/ClientRequest/MockHttpSocket.ts b/src/interceptors/ClientRequest/MockHttpSocket.ts index 2f4c28f03..fa70cc7ce 100644 --- a/src/interceptors/ClientRequest/MockHttpSocket.ts +++ b/src/interceptors/ClientRequest/MockHttpSocket.ts @@ -263,36 +263,48 @@ export class MockHttpSocket extends MockSocket { } } - // Forward TLS Socket properties onto this Socket instance - // in the case of a TLS/SSL connection. - if (Reflect.get(socket, 'encrypted')) { - const tlsProperties = [ - 'encrypted', - 'authorized', - 'getProtocol', - 'getSession', - 'isSessionReused', - 'getCipher', - ] - - tlsProperties.forEach((propertyName) => { - Object.defineProperty(this, propertyName, { - enumerable: true, - get: () => { - const value = Reflect.get(socket, propertyName) - return typeof value === 'function' ? value.bind(socket) : value - }, - }) - }) - } - socket .on('lookup', (...args) => this.emit('lookup', ...args)) .on('connect', () => { this.connecting = socket.connecting this.emit('connect') }) - .on('secureConnect', () => this.emit('secureConnect')) + .on('secureConnect', () => { + /** + * Forward TLS Socket properties onto this Socket instance + * after the TLS handshake completes. This ensures the real socket + * has valid TLS information before we start forwarding it. + * + * We do this on 'secureConnect' rather than immediately in passthrough() + * because TLSSocket.encrypted is true even before the socket connects, + * but getCipher(), getProtocol() etc. return undefined until the + * TLS handshake completes. By waiting until secureConnect, we allow + * the mock TLS properties (set in constructor) to remain accessible + * until real values are available. + */ + if (Reflect.get(socket, 'encrypted')) { + const tlsProperties = [ + 'encrypted', + 'authorized', + 'getProtocol', + 'getSession', + 'isSessionReused', + 'getCipher', + ] + + tlsProperties.forEach((propertyName) => { + Object.defineProperty(this, propertyName, { + enumerable: true, + get: () => { + const value = Reflect.get(socket, propertyName) + return typeof value === 'function' ? value.bind(socket) : value + }, + }) + }) + } + + this.emit('secureConnect') + }) .on('secure', () => this.emit('secure')) .on('session', (session) => this.emit('session', session)) .on('ready', () => this.emit('ready')) diff --git a/src/utils/handleRequest.ts b/src/utils/handleRequest.ts index 4ef6fcd5a..01d5ebc0c 100644 --- a/src/utils/handleRequest.ts +++ b/src/utils/handleRequest.ts @@ -23,6 +23,16 @@ interface HandleRequestOptions { export async function handleRequest( options: HandleRequestOptions ): Promise { + /** + * @note If there are no "request" event listeners, passthrough immediately + * without going through the full async machinery. This reduces the number + * of microtask checkpoints, which helps avoid timing issues with + * high-concurrency requests (e.g., EPIPE errors with Unix sockets). + */ + if (options.emitter.listenerCount('request') === 0) { + return options.controller.passthrough() + } + const handleResponse = async ( response: Response | Error | Record ) => { diff --git a/test/modules/http/compliance/http-passthrough-early-return.test.ts b/test/modules/http/compliance/http-passthrough-early-return.test.ts new file mode 100644 index 000000000..7ecd0404d --- /dev/null +++ b/test/modules/http/compliance/http-passthrough-early-return.test.ts @@ -0,0 +1,58 @@ +// @vitest-environment node +import { it, expect, beforeAll, afterEach, afterAll } from 'vitest' +import http from 'node:http' +import { HttpServer } from '@open-draft/test-server/http' +import { ClientRequestInterceptor } from '../../../../src/interceptors/ClientRequest' +import { waitForClientRequest } from '../../../helpers' + +const httpServer = new HttpServer((app) => { + app.get('/resource', (req, res) => { + res.send('ok') + }) +}) + +const interceptor = new ClientRequestInterceptor() + +beforeAll(async () => { + await httpServer.listen() + interceptor.apply() +}) + +afterEach(() => { + interceptor.removeAllListeners() +}) + +afterAll(async () => { + interceptor.dispose() + await httpServer.close() +}) + +/** + * When no request listeners are registered, the interceptor should call + * passthrough() immediately without going through the full async machinery. + * This prevents timing issues with high-concurrency requests. + * @see https://github.com/mswjs/interceptors/issues/760 + */ +it('performs passthrough when no request listeners are attached', async () => { + const request = http.request(httpServer.http.url('/resource')) + request.end() + const { res, text } = await waitForClientRequest(request) + + expect(res.statusCode).toBe(200) + expect(await text()).toBe('ok') +}) + +it('handles concurrent requests when no listeners are attached', async () => { + const requests = Array.from({ length: 20 }, () => { + const req = http.request(httpServer.http.url('/resource')) + req.end() + return waitForClientRequest(req) + }) + + const results = await Promise.all(requests) + + for (const { res, text } of results) { + expect(res.statusCode).toBe(200) + expect(await text()).toBe('ok') + } +}) From 362fa0e56db3b9c27e676cd6f8e560aecf8aca25 Mon Sep 17 00:00:00 2001 From: George Waters Date: Tue, 30 Dec 2025 12:23:52 +0000 Subject: [PATCH 2/4] chore: rewrite tests to run over a unix socket --- .../http-passthrough-early-return.test.ts | 40 +++++++++++-------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/test/modules/http/compliance/http-passthrough-early-return.test.ts b/test/modules/http/compliance/http-passthrough-early-return.test.ts index 7ecd0404d..9d7698a4b 100644 --- a/test/modules/http/compliance/http-passthrough-early-return.test.ts +++ b/test/modules/http/compliance/http-passthrough-early-return.test.ts @@ -1,40 +1,43 @@ // @vitest-environment node -import { it, expect, beforeAll, afterEach, afterAll } from 'vitest' +import { it, expect, beforeAll, afterAll } from 'vitest' import http from 'node:http' -import { HttpServer } from '@open-draft/test-server/http' +import path from 'node:path' +import { promisify } from 'node:util' import { ClientRequestInterceptor } from '../../../../src/interceptors/ClientRequest' import { waitForClientRequest } from '../../../helpers' -const httpServer = new HttpServer((app) => { - app.get('/resource', (req, res) => { - res.send('ok') - }) +const HTTP_SOCKET_PATH = path.join(__dirname, './test-early-return.sock') + +const httpServer = http.createServer((req, res) => { + res.writeHead(200) + res.end('ok') }) const interceptor = new ClientRequestInterceptor() beforeAll(async () => { - await httpServer.listen() + await new Promise((resolve) => { + httpServer.listen(HTTP_SOCKET_PATH, resolve) + }) interceptor.apply() }) -afterEach(() => { - interceptor.removeAllListeners() -}) - afterAll(async () => { interceptor.dispose() - await httpServer.close() + await promisify(httpServer.close.bind(httpServer))() }) /** * When no request listeners are registered, the interceptor should call * passthrough() immediately without going through the full async machinery. - * This prevents timing issues with high-concurrency requests. + * This prevents timing issues with high-concurrency Unix socket requests. * @see https://github.com/mswjs/interceptors/issues/760 */ -it('performs passthrough when no request listeners are attached', async () => { - const request = http.request(httpServer.http.url('/resource')) +it('performs passthrough over a Unix socket when no request listeners are attached', async () => { + const request = http.request({ + socketPath: HTTP_SOCKET_PATH, + path: '/resource', + }) request.end() const { res, text } = await waitForClientRequest(request) @@ -42,9 +45,12 @@ it('performs passthrough when no request listeners are attached', async () => { expect(await text()).toBe('ok') }) -it('handles concurrent requests when no listeners are attached', async () => { +it('handles concurrent Unix socket requests when no listeners are attached', async () => { const requests = Array.from({ length: 20 }, () => { - const req = http.request(httpServer.http.url('/resource')) + const req = http.request({ + socketPath: HTTP_SOCKET_PATH, + path: '/resource', + }) req.end() return waitForClientRequest(req) }) From 82a6efba53240f5886e9bf21b238366f7324d4a5 Mon Sep 17 00:00:00 2001 From: George Waters Date: Mon, 5 Jan 2026 12:14:48 +0000 Subject: [PATCH 3/4] chore: use invariant for socket.encrypted check --- .../ClientRequest/MockHttpSocket.ts | 41 ++++++++++--------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/src/interceptors/ClientRequest/MockHttpSocket.ts b/src/interceptors/ClientRequest/MockHttpSocket.ts index fa70cc7ce..085a866ac 100644 --- a/src/interceptors/ClientRequest/MockHttpSocket.ts +++ b/src/interceptors/ClientRequest/MockHttpSocket.ts @@ -282,26 +282,29 @@ export class MockHttpSocket extends MockSocket { * the mock TLS properties (set in constructor) to remain accessible * until real values are available. */ - if (Reflect.get(socket, 'encrypted')) { - const tlsProperties = [ - 'encrypted', - 'authorized', - 'getProtocol', - 'getSession', - 'isSessionReused', - 'getCipher', - ] - - tlsProperties.forEach((propertyName) => { - Object.defineProperty(this, propertyName, { - enumerable: true, - get: () => { - const value = Reflect.get(socket, propertyName) - return typeof value === 'function' ? value.bind(socket) : value - }, - }) + invariant( + Reflect.get(socket, 'encrypted'), + 'Expected socket to have property `encrypted`' + ) + + const tlsProperties = [ + 'encrypted', + 'authorized', + 'getProtocol', + 'getSession', + 'isSessionReused', + 'getCipher', + ] + + tlsProperties.forEach((propertyName) => { + Object.defineProperty(this, propertyName, { + enumerable: true, + get: () => { + const value = Reflect.get(socket, propertyName) + return typeof value === 'function' ? value.bind(socket) : value + }, }) - } + }) this.emit('secureConnect') }) From cb09e5f47e6eac9b131186c9305731f24ea9de95 Mon Sep 17 00:00:00 2001 From: George Waters Date: Mon, 5 Jan 2026 12:15:11 +0000 Subject: [PATCH 4/4] chore(test): use `expect await` rather than `await expect` --- .../http/compliance/http-passthrough-early-return.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/modules/http/compliance/http-passthrough-early-return.test.ts b/test/modules/http/compliance/http-passthrough-early-return.test.ts index 9d7698a4b..5b17316fb 100644 --- a/test/modules/http/compliance/http-passthrough-early-return.test.ts +++ b/test/modules/http/compliance/http-passthrough-early-return.test.ts @@ -42,7 +42,7 @@ it('performs passthrough over a Unix socket when no request listeners are attach const { res, text } = await waitForClientRequest(request) expect(res.statusCode).toBe(200) - expect(await text()).toBe('ok') + await expect(text()).resolves.toBe('ok') }) it('handles concurrent Unix socket requests when no listeners are attached', async () => { @@ -59,6 +59,6 @@ it('handles concurrent Unix socket requests when no listeners are attached', asy for (const { res, text } of results) { expect(res.statusCode).toBe(200) - expect(await text()).toBe('ok') + await expect(text()).resolves.toBe('ok') } })