diff --git a/docs/docs/best-practices/writing-tests.md b/docs/docs/best-practices/writing-tests.md index 57549de6357..40939542e41 100644 --- a/docs/docs/best-practices/writing-tests.md +++ b/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`) diff --git a/test/client-timeout.js b/test/client-timeout.js index 26cf21913c2..a2c18cd8fd0 100644 --- a/test/client-timeout.js +++ b/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' diff --git a/test/client-write-max-listeners.js b/test/client-write-max-listeners.js index 09ab13cafb8..abfe204db74 100644 --- a/test/client-write-max-listeners.js +++ b/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) } diff --git a/test/connect-pre-shared-session.js b/test/connect-pre-shared-session.js index 30031048da7..5bc811ff7c5 100644 --- a/test/connect-pre-shared-session.js +++ b/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' diff --git a/test/content-length.js b/test/content-length.js index 6d8e7799c91..92386b93b67 100644 --- a/test/content-length.js +++ b/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', diff --git a/test/fetch/encoding.js b/test/fetch/encoding.js index 893a24886e4..4f6c16ed687 100644 --- a/test/fetch/encoding.js +++ b/test/fetch/encoding.js @@ -100,6 +100,7 @@ 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(', ') @@ -107,9 +108,10 @@ describe('content-encoding chain limit', () => { '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' } diff --git a/test/headers-as-array.js b/test/headers-as-array.js index 9fd4e738af8..a49ccbf3a70 100644 --- a/test/headers-as-array.js +++ b/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', diff --git a/test/headers-crlf.js b/test/headers-crlf.js index ee440e06045..2eedab56db6 100644 --- a/test/headers-crlf.js +++ b/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 { diff --git a/test/http-100.js b/test/http-100.js index 055a01faaa5..96dd48d4de9 100644 --- a/test/http-100.js +++ b/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', diff --git a/test/https.js b/test/https.js index dd5078bd2d8..04b8501be16 100644 --- a/test/https.js +++ b/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) diff --git a/test/issue-803.js b/test/issue-803.js index 12540100f8d..33f59804792 100644 --- a/test/issue-803.js +++ b/test/issue-803.js @@ -39,6 +39,12 @@ test('https://github.com/nodejs/undici/issues/803', { timeout: 60000 }, 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' diff --git a/test/max-headers.js b/test/max-headers.js index 9edd8d1efdc..60e5609ea1d 100644 --- a/test/max-headers.js +++ b/test/max-headers.js @@ -25,6 +25,12 @@ test('handle a lot of 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' diff --git a/test/max-response-size.js b/test/max-response-size.js index c1b39c98204..643a76cc031 100644 --- a/test/max-response-size.js +++ b/test/max-response-size.js @@ -23,6 +23,12 @@ describe('max response size', async (t) => { const client = new Client(`http://localhost:${server.address().port}`, { maxResponseSize: -1 }) after(() => client.close()) + client.on('disconnect', () => { + if (!client.closed && !client.destroyed) { + t.fail('unexpected disconnect') + } + }) + client.request({ path: '/', method: 'GET' }, (err, { statusCode, body }) => { t.ifError(err) t.strictEqual(statusCode, 200) @@ -56,6 +62,12 @@ describe('max response size', async (t) => { const client = new Client(`http://localhost:${server.address().port}`, { maxResponseSize: 0 }) after(() => client.close()) + client.on('disconnect', () => { + if (!client.closed && !client.destroyed) { + t.fail('unexpected disconnect') + } + }) + client.request({ path: '/', method: 'GET' }, (err, { statusCode, body }) => { t.ifError(err) t.strictEqual(statusCode, 200) diff --git a/test/no-strict-content-length.js b/test/no-strict-content-length.js index 3b16d2eeb72..ecc9acaaf0c 100644 --- a/test/no-strict-content-length.js +++ b/test/no-strict-content-length.js @@ -163,6 +163,12 @@ describe('strictContentLength: false', () => { }) after(() => client.close()) + client.on('disconnect', () => { + if (!client.closed && !client.destroyed) { + t.fail('unexpected disconnect') + } + }) + client.request({ path: '/', method: 'PUT', @@ -203,6 +209,12 @@ describe('strictContentLength: false', () => { }) after(() => client.close()) + client.on('disconnect', () => { + if (!client.closed && !client.destroyed) { + t.fail('unexpected disconnect') + } + }) + client.request({ path: '/', method: 'PUT', @@ -243,6 +255,12 @@ describe('strictContentLength: false', () => { }) after(() => client.close()) + client.on('disconnect', () => { + if (!client.closed && !client.destroyed) { + t.fail('unexpected disconnect') + } + }) + client.request({ path: '/', method: 'PUT', @@ -283,6 +301,12 @@ describe('strictContentLength: false', () => { }) after(() => client.close()) + client.on('disconnect', () => { + if (!client.closed && !client.destroyed) { + t.fail('unexpected disconnect') + } + }) + client.request({ path: '/', method: 'PUT', @@ -323,6 +347,12 @@ describe('strictContentLength: false', () => { }) after(() => client.close()) + client.on('disconnect', () => { + if (!client.closed && !client.destroyed) { + t.fail('unexpected disconnect') + } + }) + client.request({ path: '/', method: 'PUT', @@ -362,6 +392,12 @@ describe('strictContentLength: false', () => { }) after(() => client.close()) + client.on('disconnect', () => { + if (!client.closed && !client.destroyed) { + t.fail('unexpected disconnect') + } + }) + client.request({ path: '/', method: 'PUT', diff --git a/test/parser-issues.js b/test/parser-issues.js index 2d9f04628de..859a5f096cb 100644 --- a/test/parser-issues.js +++ b/test/parser-issues.js @@ -26,6 +26,12 @@ test('https://github.com/mcollina/undici/issues/268', 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') + } + }) + client.request({ method: 'GET', path: '/nxt/_changes?feed=continuous&heartbeat=5000', diff --git a/test/pipeline-pipelining.js b/test/pipeline-pipelining.js index bb4cf22e84f..96452bfaeac 100644 --- a/test/pipeline-pipelining.js +++ b/test/pipeline-pipelining.js @@ -22,6 +22,12 @@ test('pipeline pipelining', async (t) => { }) after(() => client.close()) + client.on('disconnect', () => { + if (!client.closed && !client.destroyed) { + t.fail('unexpected disconnect') + } + }) + client[kConnect](() => { t.equal(client[kRunning], 0) client.pipeline({ diff --git a/test/promises.js b/test/promises.js index 3aaf909cd71..456a3a6b254 100644 --- a/test/promises.js +++ b/test/promises.js @@ -22,6 +22,12 @@ test('basic get, async await support', 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') + } + }) + try { const { statusCode, headers, body } = await client.request({ path: '/', method: 'GET' }) t.strictEqual(statusCode, 200) @@ -70,6 +76,12 @@ test('basic POST with string, async await support', 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') + } + }) + try { const { statusCode, body } = await client.request({ path: '/', method: 'POST', body: expected }) t.strictEqual(statusCode, 200) @@ -100,6 +112,12 @@ test('basic POST with Buffer, async await support', 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') + } + }) + try { const { statusCode, body } = await client.request({ path: '/', method: 'POST', body: expected }) t.strictEqual(statusCode, 200) @@ -130,6 +148,12 @@ test('basic POST with stream, async await support', 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') + } + }) + try { const { statusCode, body } = await client.request({ path: '/', @@ -167,6 +191,12 @@ test('basic POST with async-iterator, async await support', 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') + } + }) + try { const { statusCode, body } = await client.request({ path: '/', @@ -227,6 +257,12 @@ test('20 times GET with pipelining 10, async await support', async (t) => { }) after(() => client.close()) + client.on('disconnect', () => { + if (!client.closed && !client.destroyed) { + t.fail('unexpected disconnect') + } + }) + for (let i = 0; i < num; i++) { makeRequest(i) } @@ -275,6 +311,12 @@ test('pool, async await support', async (t) => { const client = new Pool(`http://localhost:${server.address().port}`) after(() => client.close()) + client.on('disconnect', () => { + if (!client.closed && !client.destroyed) { + t.fail('unexpected disconnect') + } + }) + try { const { statusCode, headers, body } = await client.request({ path: '/', method: 'GET' }) t.strictEqual(statusCode, 200) diff --git a/test/proxy.js b/test/proxy.js index 86cae7bfc93..9d209b6be10 100644 --- a/test/proxy.js +++ b/test/proxy.js @@ -23,6 +23,12 @@ test('connect through proxy', async (t) => { const client = new Client(proxyUrl) + client.on('disconnect', () => { + if (!client.closed && !client.destroyed) { + t.fail('unexpected disconnect') + } + }) + const response = await client.request({ method: 'GET', path: serverUrl + '/hello?foo=bar' @@ -62,6 +68,12 @@ test('connect through proxy with auth', async (t) => { const client = new Client(proxyUrl) + client.on('disconnect', () => { + if (!client.closed && !client.destroyed) { + t.fail('unexpected disconnect') + } + }) + const response = await client.request({ method: 'GET', path: serverUrl + '/hello?foo=bar', @@ -102,6 +114,12 @@ test('connect through proxy with auth but invalid credentials', async (t) => { const client = new Client(proxyUrl) + client.on('disconnect', () => { + if (!client.closed && !client.destroyed) { + t.fail('unexpected disconnect') + } + }) + const response = await client.request({ method: 'GET', path: serverUrl + '/hello?foo=bar', @@ -134,6 +152,12 @@ test('connect through proxy (with pool)', async (t) => { const pool = new Pool(proxyUrl) + pool.on('disconnect', () => { + if (!pool.closed && !pool.destroyed) { + t.fail('unexpected disconnect') + } + }) + const response = await pool.request({ method: 'GET', path: serverUrl + '/hello?foo=bar' diff --git a/test/request-timeout2.js b/test/request-timeout2.js index 48293f81e95..e58f2f84264 100644 --- a/test/request-timeout2.js +++ b/test/request-timeout2.js @@ -25,6 +25,12 @@ test('request timeout with slow readable body', async (t) => { const client = new Client(`http://localhost:${server.address().port}`, { headersTimeout: 50 }) after(() => client.close()) + client.on('disconnect', () => { + if (!client.closed && !client.destroyed) { + t.fail('unexpected disconnect') + } + }) + const body = new Readable({ read () { if (this._reading) { diff --git a/test/socket-back-pressure.js b/test/socket-back-pressure.js index e7150c0a9c5..25899123ab5 100644 --- a/test/socket-back-pressure.js +++ b/test/socket-back-pressure.js @@ -37,6 +37,12 @@ test('socket back-pressure', async (t) => { }) after(() => client.close()) + client.on('disconnect', () => { + if (!client.closed && !client.destroyed) { + t.fail('unexpected disconnect') + } + }) + client.request({ path: '/', method: 'GET', opaque: 'asd' }, (err, data) => { t.ifError(err) data.body diff --git a/test/stream-compat.js b/test/stream-compat.js index 15806a2447a..9b83b70db59 100644 --- a/test/stream-compat.js +++ b/test/stream-compat.js @@ -50,6 +50,12 @@ test('IncomingMessage', async (t) => { const proxyClient = new Client(`http://localhost:${server.address().port}`) after(() => proxyClient.destroy()) + proxyClient.on('disconnect', () => { + if (!proxyClient.closed && !proxyClient.destroyed) { + t.fail('unexpected disconnect') + } + }) + const proxy = createServer({ joinDuplicateHeaders: true }, (req, res) => { proxyClient.request({ path: '/', @@ -66,6 +72,12 @@ test('IncomingMessage', async (t) => { const client = new Client(`http://localhost:${proxy.address().port}`) after(() => client.destroy()) + client.on('disconnect', () => { + if (!client.closed && !client.destroyed) { + t.fail('unexpected disconnect') + } + }) + client.request({ path: '/', method: 'PUT', diff --git a/test/trailers.js b/test/trailers.js index 64a29da7b22..6815632271d 100644 --- a/test/trailers.js +++ b/test/trailers.js @@ -18,6 +18,13 @@ test('response trailers missing is OK', async (t) => { server.listen(0, async () => { const client = new Client(`http://localhost:${server.address().port}`) after(() => client.destroy()) + + client.on('disconnect', () => { + if (!client.closed && !client.destroyed) { + t.fail('unexpected disconnect') + } + }) + const { body } = await client.request({ path: '/', method: 'GET', @@ -46,6 +53,13 @@ test('response trailers missing w trailers is OK', async (t) => { server.listen(0, async () => { const client = new Client(`http://localhost:${server.address().port}`) after(() => client.destroy()) + + client.on('disconnect', () => { + if (!client.closed && !client.destroyed) { + t.fail('unexpected disconnect') + } + }) + const { body, trailers } = await client.request({ path: '/', method: 'GET',