Revert "create guardDisconnect() helper"

This reverts commit 78d583d.

Author: samayer12

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

@@ -24,31 +24,33 @@ setGlobalDispatcher(agent)
 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, use the `guardDisconnect` helper from
-`test/guard-disconnect.js` after creating a `Client` (or `Pool`):
+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')
-const { guardDisconnect } = require('./guard-disconnect')
 
 test('example with disconnect guard', async (t) => {
   t = tspl(t, { plan: 1 })
 
   const client = new Client('http://localhost:3000')
   after(() => client.close())
 
-  guardDisconnect(client, t)
+  client.on('disconnect', () => {
+    if (!client.closed && !client.destroyed) {
+      t.fail('unexpected disconnect')
+    }
+  })
 
   // ... test logic ...
 })
 ```
 
-The guard listens for `'disconnect'` events and calls `t.fail()` unless the
-client is already closing or destroyed. `client.close()` and `client.destroy()`
-both emit `'disconnect'` events, but those are expected — the guard skips them
-by checking `!client.closed && !client.destroyed`.
+`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:
 

test/client-connect.js

@@ -7,7 +7,6 @@ const { Client, errors } = require('..')
 const http = require('node:http')
 const EE = require('node:events')
 const { kBusy } = require('../lib/core/symbols')
