From 8f041dad5d8fc5e295dcbcf28b9fcd9083f792d5 Mon Sep 17 00:00:00 2001 From: Artem Zakharchenko Date: Sat, 6 Jul 2024 15:34:09 +0200 Subject: [PATCH] fix(ClientRequest): destroy socket when destroying IncomingMessage (#597) --- .../ClientRequest/MockHttpSocket.ts | 16 +++++ src/interceptors/ClientRequest/index.test.ts | 26 +++---- .../http/compliance/http-res-destroy.test.ts | 68 +++++++++++++++++++ 3 files changed, 98 insertions(+), 12 deletions(-) create mode 100644 test/modules/http/compliance/http-res-destroy.test.ts diff --git a/src/interceptors/ClientRequest/MockHttpSocket.ts b/src/interceptors/ClientRequest/MockHttpSocket.ts index 2e98cfff..b29b08da 100644 --- a/src/interceptors/ClientRequest/MockHttpSocket.ts +++ b/src/interceptors/ClientRequest/MockHttpSocket.ts @@ -140,6 +140,11 @@ export class MockHttpSocket extends MockSocket { // Normally, we shoud listen to the "close" event but it // can be suppressed by using the "emitClose: false" option. this.responseParser.free() + + if (error) { + this.emit('error', error) + } + return super.destroy(error) } @@ -153,6 +158,12 @@ export class MockHttpSocket extends MockSocket { } const socket = this.createConnection() + + // If the developer destroys the socket, destroy the original connection. + this.once('error', (error) => { + socket.destroy(error) + }) + this.address = socket.address.bind(socket) // Flush the buffered "socket.write()" calls onto @@ -308,6 +319,11 @@ export class MockHttpSocket extends MockSocket { serverResponse.removeHeader('connection') serverResponse.removeHeader('date') + // If the developer destroy the socket, gracefully destroy the response. + this.once('error', () => { + serverResponse.destroy() + }) + // Get the raw headers stored behind the symbol to preserve name casing. const headers = getRawFetchHeaders(response.headers) || response.headers for (const [name, value] of headers) { diff --git a/src/interceptors/ClientRequest/index.test.ts b/src/interceptors/ClientRequest/index.test.ts index 935b73e0..277713b6 100644 --- a/src/interceptors/ClientRequest/index.test.ts +++ b/src/interceptors/ClientRequest/index.test.ts @@ -1,5 +1,5 @@ -import { it, expect, beforeAll, afterAll } from 'vitest' -import http from 'http' +import { it, expect, beforeAll, afterEach, afterAll } from 'vitest' +import http from 'node:http' import { HttpServer } from '@open-draft/test-server/http' import { DeferredPromise } from '@open-draft/deferred-promise' import { ClientRequestInterceptor } from '.' @@ -21,6 +21,10 @@ beforeAll(async () => { await httpServer.listen() }) +afterEach(() => { + interceptor.removeAllListeners() +}) + afterAll(async () => { interceptor.dispose() await httpServer.close() @@ -60,25 +64,23 @@ it('forbids calling "respondWith" multiple times for the same request', async () it('abort the request if the abort signal is emitted', async () => { const requestUrl = httpServer.http.url('/') - const requestEmitted = new DeferredPromise() interceptor.on('request', async function delayedResponse({ request }) { - requestEmitted.resolve() - await sleep(10_000) + await sleep(1_000) request.respondWith(new Response()) }) const abortController = new AbortController() const request = http.get(requestUrl, { signal: abortController.signal }) - await requestEmitted - abortController.abort() - const requestAborted = new DeferredPromise() - request.on('error', function (err) { - expect(err.name).toEqual('AbortError') - requestAborted.resolve() + const abortErrorPromise = new DeferredPromise() + request.on('error', function (error) { + abortErrorPromise.resolve(error) }) - await requestAborted + const abortError = await abortErrorPromise + expect(abortError.name).toEqual('AbortError') + + expect(request.destroyed).toBe(true) }) diff --git a/test/modules/http/compliance/http-res-destroy.test.ts b/test/modules/http/compliance/http-res-destroy.test.ts new file mode 100644 index 00000000..82738422 --- /dev/null +++ b/test/modules/http/compliance/http-res-destroy.test.ts @@ -0,0 +1,68 @@ +// @vitest-environment node +import { vi, it, expect, beforeAll, afterEach, afterAll } from 'vitest' +import http from 'node:http' +import { ClientRequestInterceptor } from '../../../../src/interceptors/ClientRequest' +import { HttpServer } from '@open-draft/test-server/lib/http' +import { waitForClientRequest } from '../../../helpers' + +const httpServer = new HttpServer((app) => { + app.get('/', (req, res) => res.sendStatus(200)) +}) + +const interceptor = new ClientRequestInterceptor() + +beforeAll(async () => { + interceptor.apply() + await httpServer.listen() +}) + +afterEach(() => { + interceptor.removeAllListeners() +}) + +afterAll(async () => { + interceptor.dispose() + await httpServer.close() +}) + +it('emits the "error" event when a bypassed response is destroyed', async () => { + const socketErrorListener = vi.fn() + + const request = http + .get(httpServer.http.url('/')) + .on('socket', (socket) => { + socket.on('error', socketErrorListener) + }) + .on('response', (response) => { + response.destroy(new Error('reason')) + }) + + const { res } = await waitForClientRequest(request) + + expect(res.destroyed).toBe(true) + expect(socketErrorListener).toHaveBeenCalledOnce() + expect(socketErrorListener).toHaveBeenCalledWith(new Error('reason')) +}) + +it('emits the "error" event when a mocked response is destroyed', async () => { + interceptor.on('request', ({ request }) => { + request.respondWith(new Response('hello world')) + }) + + const socketErrorListener = vi.fn() + + const request = http + .get(httpServer.http.url('/')) + .on('socket', (socket) => { + socket.on('error', socketErrorListener) + }) + .on('response', (response) => { + response.destroy(new Error('reason')) + }) + + const { res } = await waitForClientRequest(request) + + expect(res.destroyed).toBe(true) + expect(socketErrorListener).toHaveBeenCalledOnce() + expect(socketErrorListener).toHaveBeenCalledWith(new Error('reason')) +})