test: move cleanup from finally to after hooks

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

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()
   }
 })
 

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}`)
   }
 })
 

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'
+  )
 })

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()
   }
 })
 

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()
     }
   })