Skip to content

Commit

Permalink
fix(ClientRequest): destroy socket when destroying IncomingMessage (#597
Browse files Browse the repository at this point in the history
)
  • Loading branch information
kettanaito committed Jul 6, 2024
1 parent fedac45 commit 8f041da
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 12 deletions.
16 changes: 16 additions & 0 deletions src/interceptors/ClientRequest/MockHttpSocket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
26 changes: 14 additions & 12 deletions src/interceptors/ClientRequest/index.test.ts
Original file line number Diff line number Diff line change
@@ -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 '.'
Expand All @@ -21,6 +21,10 @@ beforeAll(async () => {
await httpServer.listen()
})

afterEach(() => {
interceptor.removeAllListeners()
})

afterAll(async () => {
interceptor.dispose()
await httpServer.close()
Expand Down Expand Up @@ -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<void>()
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<void>()
request.on('error', function (err) {
expect(err.name).toEqual('AbortError')
requestAborted.resolve()
const abortErrorPromise = new DeferredPromise<Error>()
request.on('error', function (error) {
abortErrorPromise.resolve(error)
})

await requestAborted
const abortError = await abortErrorPromise
expect(abortError.name).toEqual('AbortError')

expect(request.destroyed).toBe(true)
})
68 changes: 68 additions & 0 deletions test/modules/http/compliance/http-res-destroy.test.ts
Original file line number Diff line number Diff line change
@@ -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'))
})

0 comments on commit 8f041da

Please sign in to comment.