-const { guardDisconnect } = require('./guard-disconnect')
 
 // TODO: move to test/node-test/client-connect.js
 test('connect aborted after connect', async (t) => {
@@ -30,7 +29,11 @@ test('connect aborted after connect', async (t) => {
     pipelining: 3
   })
   after(() => client.close())
-  guardDisconnect(client, t)
+  client.on('disconnect', () => {
+    if (!client.closed && !client.destroyed) {
+      t.fail('unexpected disconnect')
+    }
+  })
 
   client.connect({
     path: '/',

test/client-pipelining.js

@@ -9,7 +9,6 @@ const { kConnect } = require('../lib/core/symbols')
 const EE = require('node:events')
 const { kBusy, kRunning, kSize } = require('../lib/core/symbols')
 const { maybeWrapStream, consts } = require('./utils/async-iterators')
-const { guardDisconnect } = require('./guard-disconnect')
 
 test('20 times GET with pipelining 10', async (t) => {
   const num = 20
@@ -42,7 +41,11 @@ test('20 times GET with pipelining 10', async (t) => {
       pipelining: 10
     })
     after(() => client.close())
-    guardDisconnect(client, t)
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
 
     for (let i = 0; i < num; i++) {
       makeRequest(i)
@@ -100,7 +103,11 @@ test('A client should enqueue as much as twice its pipelining factor', async (t)
       pipelining: 2
     })
     after(() => client.close())
-    guardDisconnect(client, t)
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
 
     for (; sent < 2;) {
       t.ok(client[kSize] <= client.pipelining, 'client is not full')
@@ -151,7 +158,11 @@ test('pipeline 1 is 1 active request', async (t) => {
       pipelining: 1
     })
     after(() => client.destroy())
-    guardDisconnect(client, t)
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
     client.request({
       path: '/',
       method: 'GET'
@@ -205,7 +216,11 @@ test('pipelined chunked POST stream', async (t) => {
       pipelining: 2
     })
     after(() => client.close())
-    guardDisconnect(client, t)
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
 
     client.request({
       path: '/',
@@ -275,7 +290,11 @@ test('pipelined chunked POST iterator', async (t) => {
       pipelining: 2
     })
     after(() => client.close())
-    guardDisconnect(client, t)
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
 
     client.request({
       path: '/',
@@ -399,7 +418,11 @@ test('pipelining non-idempotent', async (t) => {
       pipelining: 2
     })
     after(() => client.close())
-    guardDisconnect(client, t)
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
 
     let ended = false
     client.request({
@@ -446,7 +469,11 @@ function pipeliningNonIdempotentWithBody (bodyType) {
         pipelining: 2
       })
       after(() => client.close())
-      guardDisconnect(client, t)
+      client.on('disconnect', () => {
+        if (!client.closed && !client.destroyed) {
+          t.fail('unexpected disconnect')
+        }
+      })
 
       let ended = false
       let reading = false
@@ -755,7 +782,11 @@ test('pipelining blocked', async (t) => {
       pipelining: 10
     })
     after(() => client.close())
-    guardDisconnect(client, t)
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
     client.request({
       path: '/',
       method: 'GET',

test/client-post.js

@@ -5,7 +5,6 @@ const { test, after } = require('node:test')
 const { once } = require('node:events')
 const { Client } = require('..')
 const { createServer } = require('node:http')
-const { guardDisconnect } = require('./guard-disconnect')
 
 test('request post blob', async (t) => {
   t = tspl(t, { plan: 3 })
@@ -27,7 +26,11 @@ test('request post blob', async (t) => {
 
   const client = new Client(`http://localhost:${server.address().port}`)
   after(client.destroy.bind(client))
-  guardDisconnect(client, t)
+  client.on('disconnect', () => {
+    if (!client.closed && !client.destroyed) {
+      t.fail('unexpected disconnect')
+    }
+  })
 
   client.request({
     path: '/',
@@ -64,7 +67,11 @@ test('request post arrayBuffer', async (t) => {
 
   const client = new Client(`http://localhost:${server.address().port}`)
   after(() => client.destroy())
-  guardDisconnect(client, t)
+  client.on('disconnect', () => {
+    if (!client.closed && !client.destroyed) {
+      t.fail('unexpected disconnect')
+    }
+  })
 
   const buf = Buffer.from('asd')
   const dst = new ArrayBuffer(buf.byteLength)

test/client-stream.js

@@ -6,7 +6,6 @@ const { Client, errors } = require('..')
 const { createServer } = require('node:http')
 const { PassThrough, Writable, Readable } = require('node:stream')
 const EE = require('node:events')
-const { guardDisconnect } = require('./guard-disconnect')
 
 test('stream get', async (t) => {
   t = tspl(t, { plan: 9 })
@@ -23,7 +22,11 @@ test('stream get', async (t) => {
   server.listen(0, () => {
     const client = new Client(`http://localhost:${server.address().port}`)
     after(() => client.close())
-    guardDisconnect(client, t)
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
 
     const signal = new EE()
     client.stream({
@@ -67,7 +70,11 @@ test('stream promise get', async (t) => {
   server.listen(0, async () => {
     const client = new Client(`http://localhost:${server.address().port}`)
     after(() => client.close())
-    guardDisconnect(client, t)
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
 
     await client.stream({
       path: '/',
@@ -257,7 +264,11 @@ test('stream waits only for writable side', async (t) => {
   server.listen(0, () => {
     const client = new Client(`http://localhost:${server.address().port}`)
     after(() => client.close())
-    guardDisconnect(client, t)
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
 
     const pt = new PassThrough({ autoDestroy: false })
     client.stream({
@@ -325,7 +336,11 @@ test('stream destroy if not readable', async (t) => {
   server.listen(0, () => {
     const client = new Client(`http://localhost:${server.address().port}`)
     after(client.destroy.bind(client))
-    guardDisconnect(client, t)
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
 
     client.stream({
       path: '/',
@@ -402,7 +417,11 @@ test('stream body without destroy', async (t) => {
   server.listen(0, () => {
     const client = new Client(`http://localhost:${server.address().port}`)
     after(client.destroy.bind(client))
-    guardDisconnect(client, t)
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
 
     client.stream({
       path: '/',
@@ -526,7 +545,11 @@ test('stream abort after complete', async (t) => {
   server.listen(0, () => {
     const client = new Client(`http://localhost:${server.address().port}`)
     after(client.destroy.bind(client))
-    guardDisconnect(client, t)
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
 
     const pt = new PassThrough()
     const signal = new EE()
@@ -587,7 +610,11 @@ test('trailers', async (t) => {
   server.listen(0, () => {
     const client = new Client(`http://localhost:${server.address().port}`)
     after(() => client.close())
-    guardDisconnect(client, t)
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
 
     client.stream({
       path: '/',
@@ -613,7 +640,11 @@ test('stream ignore 1xx', async (t) => {
   server.listen(0, () => {
     const client = new Client(`http://localhost:${server.address().port}`)
     after(() => client.close())
-    guardDisconnect(client, t)
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
 
     let buf = ''
     client.stream({
@@ -646,7 +677,11 @@ test('stream ignore 1xx and use onInfo', async (t) => {
   server.listen(0, () => {
     const client = new Client(`http://localhost:${server.address().port}`)
     after(() => client.close())
-    guardDisconnect(client, t)
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
 
     let buf = ''
     client.stream({
@@ -685,7 +720,11 @@ test('stream backpressure', async (t) => {
   server.listen(0, () => {
     const client = new Client(`http://localhost:${server.address().port}`)
     after(() => client.close())
-    guardDisconnect(client, t)
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
 
     let buf = ''
     client.stream({
@@ -747,7 +786,11 @@ test('stream needDrain', async (t) => {
     after(() => {
       client.destroy()
     })
-    guardDisconnect(client, t)
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
 
     const dst = new PassThrough()
     dst.pause()
@@ -804,7 +847,11 @@ test('stream legacy needDrain', async (t) => {
     after(() => {
       client.destroy()
     })
-    guardDisconnect(client, t)
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
 
     const dst = new PassThrough()
     dst.pause()

test/client-timeout.js

@@ -7,7 +7,6 @@ const { createServer } = require('node:http')
 const { Readable } = require('node:stream')
 const FakeTimers = require('@sinonjs/fake-timers')
 const timers = require('../lib/util/timers')
-const { guardDisconnect } = require('./guard-disconnect')
 
 test('refresh timeout on pause', async (t) => {
   t = tspl(t, { plan: 1 })
@@ -181,7 +180,11 @@ test('parser resume with no body timeout', async (t) => {
     })
     after(() => client.destroy())
 
-    guardDisconnect(client, t)
+    client.on('disconnect', () => {
+      if (!client.closed && !client.destroyed) {
+        t.fail('unexpected disconnect')
+      }
+    })
 
     client.dispatch({
       path: '/',