test: add unexpected disconnect guards to more client test files (#4844)

Author: samayer12

docs/docs/best-practices/writing-tests.md

@@ -18,3 +18,46 @@ const agent = new Agent({
 
 setGlobalDispatcher(agent)
 ```
+
+## Guarding against unexpected disconnects
+
+Undici's `Client` automatically reconnects after a socket error. This means
+a test can silently disconnect, reconnect, and still pass. Unfortunately, 
+this could mask bugs like unexpected parser errors or protocol violations.
+To catch these silent reconnections, add a disconnect guard after creating 
+a `Client`:
+
+```js
+const { Client } = require('undici')
+const { test, after } = require('node:test')
+const { tspl } = require('@matteo.collina/tspl')
+
+test('example with disconnect guard', async (t) => {
+  t = tspl(t, { plan: 1 })
+
+  const client = new Client('http://localhost:3000')
+  after(() => client.close())
+
+  client.on('disconnect', () => {
+    if (!client.closed && !client.destroyed) {
+      t.fail('unexpected disconnect')
+    }
+  })
+
+  // ... test logic ...
+})
+```
+
+`client.close()` and `client.destroy()` both emit `'disconnect'` events, but
+those are expected. The guard only fails when a disconnect happens during the
+active test (i.e., `!client.closed && !client.destroyed` is true).
+
+Skip the guard for tests where a disconnect is expected behavior, such as:
+
+- Signal aborts (`signal.emit('abort')`, `ac.abort()`)
+- Server-side destruction (`res.destroy()`, `req.socket.destroy()`)
+- Client-side body destruction mid-stream (`data.body.destroy()`)
+- Timeout errors (`HeadersTimeoutError`, `BodyTimeoutError`)
+- Successful upgrades (the socket is detached from the `Client`)
+- Retry/reconnect tests where the disconnect triggers the retry
+- HTTP parser errors from malformed responses (`HTTPParserError`)

test/client-timeout.js

@@ -180,6 +180,12 @@ test('parser resume with no body timeout', async (t) => {
     })
     after(() => client.destroy())
 
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
+
     client.dispatch({
       path: '/',
       method: 'GET'

test/client-write-max-listeners.js

@@ -48,6 +48,12 @@ test('socket close listener does not leak', async (t) => {
   const client = new Client(`http://localhost:${server.address().port}`)
   after(() => client.destroy())
 
+  client.on('disconnect', () => {
+    if (!client.closed && !client.destroyed) {
+      t.fail('unexpected disconnect')
+    }
+  })
+
   for (let n = 0; n < 16; ++n) {
     client.request({ path: '/', method: 'GET', body: makeBody() }, onRequest)
   }

test/connect-pre-shared-session.js

@@ -31,6 +31,12 @@ test('custom session passed to client will be used in tls connect call', async (
     })
     after(() => client.close())
 
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
+
     const { statusCode, headers, body } = await client.request({
       path: '/',
       method: 'GET'

test/content-length.js

@@ -209,6 +209,12 @@ test('request streaming no body data when content-length=0', async (t) => {
     const client = new Client(`http://localhost:${server.address().port}`)
     after(() => client.close())
 
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
+
     client.request({
       path: '/',
       method: 'PUT',
@@ -278,6 +284,12 @@ test('request streaming with Readable.from(buf)', async (t) => {
     const client = new Client(`http://localhost:${server.address().port}`)
     after(() => client.close())
 
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
+
     client.request({
       path: '/',
       method: 'PUT',

test/fetch/encoding.js

@@ -100,16 +100,18 @@ describe('content-encoding chain limit', () => {
     server = createServer({
       noDelay: true
     }, (req, res) => {
+      res.socket.setNoDelay(true)
       const encodingCount = parseInt(req.headers['x-encoding-count'] || '1', 10)
       const encodings = Array(encodingCount).fill('identity').join(', ')
 
       res.writeHead(200, {
         'Content-Encoding': encodings,
         'Content-Type': 'text/plain'
       })
+      res.flushHeaders()
       res.end('test')
     })
-    await once(server.listen(0), 'listening')
+    await once(server.listen(0, '127.0.0.1'), 'listening')
   })
 
   after(() => {
@@ -118,10 +120,10 @@ describe('content-encoding chain limit', () => {
   })
 
   test(`should allow exactly ${MAX_CONTENT_ENCODINGS} content-encodings`, async (t) => {
-    const client = new Client(`http://localhost:${server.address().port}`)
+    const client = new Client(`http://127.0.0.1:${server.address().port}`)
     t.after(() => client.close())
 
-    const response = await fetch(`http://localhost:${server.address().port}`, {
+    const response = await fetch(`http://127.0.0.1:${server.address().port}`, {
       dispatcher: client,
       keepalive: false,
       headers: { 'x-encoding-count': String(MAX_CONTENT_ENCODINGS) }
@@ -133,11 +135,11 @@ describe('content-encoding chain limit', () => {
   })
 
   test(`should reject more than ${MAX_CONTENT_ENCODINGS} content-encodings`, async (t) => {
-    const client = new Client(`http://localhost:${server.address().port}`)
+    const client = new Client(`http://127.0.0.1:${server.address().port}`)
     t.after(() => client.close())
 
     await t.assert.rejects(
-      fetch(`http://localhost:${server.address().port}`, {
+      fetch(`http://127.0.0.1:${server.address().port}`, {
         dispatcher: client,
         keepalive: false,
         headers: { 'x-encoding-count': String(MAX_CONTENT_ENCODINGS + 1) }
@@ -150,11 +152,11 @@ describe('content-encoding chain limit', () => {
   })
 
   test('should reject excessive content-encoding chains', async (t) => {
-    const client = new Client(`http://localhost:${server.address().port}`)
+    const client = new Client(`http://127.0.0.1:${server.address().port}`)
     t.after(() => client.close())
 
     await t.assert.rejects(
-      fetch(`http://localhost:${server.address().port}`, {
+      fetch(`http://127.0.0.1:${server.address().port}`, {
         dispatcher: client,
         keepalive: false,
         headers: { 'x-encoding-count': '100' }

test/headers-as-array.js

@@ -20,6 +20,12 @@ test('handle headers as array', async (t) => {
     const client = new Client(`http://localhost:${server.address().port}`)
     after(() => client.close())
 
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
+
     client.request({
       path: '/',
       method: 'GET',
@@ -46,6 +52,12 @@ test('handle multi-valued headers as array', async (t) => {
     const client = new Client(`http://localhost:${server.address().port}`)
     after(() => client.close())
 
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
+
     client.request({
       path: '/',
       method: 'GET',
@@ -72,6 +84,12 @@ test('handle headers with array', async (t) => {
     const client = new Client(`http://localhost:${server.address().port}`)
     after(() => client.close())
 
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
+
     client.request({
       path: '/',
       method: 'GET',
@@ -98,6 +116,12 @@ test('handle multi-valued headers', async (t) => {
     const client = new Client(`http://localhost:${server.address().port}`)
     after(() => client.close())
 
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
+
     client.request({
       path: '/',
       method: 'GET',
@@ -118,6 +142,12 @@ test('fail if headers array is odd', async (t) => {
     const client = new Client(`http://localhost:${server.address().port}`)
     after(() => client.close())
 
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
+
     client.request({
       path: '/',
       method: 'GET',
@@ -141,6 +171,12 @@ test('fail if headers is not an object or an array', async (t) => {
     const client = new Client(`http://localhost:${server.address().port}`)
     after(() => client.close())
 
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
+
     client.request({
       path: '/',
       method: 'GET',

test/headers-crlf.js

@@ -21,6 +21,12 @@ test('CRLF Injection in Nodejs ‘undici’ via host', async (t) => {
   const client = new Client(`http://localhost:${server.address().port}`)
   after(() => client.close())
 
+  client.on('disconnect', () => {
+    if (!client.closed && !client.destroyed) {
+      t.fail('unexpected disconnect')
+    }
+  })
+
   const unsanitizedContentTypeInput = '12 \r\n\r\naaa:aaa'
 
   try {

test/http-100.js

@@ -21,6 +21,12 @@ test('ignore informational response', async (t) => {
   const client = new Client(`http://localhost:${server.address().port}`)
   after(() => client.close())
 
+  client.on('disconnect', () => {
+    if (!client.closed && !client.destroyed) {
+      t.fail('unexpected disconnect')
+    }
+  })
+
   client.request({
     path: '/',
     method: 'POST',
@@ -137,6 +143,12 @@ test('1xx response without timeouts', async t => {
   })
   after(() => client.close())
 
+  client.on('disconnect', () => {
+    if (!client.closed && !client.destroyed) {
+      t.fail('unexpected disconnect')
+    }
+  })
+
   client.request({
     path: '/',
     method: 'POST',

test/https.js

@@ -25,6 +25,12 @@ test('https get with tls opts', async (t) => {
     })
     after(() => client.close())
 
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
+
     client.request({ path: '/', method: 'GET' }, (err, { statusCode, headers, body }) => {
       t.ifError(err)
       t.strictEqual(statusCode, 200)
@@ -61,6 +67,12 @@ test('https get with tls opts ip', async (t) => {
     })
     after(() => client.close())
 
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
+
     client.request({ path: '/', method: 'GET' }, (err, { statusCode, headers, body }) => {
       t.ifError(err)
       t.strictEqual(statusCode, 200)