diff --git a/test/http2-dispatcher.js b/test/http2-dispatcher.js index f14a29099ac..bbdd2e112a3 100644 --- a/test/http2-dispatcher.js +++ b/test/http2-dispatcher.js @@ -239,8 +239,8 @@ test('Dispatcher#Upgrade - Should throw on non-websocket upgrade', async t => { allowH2: true }) - after(() => server.close()) after(() => client.close()) + after(() => server.close()) try { await client.upgrade({ path: '/', protocol: 'any' }) @@ -334,6 +334,8 @@ test('Dispatcher#Upgrade resumes queued requests after successful WebSocket upgr }, allowH2: true }) + after(() => client.close()) + after(() => server.close()) let upgradeSocket @@ -355,8 +357,6 @@ test('Dispatcher#Upgrade resumes queued requests after successful WebSocket upgr } finally { upgradeSocket?.on('error', () => {}) upgradeSocket?.end() - await client.close() - await new Promise((resolve) => server.close(resolve)) } await t.completed diff --git a/test/interceptors/redirect.js b/test/interceptors/redirect.js index 716379d65da..cabe5c1f47f 100644 --- a/test/interceptors/redirect.js +++ b/test/interceptors/redirect.js @@ -828,6 +828,8 @@ test('same-origin redirect preserves plain object headers with polluted Object.p res.end('redirected') }).listen(0) + after(() => server.close()) + const originalIterator = Object.prototype[Symbol.iterator] // eslint-disable-next-line no-extend-native Object.prototype[Symbol.iterator] = function * () {} @@ -851,7 +853,6 @@ test('same-origin redirect preserves plain object headers with polluted Object.p // eslint-disable-next-line no-extend-native Object.prototype[Symbol.iterator] = originalIterator } - server.close() } }) diff --git a/test/ip-prioritization.js b/test/ip-prioritization.js index 1774ef9490b..8dc0412fb25 100644 --- a/test/ip-prioritization.js +++ b/test/ip-prioritization.js @@ -1,6 +1,6 @@ 'use strict' -const { test } = require('node:test') +const { test, after } = require('node:test') const { Client } = require('..') const { createServer } = require('node:http') const { once } = require('node:events') @@ -30,21 +30,18 @@ test('HTTP/1.1 Request Prioritization', async (t) => { return socket } }) + after(() => client.close()) + after(() => server.close()) - try { - await client.request({ - path: '/', - method: 'GET', - typeOfService: 42 - }) + await client.request({ + path: '/', + method: 'GET', + typeOfService: 42 + }) - // Check if priority was set - if (priority !== 42) { - throw new Error(`Expected priority 42, got ${priority}`) - } - } finally { - await client.close() - server.close() + // Check if priority was set + if (priority !== 42) { + throw new Error(`Expected priority 42, got ${priority}`) } }) diff --git a/test/pool-connection-error-memory-leak.js b/test/pool-connection-error-memory-leak.js index 7ecaa601c4e..efca85df63b 100644 --- a/test/pool-connection-error-memory-leak.js +++ b/test/pool-connection-error-memory-leak.js @@ -1,6 +1,6 @@ 'use strict' -const { test } = require('node:test') +const { test, after } = require('node:test') const assert = require('node:assert') const { Pool } = require('..') const { createServer } = require('node:http') @@ -16,87 +16,84 @@ test('Pool client count does not grow on repeated connection errors', async (t) bodyTimeout: 100, headersTimeout: 100 }) + after(() => pool.close()) + + const clientCounts = [] + + // Track initial client count + clientCounts.push(pool[kClients].length) + + // Make several requests that will fail with connection errors + const requests = 5 + for (let i = 0; i < requests; i++) { + try { + await pool.request({ + path: `/${i}`, + method: 'GET' + }) + assert.fail('Request should have failed with a connection error') + } catch (err) { + // We expect connection errors, but the error might be wrapped + assert.ok( + err.code === 'ECONNREFUSED' || + err.cause?.code === 'ECONNREFUSED' || + err.code === 'UND_ERR_CONNECT', + `Expected connection error but got: ${err.message} (${err.code})` + ) + } - try { - const clientCounts = [] - - // Track initial client count + // Track client count after each request clientCounts.push(pool[kClients].length) - // Make several requests that will fail with connection errors - const requests = 5 - for (let i = 0; i < requests; i++) { - try { - await pool.request({ - path: `/${i}`, - method: 'GET' - }) - assert.fail('Request should have failed with a connection error') - } catch (err) { - // We expect connection errors, but the error might be wrapped - assert.ok( - err.code === 'ECONNREFUSED' || - err.cause?.code === 'ECONNREFUSED' || - err.code === 'UND_ERR_CONNECT', - `Expected connection error but got: ${err.message} (${err.code})` - ) - } - - // Track client count after each request - clientCounts.push(pool[kClients].length) - - // Small delay to allow for client cleanup - await new Promise(resolve => setTimeout(resolve, 10)) - } + // Small delay to allow for client cleanup + await new Promise(resolve => setTimeout(resolve, 10)) + } - // The key test: verify that client count doesn't increase monotonically, - // which would indicate the memory leak that was fixed - const maxCount = Math.max(...clientCounts) - assert.ok( - clientCounts[clientCounts.length - 1] <= maxCount, - `Client count should not increase continuously. Counts: ${clientCounts.join(', ')}` - ) - - // Ensure the last two counts are similar (stabilized) - const lastCount = clientCounts[clientCounts.length - 1] - const secondLastCount = clientCounts[clientCounts.length - 2] - - assert.ok( - Math.abs(lastCount - secondLastCount) <= 1, - `Client count should stabilize. Last counts: ${secondLastCount}, ${lastCount}` - ) - - // Additional verification: make many more requests to check for significant growth - const moreRequests = 10 - const startCount = pool[kClients].length - - for (let i = 0; i < moreRequests; i++) { - try { - await pool.request({ - path: `/more-${i}`, - method: 'GET' - }) - } catch (err) { - // Expected error - } - - // Small delay to allow for client cleanup - await new Promise(resolve => setTimeout(resolve, 10)) + // The key test: verify that client count doesn't increase monotonically, + // which would indicate the memory leak that was fixed + const maxCount = Math.max(...clientCounts) + assert.ok( + clientCounts[clientCounts.length - 1] <= maxCount, + `Client count should not increase continuously. Counts: ${clientCounts.join(', ')}` + ) + + // Ensure the last two counts are similar (stabilized) + const lastCount = clientCounts[clientCounts.length - 1] + const secondLastCount = clientCounts[clientCounts.length - 2] + + assert.ok( + Math.abs(lastCount - secondLastCount) <= 1, + `Client count should stabilize. Last counts: ${secondLastCount}, ${lastCount}` + ) + + // Additional verification: make many more requests to check for significant growth + const moreRequests = 10 + const startCount = pool[kClients].length + + for (let i = 0; i < moreRequests; i++) { + try { + await pool.request({ + path: `/more-${i}`, + method: 'GET' + }) + } catch (err) { + // Expected error } - const endCount = pool[kClients].length + // Small delay to allow for client cleanup + await new Promise(resolve => setTimeout(resolve, 10)) + } - // The maximum tolerable growth - some growth may occur due to timing issues, - // but it should be limited and not proportional to the number of requests - const maxGrowth = 3 + const endCount = pool[kClients].length - assert.ok( - endCount - startCount <= maxGrowth, - `Client count should not grow significantly after many failed requests. Start: ${startCount}, End: ${endCount}` - ) - } finally { - await pool.close() - } + // The maximum tolerable growth - some growth may occur due to timing issues, + // but it should be limited and not proportional to the number of requests + const maxGrowth = 3 + + assert.ok( + endCount - startCount <= maxGrowth, + `Client count should not grow significantly after many failed requests. Start: ${startCount}, End: ${endCount}` + ) }) // This test specifically verifies the fix in pool-base.js for connectionError event @@ -113,46 +110,43 @@ test('Pool clients are removed on connectionError event', async (t) => { const pool = new Pool(`http://localhost:${port}`, { connections: 3 // Small pool to make testing easier }) + after(() => pool.close()) + after(() => server.close()) - try { - // Make an initial successful request to create a client - await pool.request({ - path: '/', - method: 'GET' - }) - - // Save the initial number of clients - const initialCount = pool[kClients].length - assert.ok(initialCount > 0, 'Should have at least one client after a successful request') + // Make an initial successful request to create a client + await pool.request({ + path: '/', + method: 'GET' + }) - // Manually trigger a connectionError on all clients - for (const client of [...pool[kClients]]) { - client.emit('connectionError', 'origin', [client], new Error('Simulated connection error')) - } + // Save the initial number of clients + const initialCount = pool[kClients].length + assert.ok(initialCount > 0, 'Should have at least one client after a successful request') - // Allow some time for the event to be processed - await new Promise(resolve => setTimeout(resolve, 50)) - - // After the fix, all clients should be removed when they emit a connectionError - assert.strictEqual( - pool[kClients].length, - 0, - 'All clients should be removed from pool after connectionError events' - ) - - // Make another request to verify the pool can create new clients - await pool.request({ - path: '/after-error', - method: 'GET' - }) - - // Verify new clients were created - assert.ok( - pool[kClients].length > 0, - 'Pool should create new clients after previous ones were removed' - ) - } finally { - await pool.close() - await new Promise(resolve => server.close(resolve)) + // Manually trigger a connectionError on all clients + for (const client of [...pool[kClients]]) { + client.emit('connectionError', 'origin', [client], new Error('Simulated connection error')) } + + // Allow some time for the event to be processed + await new Promise(resolve => setTimeout(resolve, 50)) + + // After the fix, all clients should be removed when they emit a connectionError + assert.strictEqual( + pool[kClients].length, + 0, + 'All clients should be removed from pool after connectionError events' + ) + + // Make another request to verify the pool can create new clients + await pool.request({ + path: '/after-error', + method: 'GET' + }) + + // Verify new clients were created + assert.ok( + pool[kClients].length > 0, + 'Pool should create new clients after previous ones were removed' + ) }) diff --git a/test/proxy-agent.js b/test/proxy-agent.js index 9fe6c3b8622..fdf9024b407 100644 --- a/test/proxy-agent.js +++ b/test/proxy-agent.js @@ -144,6 +144,8 @@ test('ProxyAgent forwards connectTimeout to the proxy connector', async (t) => { } }) + after(() => proxyAgent.close()) + try { net.connect = function (options) { return new net.Socket(options) @@ -177,7 +179,6 @@ test('ProxyAgent forwards connectTimeout to the proxy connector', async (t) => { if (socket && !socket.destroyed) { socket.destroy() } - await proxyAgent.close() } }) diff --git a/test/request.js b/test/request.js index 0fa493cd7b7..c1a8428a37f 100644 --- a/test/request.js +++ b/test/request.js @@ -447,6 +447,11 @@ describe('Should include headers from iterable objects', scope => { hello: 'world' } + after(() => { + server.closeAllConnections?.() + server.close() + }) + const originalIterator = Object.prototype[Symbol.iterator] // eslint-disable-next-line no-extend-native Object.prototype[Symbol.iterator] = function * () {} @@ -472,8 +477,6 @@ describe('Should include headers from iterable objects', scope => { // eslint-disable-next-line no-extend-native Object.prototype[Symbol.iterator] = originalIterator } - server.closeAllConnections?.() - server.close() } }) diff --git a/test/socks5-proxy-agent.js b/test/socks5-proxy-agent.js index 51587f6d4cb..ce0c0dab612 100644 --- a/test/socks5-proxy-agent.js +++ b/test/socks5-proxy-agent.js @@ -1,7 +1,7 @@ 'use strict' const { tspl } = require('@matteo.collina/tspl') -const { test } = require('node:test') +const { test, after } = require('node:test') const { request } = require('..') const { InvalidArgumentError } = require('../lib/core/errors') const Socks5ProxyAgent = require('../lib/dispatcher/socks5-proxy-agent') @@ -53,26 +53,24 @@ test('Socks5ProxyAgent - basic HTTP connection', async (t) => { const socksServer = new TestSocks5Server() const socksAddress = await socksServer.listen() - try { - // Create Socks5ProxyAgent - const proxyWrapper = new Socks5ProxyAgent(`socks5://localhost:${socksAddress.port}`) + after(() => socksServer.close()) + after(() => server.close()) - // Make request through SOCKS5 proxy - const response = await request(`http://localhost:${serverPort}/test`, { - dispatcher: proxyWrapper - }) + // Create Socks5ProxyAgent + const proxyWrapper = new Socks5ProxyAgent(`socks5://localhost:${socksAddress.port}`) + + // Make request through SOCKS5 proxy + const response = await request(`http://localhost:${serverPort}/test`, { + dispatcher: proxyWrapper + }) - p.equal(response.statusCode, 200, 'should get 200 status code') + p.equal(response.statusCode, 200, 'should get 200 status code') - const body = await response.body.json() - p.deepEqual(body, { - message: 'Hello from target server', - path: '/test' - }, 'should get correct response body') - } finally { - await socksServer.close() - server.close() - } + const body = await response.body.json() + p.deepEqual(body, { + message: 'Hello from target server', + path: '/test' + }, 'should get correct response body') await p.completed }) @@ -104,25 +102,23 @@ test('Socks5ProxyAgent - with authentication', async (t) => { }) const socksAddress = await socksServer.listen() - try { - // Create Socks5ProxyAgent with auth - const proxyWrapper = new Socks5ProxyAgent(`socks5://testuser:testpass@localhost:${socksAddress.port}`) + after(() => socksServer.close()) + after(() => server.close()) - // Make request through SOCKS5 proxy - const response = await request(`http://localhost:${serverPort}/auth-test`, { - dispatcher: proxyWrapper - }) + // Create Socks5ProxyAgent with auth + const proxyWrapper = new Socks5ProxyAgent(`socks5://testuser:testpass@localhost:${socksAddress.port}`) - p.equal(response.statusCode, 200, 'should get 200 status code') + // Make request through SOCKS5 proxy + const response = await request(`http://localhost:${serverPort}/auth-test`, { + dispatcher: proxyWrapper + }) - const body = await response.body.json() - p.deepEqual(body, { - message: 'Authenticated request successful' - }, 'should get correct response body') - } finally { - await socksServer.close() - server.close() - } + p.equal(response.statusCode, 200, 'should get 200 status code') + + const body = await response.body.json() + p.deepEqual(body, { + message: 'Authenticated request successful' + }, 'should get correct response body') await p.completed }) @@ -149,28 +145,26 @@ test('Socks5ProxyAgent - authentication with options', async (t) => { }) const socksAddress = await socksServer.listen() - try { - // Create Socks5ProxyAgent with auth in options - const proxyWrapper = new Socks5ProxyAgent(`socks5://localhost:${socksAddress.port}`, { - username: 'optuser', - password: 'optpass' - }) + after(() => socksServer.close()) + after(() => server.close()) - // Make request through SOCKS5 proxy - const response = await request(`http://localhost:${serverPort}/options-auth`, { - dispatcher: proxyWrapper - }) + // Create Socks5ProxyAgent with auth in options + const proxyWrapper = new Socks5ProxyAgent(`socks5://localhost:${socksAddress.port}`, { + username: 'optuser', + password: 'optpass' + }) - p.equal(response.statusCode, 200, 'should get 200 status code') + // Make request through SOCKS5 proxy + const response = await request(`http://localhost:${serverPort}/options-auth`, { + dispatcher: proxyWrapper + }) - const body = await response.body.json() - p.deepEqual(body, { - message: 'Options auth successful' - }, 'should get correct response body') - } finally { - await socksServer.close() - server.close() - } + p.equal(response.statusCode, 200, 'should get 200 status code') + + const body = await response.body.json() + p.deepEqual(body, { + message: 'Options auth successful' + }, 'should get correct response body') await p.completed }) @@ -196,31 +190,29 @@ test('Socks5ProxyAgent - multiple requests through same proxy', async (t) => { const socksServer = new TestSocks5Server() const socksAddress = await socksServer.listen() - try { - // Create Socks5ProxyAgent - const proxyWrapper = new Socks5ProxyAgent(`socks5://localhost:${socksAddress.port}`) + after(() => socksServer.close()) + after(() => server.close()) - // Make first request - const response1 = await request(`http://localhost:${serverPort}/request1`, { - dispatcher: proxyWrapper - }) + // Create Socks5ProxyAgent + const proxyWrapper = new Socks5ProxyAgent(`socks5://localhost:${socksAddress.port}`) - p.equal(response1.statusCode, 200, 'should get 200 status code for first request') - const body1 = await response1.body.json() - p.deepEqual(body1, { message: 'Request 1', path: '/request1' }, 'should get correct response body for first request') + // Make first request + const response1 = await request(`http://localhost:${serverPort}/request1`, { + dispatcher: proxyWrapper + }) - // Make second request through same proxy - const response2 = await request(`http://localhost:${serverPort}/request2`, { - dispatcher: proxyWrapper - }) + p.equal(response1.statusCode, 200, 'should get 200 status code for first request') + const body1 = await response1.body.json() + p.deepEqual(body1, { message: 'Request 1', path: '/request1' }, 'should get correct response body for first request') - p.equal(response2.statusCode, 200, 'should get 200 status code for second request') - const body2 = await response2.body.json() - p.deepEqual(body2, { message: 'Request 2', path: '/request2' }, 'should get correct response body for second request') - } finally { - await socksServer.close() - server.close() - } + // Make second request through same proxy + const response2 = await request(`http://localhost:${serverPort}/request2`, { + dispatcher: proxyWrapper + }) + + p.equal(response2.statusCode, 200, 'should get 200 status code for second request') + const body2 = await response2.body.json() + p.deepEqual(body2, { message: 'Request 2', path: '/request2' }, 'should get correct response body for second request') await p.completed }) @@ -246,23 +238,21 @@ test('Socks5ProxyAgent - requests to different origins are routed correctly', as const socksServer = new TestSocks5Server() const socksAddress = await socksServer.listen() - try { - const proxyWrapper = new Socks5ProxyAgent(`socks5://127.0.0.1:${socksAddress.port}`) - - // First request goes to server A — establishes a pool - const respA = await request(`http://127.0.0.1:${portA}/a`, { dispatcher: proxyWrapper }) - p.equal(respA.statusCode, 200) - p.deepEqual(await respA.body.json(), { server: 'A', path: '/a' }) - - // Second request goes to server B — must NOT reuse the pool from origin A - const respB = await request(`http://127.0.0.1:${portB}/b`, { dispatcher: proxyWrapper }) - p.equal(respB.statusCode, 200) - p.deepEqual(await respB.body.json(), { server: 'B', path: '/b' }, 'request to origin B must reach server B, not server A') - } finally { - await socksServer.close() - serverA.close() - serverB.close() - } + after(() => socksServer.close()) + after(() => serverA.close()) + after(() => serverB.close()) + + const proxyWrapper = new Socks5ProxyAgent(`socks5://127.0.0.1:${socksAddress.port}`) + + // First request goes to server A — establishes a pool + const respA = await request(`http://127.0.0.1:${portA}/a`, { dispatcher: proxyWrapper }) + p.equal(respA.statusCode, 200) + p.deepEqual(await respA.body.json(), { server: 'A', path: '/a' }) + + // Second request goes to server B — must NOT reuse the pool from origin A + const respB = await request(`http://127.0.0.1:${portB}/b`, { dispatcher: proxyWrapper }) + p.equal(respB.statusCode, 200) + p.deepEqual(await respB.body.json(), { server: 'B', path: '/b' }, 'request to origin B must reach server B, not server A') await p.completed }) @@ -303,6 +293,9 @@ test('Socks5ProxyAgent - proxy connection refused', async (t) => { const socksServer = new TestSocks5Server({ simulateFailure: true }) const socksAddress = await socksServer.listen() + after(() => socksServer.close()) + after(() => server.close()) + try { const proxyWrapper = new Socks5ProxyAgent(`socks5://localhost:${socksAddress.port}`) @@ -312,9 +305,6 @@ test('Socks5ProxyAgent - proxy connection refused', async (t) => { p.fail('should have thrown an error') } catch (err) { p.ok(err, 'should throw error when SOCKS5 proxy refuses connection') - } finally { - await socksServer.close() - server.close() } await p.completed