From 7393ea3014ed56f6ba580d32245b2a65af9697fa Mon Sep 17 00:00:00 2001 From: uzlopak Date: Thu, 5 Sep 2024 08:47:08 +0200 Subject: [PATCH 01/14] fix: use fasttimers for all connection timeouts --- lib/dispatcher/client-h1.js | 8 +- lib/util/timers.js | 34 +- test/issue-3356.js | 6 +- test/request-timeout.js | 1385 ++++++++++++++++++----------------- 4 files changed, 736 insertions(+), 697 deletions(-) diff --git a/lib/dispatcher/client-h1.js b/lib/dispatcher/client-h1.js index ca8ce650af2..3895fc5874f 100644 --- a/lib/dispatcher/client-h1.js +++ b/lib/dispatcher/client-h1.js @@ -165,13 +165,9 @@ class Parser { setTimeout (delay, type) { this.timeoutType = type if (delay !== this.timeoutValue) { - this.timeout && timers.clearTimeout(this.timeout) + this.timeout && timers.clearFastTimeout(this.timeout) if (delay) { - this.timeout = timers.setTimeout(onParserTimeout, delay, new WeakRef(this)) - // istanbul ignore else: only for jest - if (this.timeout.unref) { - this.timeout.unref() - } + this.timeout = timers.setFastTimeout(onParserTimeout, delay, new WeakRef(this)) } else { this.timeout = null } diff --git a/lib/util/timers.js b/lib/util/timers.js index 8fa3ac56b7b..871ddd2d4a3 100644 --- a/lib/util/timers.js +++ b/lib/util/timers.js @@ -347,7 +347,7 @@ module.exports = { * The clearTimeout method cancels an instantiated Timer previously created * by calling setTimeout. * - * @param {FastTimer} timeout + * @param {NodeJS.Timeout|FastTimer} timeout */ clearTimeout (timeout) { // If the timeout is a FastTimer, call its own clear method. @@ -362,6 +362,31 @@ module.exports = { nativeClearTimeout(timeout) } }, + /** + * The setFastTimeout() method sets a fastTimer which executes a function once + * the timer expires. + * @param {Function} callback A function to be executed after the timer + * expires. + * @param {number} delay The time, in milliseconds that the timer should + * wait before the specified function or code is executed. + * @param {*} [arg] An optional argument to be passed to the callback function + * when the timer expires. + * @returns {FastTimer} + */ + setFastTimeout (callback, delay, arg) { + // If the delay is less than or equal to the RESOLUTION_MS value return a + // native Node.js Timer instance. + return new FastTimer(callback, delay, arg) + }, + /** + * The clearTimeout method cancels an instantiated FastTimer previously + * created by calling setFastTimeout. + * + * @param {FastTimer} timeout + */ + clearFastTimeout (timeout) { + timeout.clear() + }, /** * The now method returns the value of the internal fast timer clock. * @@ -370,6 +395,13 @@ module.exports = { now () { return fastNow }, + /** + * Trigger the onTick function to process the fastTimers array. + * Exported for testing purposes only. + */ + tick () { + onTick() + }, /** * Exporting for testing purposes only. * Marking as deprecated to discourage any use outside of testing. diff --git a/test/issue-3356.js b/test/issue-3356.js index 927208583a9..b7820204dd9 100644 --- a/test/issue-3356.js +++ b/test/issue-3356.js @@ -4,7 +4,7 @@ const { tspl } = require('@matteo.collina/tspl') const { test, after } = require('node:test') const { createServer } = require('node:http') const { once } = require('node:events') - +const { tick: fastTimerTick } = require('../lib/util/timers') const { fetch, Agent, RetryAgent } = require('..') test('https://github.com/nodejs/undici/issues/3356', async (t) => { @@ -42,6 +42,10 @@ test('https://github.com/nodejs/undici/issues/3356', async (t) => { const response = await fetch(`http://localhost:${server.address().port}`, { dispatcher: agent }) + + fastTimerTick() + fastTimerTick() + setTimeout(async () => { try { t.equal(response.status, 200) diff --git a/test/request-timeout.js b/test/request-timeout.js index 03d34c9bef5..f9f9d1bb5ce 100644 --- a/test/request-timeout.js +++ b/test/request-timeout.js @@ -1,7 +1,7 @@ 'use strict' const { tspl } = require('@matteo.collina/tspl') -const { test, after } = require('node:test') +const { describe, test, after } = require('node:test') const { createReadStream, writeFileSync, unlinkSync } = require('node:fs') const { Client, errors } = require('..') const { kConnect } = require('../lib/core/symbols') @@ -16,902 +16,909 @@ const { Writable, PassThrough } = require('node:stream') +const { + tick: fastTimerTick +} = require('../lib/util/timers') -test('request timeout', async (t) => { - t = tspl(t, { plan: 1 }) - - const server = createServer((req, res) => { - setTimeout(() => { - res.end('hello') - }, 1000) - }) - after(() => server.close()) - - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { headersTimeout: 500 }) - after(() => client.destroy()) +describe('request timeout', { skip: true }, () => { + test('request timeout', async (t) => { + t = tspl(t, { plan: 1 }) - client.request({ path: '/', method: 'GET' }, (err, response) => { - t.ok(err instanceof errors.HeadersTimeoutError) + const server = createServer((req, res) => { + setTimeout(() => { + res.end('hello') + }, 1000) }) - }) + after(() => server.close()) - await t.completed -}) + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { headersTimeout: 500 }) + after(() => client.destroy()) -test('request timeout with readable body', async (t) => { - t = tspl(t, { plan: 1 }) + client.request({ path: '/', method: 'GET' }, (err, response) => { + t.ok(err instanceof errors.HeadersTimeoutError) + }) + }) - const server = createServer((req, res) => { + await t.completed }) - after(() => server.close()) - - const tempfile = `${__filename}.10mb.txt` - writeFileSync(tempfile, Buffer.alloc(10 * 1024 * 1024)) - after(() => unlinkSync(tempfile)) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { headersTimeout: 1e3 }) - after(() => client.destroy()) + test('request timeout with readable body', async (t) => { + t = tspl(t, { plan: 1 }) - const body = createReadStream(tempfile) - client.request({ path: '/', method: 'POST', body }, (err, response) => { - t.ok(err instanceof errors.HeadersTimeoutError) + const server = createServer((req, res) => { }) - }) - - await t.completed -}) - -test('body timeout', async (t) => { - t = tspl(t, { plan: 2 }) + after(() => server.close()) - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] - }) - after(() => clock.uninstall()) + const tempfile = `${__filename}.10mb.txt` + writeFileSync(tempfile, Buffer.alloc(10 * 1024 * 1024)) + after(() => unlinkSync(tempfile)) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { headersTimeout: 1e3 }) + after(() => client.destroy()) - const server = createServer((req, res) => { - res.write('hello') - }) - after(() => server.close()) - - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { bodyTimeout: 50 }) - after(() => client.destroy()) - - client.request({ path: '/', method: 'GET' }, (err, { body }) => { - t.ifError(err) - body.on('data', () => { - clock.tick(100) - }).on('error', (err) => { - t.ok(err instanceof errors.BodyTimeoutError) + const body = createReadStream(tempfile) + client.request({ path: '/', method: 'POST', body }, (err, response) => { + t.ok(err instanceof errors.HeadersTimeoutError) }) }) - clock.tick(50) - }) - - await t.completed -}) - -test('overridden request timeout', async (t) => { - t = tspl(t, { plan: 1 }) - - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] + await t.completed }) - after(() => clock.uninstall()) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) + test('body timeout', async (t) => { + t = tspl(t, { plan: 2 }) - const server = createServer((req, res) => { - setTimeout(() => { - res.end('hello') - }, 100) - clock.tick(100) - }) - after(() => server.close()) + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] + }) + after(() => clock.uninstall()) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { headersTimeout: 500 }) - after(() => client.destroy()) + const orgTimers = { ...timers } + Object.assign(timers, { setTimeout, clearTimeout }) + after(() => { + Object.assign(timers, orgTimers) + }) - client.request({ path: '/', method: 'GET', headersTimeout: 50 }, (err, response) => { - t.ok(err instanceof errors.HeadersTimeoutError) + const server = createServer((req, res) => { + res.write('hello') }) + after(() => server.close()) - clock.tick(50) - }) + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { bodyTimeout: 50 }) + after(() => client.destroy()) - await t.completed -}) + client.request({ path: '/', method: 'GET' }, (err, { body }) => { + t.ifError(err) + body.on('data', () => { + clock.tick(100) + }).on('error', (err) => { + t.ok(err instanceof errors.BodyTimeoutError) + }) + }) -test('overridden body timeout', async (t) => { - t = tspl(t, { plan: 2 }) + clock.tick(50) + }) - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] + await t.completed }) - after(() => clock.uninstall()) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) + test('overridden request timeout', async (t) => { + t = tspl(t, { plan: 1 }) - const server = createServer((req, res) => { - res.write('hello') - }) - after(() => server.close()) + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] + }) + after(() => clock.uninstall()) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { bodyTimeout: 500 }) - after(() => client.destroy()) + const orgTimers = { ...timers } + Object.assign(timers, { setTimeout, clearTimeout }) + after(() => { + Object.assign(timers, orgTimers) + }) - client.request({ path: '/', method: 'GET', bodyTimeout: 50 }, (err, { body }) => { - t.ifError(err) - body.on('data', () => { - clock.tick(100) - }).on('error', (err) => { - t.ok(err instanceof errors.BodyTimeoutError) - }) + const server = createServer((req, res) => { + setTimeout(() => { + res.end('hello') + }, 100) + clock.tick(100) }) + after(() => server.close()) - clock.tick(50) - }) + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { headersTimeout: 500 }) + after(() => client.destroy()) - await t.completed -}) + client.request({ path: '/', method: 'GET', headersTimeout: 50 }, (err, response) => { + t.ok(err instanceof errors.HeadersTimeoutError) + }) -test('With EE signal', async (t) => { - t = tspl(t, { plan: 1 }) + clock.tick(50) + }) - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] + await t.completed }) - after(() => clock.uninstall()) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) + test('overridden body timeout', async (t) => { + t = tspl(t, { plan: 2 }) - const server = createServer((req, res) => { - setTimeout(() => { - res.end('hello') - }, 100) - clock.tick(100) - }) - after(() => server.close()) + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] + }) + after(() => clock.uninstall()) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { - headersTimeout: 50 + const orgTimers = { ...timers } + Object.assign(timers, { setTimeout, clearTimeout }) + after(() => { + Object.assign(timers, orgTimers) }) - const ee = new EventEmitter() - after(() => client.destroy()) - client.request({ path: '/', method: 'GET', signal: ee }, (err, response) => { - t.ok(err instanceof errors.HeadersTimeoutError) + const server = createServer((req, res) => { + res.write('hello') }) + after(() => server.close()) - clock.tick(50) - }) + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { bodyTimeout: 500 }) + after(() => client.destroy()) - await t.completed -}) + client.request({ path: '/', method: 'GET', bodyTimeout: 50 }, (err, { body }) => { + t.ifError(err) + body.on('data', () => { + fastTimerTick() + fastTimerTick() + }).on('error', (err) => { + t.ok(err instanceof errors.BodyTimeoutError) + }) + }) -test('With abort-controller signal', async (t) => { - t = tspl(t, { plan: 1 }) + fastTimerTick() + fastTimerTick() + }) - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] + await t.completed }) - after(() => clock.uninstall()) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) + test('With EE signal', async (t) => { + t = tspl(t, { plan: 1 }) - const server = createServer((req, res) => { - setTimeout(() => { - res.end('hello') - }, 100) - clock.tick(100) - }) - after(() => server.close()) + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] + }) + after(() => clock.uninstall()) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { - headersTimeout: 50 + const orgTimers = { ...timers } + Object.assign(timers, { setTimeout, clearTimeout }) + after(() => { + Object.assign(timers, orgTimers) }) - const abortController = new AbortController() - after(() => client.destroy()) - client.request({ path: '/', method: 'GET', signal: abortController.signal }, (err, response) => { - t.ok(err instanceof errors.HeadersTimeoutError) + const server = createServer((req, res) => { + setTimeout(() => { + res.end('hello') + }, 100) + clock.tick(100) }) + after(() => server.close()) - clock.tick(50) - }) + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + headersTimeout: 50 + }) + const ee = new EventEmitter() + after(() => client.destroy()) - await t.completed -}) + client.request({ path: '/', method: 'GET', signal: ee }, (err, response) => { + t.ok(err instanceof errors.HeadersTimeoutError) + }) -test('Abort before timeout (EE)', async (t) => { - t = tspl(t, { plan: 1 }) + clock.tick(50) + }) - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] + await t.completed }) - after(() => clock.uninstall()) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) + test('With abort-controller signal', async (t) => { + t = tspl(t, { plan: 1 }) - const ee = new EventEmitter() - const server = createServer((req, res) => { - setTimeout(() => { - res.end('hello') - }, 100) - ee.emit('abort') - clock.tick(50) - }) - after(() => server.close()) + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] + }) + after(() => clock.uninstall()) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { - headersTimeout: 50 + const orgTimers = { ...timers } + Object.assign(timers, { setTimeout, clearTimeout }) + after(() => { + Object.assign(timers, orgTimers) }) - after(() => client.destroy()) - client.request({ path: '/', method: 'GET', signal: ee }, (err, response) => { - t.ok(err instanceof errors.RequestAbortedError) + const server = createServer((req, res) => { + setTimeout(() => { + res.end('hello') + }, 100) clock.tick(100) }) - }) + after(() => server.close()) - await t.completed -}) + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + headersTimeout: 50 + }) + const abortController = new AbortController() + after(() => client.destroy()) -test('Abort before timeout (abort-controller)', async (t) => { - t = tspl(t, { plan: 1 }) + client.request({ path: '/', method: 'GET', signal: abortController.signal }, (err, response) => { + t.ok(err instanceof errors.HeadersTimeoutError) + }) - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] - }) - after(() => clock.uninstall()) + clock.tick(50) + }) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) + await t.completed }) - const abortController = new AbortController() - const server = createServer((req, res) => { - setTimeout(() => { - res.end('hello') - }, 100) - abortController.abort() - clock.tick(50) - }) - after(() => server.close()) + test('Abort before timeout (EE)', async (t) => { + t = tspl(t, { plan: 1 }) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { - headersTimeout: 50 + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] }) - after(() => client.destroy()) + after(() => clock.uninstall()) - client.request({ path: '/', method: 'GET', signal: abortController.signal }, (err, response) => { - t.ok(err instanceof errors.RequestAbortedError) - clock.tick(100) + const orgTimers = { ...timers } + Object.assign(timers, { setTimeout, clearTimeout }) + after(() => { + Object.assign(timers, orgTimers) }) - }) - - await t.completed -}) -test('Timeout with pipelining', async (t) => { - t = tspl(t, { plan: 3 }) + const ee = new EventEmitter() + const server = createServer((req, res) => { + setTimeout(() => { + res.end('hello') + }, 100) + ee.emit('abort') + clock.tick(50) + }) + after(() => server.close()) + + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + headersTimeout: 50 + }) + after(() => client.destroy()) - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] - }) - after(() => clock.uninstall()) + client.request({ path: '/', method: 'GET', signal: ee }, (err, response) => { + t.ok(err instanceof errors.RequestAbortedError) + clock.tick(100) + }) + }) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) + await t.completed }) - const server = createServer((req, res) => { - setTimeout(() => { - res.end('hello') - }, 100) - clock.tick(50) - }) - after(() => server.close()) + test('Abort before timeout (abort-controller)', async (t) => { + t = tspl(t, { plan: 1 }) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { - pipelining: 10, - headersTimeout: 50 + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] }) - after(() => client.destroy()) + after(() => clock.uninstall()) - client.request({ path: '/', method: 'GET' }, (err, response) => { - t.ok(err instanceof errors.HeadersTimeoutError) + const orgTimers = { ...timers } + Object.assign(timers, { setTimeout, clearTimeout }) + after(() => { + Object.assign(timers, orgTimers) }) - client.request({ path: '/', method: 'GET' }, (err, response) => { - t.ok(err instanceof errors.HeadersTimeoutError) - }) + const abortController = new AbortController() + const server = createServer((req, res) => { + setTimeout(() => { + res.end('hello') + }, 100) + abortController.abort() + clock.tick(50) + }) + after(() => server.close()) + + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + headersTimeout: 50 + }) + after(() => client.destroy()) - client.request({ path: '/', method: 'GET' }, (err, response) => { - t.ok(err instanceof errors.HeadersTimeoutError) + client.request({ path: '/', method: 'GET', signal: abortController.signal }, (err, response) => { + t.ok(err instanceof errors.RequestAbortedError) + clock.tick(100) + }) }) - }) - - await t.completed -}) - -test('Global option', async (t) => { - t = tspl(t, { plan: 1 }) - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] + await t.completed }) - after(() => clock.uninstall()) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) + test('Timeout with pipelining', async (t) => { + t = tspl(t, { plan: 3 }) - const server = createServer((req, res) => { - setTimeout(() => { - res.end('hello') - }, 100) - clock.tick(100) - }) - after(() => server.close()) + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] + }) + after(() => clock.uninstall()) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { - headersTimeout: 50 + const orgTimers = { ...timers } + Object.assign(timers, { setTimeout, clearTimeout }) + after(() => { + Object.assign(timers, orgTimers) }) - after(() => client.destroy()) - client.request({ path: '/', method: 'GET' }, (err, response) => { - t.ok(err instanceof errors.HeadersTimeoutError) + const server = createServer((req, res) => { + setTimeout(() => { + res.end('hello') + }, 100) + clock.tick(50) }) + after(() => server.close()) - clock.tick(50) - }) + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + pipelining: 10, + headersTimeout: 50 + }) + after(() => client.destroy()) - await t.completed -}) + client.request({ path: '/', method: 'GET' }, (err, response) => { + t.ok(err instanceof errors.HeadersTimeoutError) + }) -test('Request options overrides global option', async (t) => { - t = tspl(t, { plan: 1 }) + client.request({ path: '/', method: 'GET' }, (err, response) => { + t.ok(err instanceof errors.HeadersTimeoutError) + }) - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] - }) - after(() => clock.uninstall()) + client.request({ path: '/', method: 'GET' }, (err, response) => { + t.ok(err instanceof errors.HeadersTimeoutError) + }) + }) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) + await t.completed }) - const server = createServer((req, res) => { - setTimeout(() => { - res.end('hello') - }, 100) - clock.tick(100) - }) - after(() => server.close()) + test('Global option', async (t) => { + t = tspl(t, { plan: 1 }) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { - headersTimeout: 50 + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] }) - after(() => client.destroy()) + after(() => clock.uninstall()) - client.request({ path: '/', method: 'GET' }, (err, response) => { - t.ok(err instanceof errors.HeadersTimeoutError) + const orgTimers = { ...timers } + Object.assign(timers, { setTimeout, clearTimeout }) + after(() => { + Object.assign(timers, orgTimers) }) - clock.tick(50) - }) + const server = createServer((req, res) => { + setTimeout(() => { + res.end('hello') + }, 100) + clock.tick(100) + }) + after(() => server.close()) - await t.completed -}) + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + headersTimeout: 50 + }) + after(() => client.destroy()) -test('client.destroy should cancel the timeout', async (t) => { - t = tspl(t, { plan: 2 }) + client.request({ path: '/', method: 'GET' }, (err, response) => { + t.ok(err instanceof errors.HeadersTimeoutError) + }) - const server = createServer((req, res) => { - res.end('hello') + clock.tick(50) + }) + + await t.completed }) - after(() => server.close()) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { - headersTimeout: 100 + test('Request options overrides global option', async (t) => { + t = tspl(t, { plan: 1 }) + + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] }) + after(() => clock.uninstall()) - client.request({ path: '/', method: 'GET' }, (err, response) => { - t.ok(err instanceof errors.ClientDestroyedError) + const orgTimers = { ...timers } + Object.assign(timers, { setTimeout, clearTimeout }) + after(() => { + Object.assign(timers, orgTimers) }) - client.destroy(err => { - t.ifError(err) + const server = createServer((req, res) => { + setTimeout(() => { + res.end('hello') + }, 100) + clock.tick(100) }) - }) + after(() => server.close()) - await t.completed -}) + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + headersTimeout: 50 + }) + after(() => client.destroy()) -test('client.close should wait for the timeout', async (t) => { - t = tspl(t, { plan: 2 }) + client.request({ path: '/', method: 'GET' }, (err, response) => { + t.ok(err instanceof errors.HeadersTimeoutError) + }) - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] - }) - after(() => clock.uninstall()) + clock.tick(50) + }) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) + await t.completed }) - const server = createServer((req, res) => { - }) - after(() => server.close()) + test('client.destroy should cancel the timeout', async (t) => { + t = tspl(t, { plan: 2 }) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { - headersTimeout: 100 + const server = createServer((req, res) => { + res.end('hello') }) - after(() => client.destroy()) + after(() => server.close()) - client.request({ path: '/', method: 'GET' }, (err, response) => { - t.ok(err instanceof errors.HeadersTimeoutError) - }) + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + headersTimeout: 100 + }) - client.close((err) => { - t.ifError(err) - }) + client.request({ path: '/', method: 'GET' }, (err, response) => { + t.ok(err instanceof errors.ClientDestroyedError) + }) - client.on('connect', () => { - process.nextTick(() => { - clock.tick(100) + client.destroy(err => { + t.ifError(err) }) }) - }) - await t.completed -}) - -test('Validation', async (t) => { - t = tspl(t, { plan: 4 }) + await t.completed + }) - try { - const client = new Client('http://localhost:3000', { - headersTimeout: 'foobar' - }) - after(() => client.destroy()) - } catch (err) { - t.ok(err instanceof errors.InvalidArgumentError) - } + test('client.close should wait for the timeout', async (t) => { + t = tspl(t, { plan: 2 }) - try { - const client = new Client('http://localhost:3000', { - headersTimeout: -1 + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] }) - after(() => client.destroy()) - } catch (err) { - t.ok(err instanceof errors.InvalidArgumentError) - } + after(() => clock.uninstall()) - try { - const client = new Client('http://localhost:3000', { - bodyTimeout: 'foobar' + const orgTimers = { ...timers } + Object.assign(timers, { setTimeout, clearTimeout }) + after(() => { + Object.assign(timers, orgTimers) }) - after(() => client.destroy()) - } catch (err) { - t.ok(err instanceof errors.InvalidArgumentError) - } - try { - const client = new Client('http://localhost:3000', { - bodyTimeout: -1 + const server = createServer((req, res) => { }) - after(() => client.destroy()) - } catch (err) { - t.ok(err instanceof errors.InvalidArgumentError) - } + after(() => server.close()) - await t.completed -}) - -test('Disable request timeout', async (t) => { - t = tspl(t, { plan: 2 }) - - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] - }) - after(() => clock.uninstall()) - - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) - - const server = createServer((req, res) => { - setTimeout(() => { - res.end('hello') - }, 32e3) - clock.tick(33e3) - }) - after(() => server.close()) + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + headersTimeout: 100 + }) + after(() => client.destroy()) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { - headersTimeout: 0, - connectTimeout: 0 - }) - after(() => client.destroy()) + client.request({ path: '/', method: 'GET' }, (err, response) => { + t.ok(err instanceof errors.HeadersTimeoutError) + }) - client.request({ path: '/', method: 'GET' }, (err, response) => { - t.ifError(err) - const bufs = [] - response.body.on('data', (buf) => { - bufs.push(buf) + client.close((err) => { + t.ifError(err) }) - response.body.on('end', () => { - t.strictEqual('hello', Buffer.concat(bufs).toString('utf8')) + + client.on('connect', () => { + process.nextTick(() => { + clock.tick(100) + }) }) }) - clock.tick(31e3) + await t.completed }) - await t.completed -}) + test('Validation', async (t) => { + t = tspl(t, { plan: 4 }) -test('Disable request timeout for a single request', async (t) => { - t = tspl(t, { plan: 2 }) + try { + const client = new Client('http://localhost:3000', { + headersTimeout: 'foobar' + }) + after(() => client.destroy()) + } catch (err) { + t.ok(err instanceof errors.InvalidArgumentError) + } + + try { + const client = new Client('http://localhost:3000', { + headersTimeout: -1 + }) + after(() => client.destroy()) + } catch (err) { + t.ok(err instanceof errors.InvalidArgumentError) + } + + try { + const client = new Client('http://localhost:3000', { + bodyTimeout: 'foobar' + }) + after(() => client.destroy()) + } catch (err) { + t.ok(err instanceof errors.InvalidArgumentError) + } + + try { + const client = new Client('http://localhost:3000', { + bodyTimeout: -1 + }) + after(() => client.destroy()) + } catch (err) { + t.ok(err instanceof errors.InvalidArgumentError) + } - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] + await t.completed }) - after(() => clock.uninstall()) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) + test('Disable request timeout', async (t) => { + t = tspl(t, { plan: 2 }) - const server = createServer((req, res) => { - setTimeout(() => { - res.end('hello') - }, 32e3) - clock.tick(33e3) - }) - after(() => server.close()) + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] + }) + after(() => clock.uninstall()) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { - headersTimeout: 0, - connectTimeout: 0 + const orgTimers = { ...timers } + Object.assign(timers, { setTimeout, clearTimeout }) + after(() => { + Object.assign(timers, orgTimers) }) - after(() => client.destroy()) - client.request({ path: '/', method: 'GET' }, (err, response) => { - t.ifError(err) - const bufs = [] - response.body.on('data', (buf) => { - bufs.push(buf) - }) - response.body.on('end', () => { - t.strictEqual('hello', Buffer.concat(bufs).toString('utf8')) - }) + const server = createServer((req, res) => { + setTimeout(() => { + res.end('hello') + }, 32e3) + clock.tick(33e3) }) + after(() => server.close()) - clock.tick(31e3) - }) + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + headersTimeout: 0, + connectTimeout: 0 + }) + after(() => client.destroy()) - await t.completed -}) + client.request({ path: '/', method: 'GET' }, (err, response) => { + t.ifError(err) + const bufs = [] + response.body.on('data', (buf) => { + bufs.push(buf) + }) + response.body.on('end', () => { + t.strictEqual('hello', Buffer.concat(bufs).toString('utf8')) + }) + }) -test('stream timeout', async (t) => { - t = tspl(t, { plan: 1 }) + clock.tick(31e3) + }) - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] + await t.completed }) - after(() => clock.uninstall()) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) + test('Disable request timeout for a single request', async (t) => { + t = tspl(t, { plan: 2 }) - const server = createServer((req, res) => { - setTimeout(() => { - res.end('hello') - }, 301e3) - clock.tick(301e3) - }) - after(() => server.close()) + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] + }) + after(() => clock.uninstall()) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { connectTimeout: 0 }) - after(() => client.destroy()) + const orgTimers = { ...timers } + Object.assign(timers, { setTimeout, clearTimeout }) + after(() => { + Object.assign(timers, orgTimers) + }) - client.stream({ - path: '/', - method: 'GET', - opaque: new PassThrough() - }, (result) => { - t.fail('Should not be called') - }, (err) => { - t.ok(err instanceof errors.HeadersTimeoutError) + const server = createServer((req, res) => { + setTimeout(() => { + res.end('hello') + }, 32e3) + clock.tick(33e3) }) - }) + after(() => server.close()) - await t.completed -}) + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + headersTimeout: 0, + connectTimeout: 0 + }) + after(() => client.destroy()) -test('stream custom timeout', async (t) => { - t = tspl(t, { plan: 1 }) + client.request({ path: '/', method: 'GET' }, (err, response) => { + t.ifError(err) + const bufs = [] + response.body.on('data', (buf) => { + bufs.push(buf) + }) + response.body.on('end', () => { + t.strictEqual('hello', Buffer.concat(bufs).toString('utf8')) + }) + }) - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] - }) - after(() => clock.uninstall()) + clock.tick(31e3) + }) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) + await t.completed }) - const server = createServer((req, res) => { - setTimeout(() => { - res.end('hello') - }, 31e3) - clock.tick(31e3) - }) - after(() => server.close()) + test('stream timeout', async (t) => { + t = tspl(t, { plan: 1 }) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { - headersTimeout: 30e3 + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] }) - after(() => client.destroy()) + after(() => clock.uninstall()) - client.stream({ - path: '/', - method: 'GET', - opaque: new PassThrough() - }, (result) => { - t.fail('Should not be called') - }, (err) => { - t.ok(err instanceof errors.HeadersTimeoutError) + const orgTimers = { ...timers } + Object.assign(timers, { setTimeout, clearTimeout }) + after(() => { + Object.assign(timers, orgTimers) }) - }) - await t.completed -}) - -test('pipeline timeout', async (t) => { - t = tspl(t, { plan: 1 }) - - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] - }) - after(() => clock.uninstall()) + const server = createServer((req, res) => { + setTimeout(() => { + res.end('hello') + }, 301e3) + clock.tick(301e3) + }) + after(() => server.close()) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { connectTimeout: 0 }) + after(() => client.destroy()) - const server = createServer((req, res) => { - setTimeout(() => { - req.pipe(res) - }, 301e3) - clock.tick(301e3) - }) - after(() => server.close()) - - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`) - after(() => client.destroy()) - - const buf = Buffer.alloc(1e6).toString() - pipeline( - new Readable({ - read () { - this.push(buf) - this.push(null) - } - }), - client.pipeline({ + client.stream({ path: '/', - method: 'PUT' + method: 'GET', + opaque: new PassThrough() }, (result) => { t.fail('Should not be called') - }, (e) => { - t.fail('Should not be called') - }), - new Writable({ - write (chunk, encoding, callback) { - callback() - }, - final (callback) { - callback() - } - }), - (err) => { + }, (err) => { t.ok(err instanceof errors.HeadersTimeoutError) - } - ) - }) - - await t.completed -}) - -test('pipeline timeout', async (t) => { - t = tspl(t, { plan: 1 }) + }) + }) - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] + await t.completed }) - after(() => clock.uninstall()) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) + test('stream custom timeout', async (t) => { + t = tspl(t, { plan: 1 }) - const server = createServer((req, res) => { - setTimeout(() => { - req.pipe(res) - }, 31e3) - clock.tick(31e3) - }) - after(() => server.close()) + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] + }) + after(() => clock.uninstall()) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { - headersTimeout: 30e3 + const orgTimers = { ...timers } + Object.assign(timers, { setTimeout, clearTimeout }) + after(() => { + Object.assign(timers, orgTimers) }) - after(() => client.destroy()) - const buf = Buffer.alloc(1e6).toString() - pipeline( - new Readable({ - read () { - this.push(buf) - this.push(null) - } - }), - client.pipeline({ + const server = createServer((req, res) => { + setTimeout(() => { + res.end('hello') + }, 31e3) + clock.tick(31e3) + }) + after(() => server.close()) + + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + headersTimeout: 30e3 + }) + after(() => client.destroy()) + + client.stream({ path: '/', - method: 'PUT' + method: 'GET', + opaque: new PassThrough() }, (result) => { t.fail('Should not be called') - }, (e) => { - t.fail('Should not be called') - }), - new Writable({ - write (chunk, encoding, callback) { - callback() - }, - final (callback) { - callback() - } - }), - (err) => { + }, (err) => { t.ok(err instanceof errors.HeadersTimeoutError) - } - ) + }) + }) + + await t.completed + }) + + test('pipeline timeout', async (t) => { + t = tspl(t, { plan: 1 }) + + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] + }) + after(() => clock.uninstall()) + + const orgTimers = { ...timers } + Object.assign(timers, { setTimeout, clearTimeout }) + after(() => { + Object.assign(timers, orgTimers) + }) + + const server = createServer((req, res) => { + setTimeout(() => { + req.pipe(res) + }, 301e3) + clock.tick(301e3) + }) + after(() => server.close()) + + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`) + after(() => client.destroy()) + + const buf = Buffer.alloc(1e6).toString() + pipeline( + new Readable({ + read () { + this.push(buf) + this.push(null) + } + }), + client.pipeline({ + path: '/', + method: 'PUT' + }, (result) => { + t.fail('Should not be called') + }, (e) => { + t.fail('Should not be called') + }), + new Writable({ + write (chunk, encoding, callback) { + callback() + }, + final (callback) { + callback() + } + }), + (err) => { + t.ok(err instanceof errors.HeadersTimeoutError) + } + ) + }) + + await t.completed }) - await t.completed -}) + test('pipeline timeout', async (t) => { + t = tspl(t, { plan: 1 }) -test('client.close should not deadlock', async (t) => { - t = tspl(t, { plan: 2 }) + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] + }) + after(() => clock.uninstall()) - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] - }) - after(() => clock.uninstall()) + const orgTimers = { ...timers } + Object.assign(timers, { setTimeout, clearTimeout }) + after(() => { + Object.assign(timers, orgTimers) + }) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) + const server = createServer((req, res) => { + setTimeout(() => { + req.pipe(res) + }, 31e3) + clock.tick(31e3) + }) + after(() => server.close()) + + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + headersTimeout: 30e3 + }) + after(() => client.destroy()) + + const buf = Buffer.alloc(1e6).toString() + pipeline( + new Readable({ + read () { + this.push(buf) + this.push(null) + } + }), + client.pipeline({ + path: '/', + method: 'PUT' + }, (result) => { + t.fail('Should not be called') + }, (e) => { + t.fail('Should not be called') + }), + new Writable({ + write (chunk, encoding, callback) { + callback() + }, + final (callback) { + callback() + } + }), + (err) => { + t.ok(err instanceof errors.HeadersTimeoutError) + } + ) + }) - const server = createServer((req, res) => { + await t.completed }) - after(() => server.close()) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { - bodyTimeout: 200, - headersTimeout: 100 + test('client.close should not deadlock', async (t) => { + t = tspl(t, { plan: 2 }) + + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] }) - after(() => client.destroy()) + after(() => clock.uninstall()) - client[kConnect](() => { - client.request({ - path: '/', - method: 'GET' - }, (err, response) => { - t.ok(err instanceof errors.HeadersTimeoutError) - }) + const orgTimers = { ...timers } + Object.assign(timers, { setTimeout, clearTimeout }) + after(() => { + Object.assign(timers, orgTimers) + }) - client.close((err) => { - t.ifError(err) + const server = createServer((req, res) => { + }) + after(() => server.close()) + + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + bodyTimeout: 200, + headersTimeout: 100 }) + after(() => client.destroy()) - clock.tick(100) + client[kConnect](() => { + client.request({ + path: '/', + method: 'GET' + }, (err, response) => { + t.ok(err instanceof errors.HeadersTimeoutError) + }) + + client.close((err) => { + t.ifError(err) + }) + + clock.tick(100) + }) }) + await t.completed }) - await t.completed }) From 391d3369013aac2506b239457899327d48ced6ed Mon Sep 17 00:00:00 2001 From: Aras Abbasi Date: Thu, 5 Sep 2024 09:03:38 +0200 Subject: [PATCH 02/14] Apply suggestions from code review --- lib/util/timers.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/util/timers.js b/lib/util/timers.js index 871ddd2d4a3..7c23386b9a4 100644 --- a/lib/util/timers.js +++ b/lib/util/timers.js @@ -374,8 +374,6 @@ module.exports = { * @returns {FastTimer} */ setFastTimeout (callback, delay, arg) { - // If the delay is less than or equal to the RESOLUTION_MS value return a - // native Node.js Timer instance. return new FastTimer(callback, delay, arg) }, /** From 5a2d9db65cf798b8863fe959bfce513b18d2d66e Mon Sep 17 00:00:00 2001 From: uzlopak Date: Thu, 5 Sep 2024 14:34:49 +0200 Subject: [PATCH 03/14] activate some tests --- lib/util/timers.js | 5 +- test/request-timeout.js | 146 +++++++++------------------------------- 2 files changed, 36 insertions(+), 115 deletions(-) diff --git a/lib/util/timers.js b/lib/util/timers.js index 7c23386b9a4..d840c87967b 100644 --- a/lib/util/timers.js +++ b/lib/util/timers.js @@ -396,8 +396,11 @@ module.exports = { /** * Trigger the onTick function to process the fastTimers array. * Exported for testing purposes only. + * @param {number} [delay=0] The delay in milliseconds to add to the now value. */ - tick () { + tick (delay = 0) { + fastNow += delay - RESOLUTION_MS + 1 + onTick() onTick() }, /** diff --git a/test/request-timeout.js b/test/request-timeout.js index f9f9d1bb5ce..c5c5bdd996d 100644 --- a/test/request-timeout.js +++ b/test/request-timeout.js @@ -5,7 +5,6 @@ const { describe, test, after } = require('node:test') const { createReadStream, writeFileSync, unlinkSync } = require('node:fs') const { Client, errors } = require('..') const { kConnect } = require('../lib/core/symbols') -const timers = require('../lib/util/timers') const { createServer } = require('node:http') const EventEmitter = require('node:events') const FakeTimers = require('@sinonjs/fake-timers') @@ -20,7 +19,7 @@ const { tick: fastTimerTick } = require('../lib/util/timers') -describe('request timeout', { skip: true }, () => { +describe('request timeout', () => { test('request timeout', async (t) => { t = tspl(t, { plan: 1 }) @@ -76,12 +75,6 @@ describe('request timeout', { skip: true }, () => { }) after(() => clock.uninstall()) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) - const server = createServer((req, res) => { res.write('hello') }) @@ -95,12 +88,14 @@ describe('request timeout', { skip: true }, () => { t.ifError(err) body.on('data', () => { clock.tick(100) + fastTimerTick(100) }).on('error', (err) => { t.ok(err instanceof errors.BodyTimeoutError) }) }) clock.tick(50) + fastTimerTick(50) }) await t.completed @@ -115,17 +110,12 @@ describe('request timeout', { skip: true }, () => { }) after(() => clock.uninstall()) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) - const server = createServer((req, res) => { setTimeout(() => { res.end('hello') }, 100) clock.tick(100) + fastTimerTick(100) }) after(() => server.close()) @@ -138,6 +128,7 @@ describe('request timeout', { skip: true }, () => { }) clock.tick(50) + fastTimerTick(50) }) await t.completed @@ -152,12 +143,6 @@ describe('request timeout', { skip: true }, () => { }) after(() => clock.uninstall()) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) - const server = createServer((req, res) => { res.write('hello') }) @@ -193,17 +178,12 @@ describe('request timeout', { skip: true }, () => { }) after(() => clock.uninstall()) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) - const server = createServer((req, res) => { setTimeout(() => { res.end('hello') }, 100) clock.tick(100) + fastTimerTick(100) }) after(() => server.close()) @@ -219,6 +199,7 @@ describe('request timeout', { skip: true }, () => { }) clock.tick(50) + fastTimerTick(50) }) await t.completed @@ -233,17 +214,12 @@ describe('request timeout', { skip: true }, () => { }) after(() => clock.uninstall()) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) - const server = createServer((req, res) => { setTimeout(() => { res.end('hello') }, 100) clock.tick(100) + fastTimerTick(100) }) after(() => server.close()) @@ -259,6 +235,7 @@ describe('request timeout', { skip: true }, () => { }) clock.tick(50) + fastTimerTick(50) }) await t.completed @@ -273,12 +250,6 @@ describe('request timeout', { skip: true }, () => { }) after(() => clock.uninstall()) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) - const ee = new EventEmitter() const server = createServer((req, res) => { setTimeout(() => { @@ -286,6 +257,7 @@ describe('request timeout', { skip: true }, () => { }, 100) ee.emit('abort') clock.tick(50) + fastTimerTick(50) }) after(() => server.close()) @@ -298,6 +270,7 @@ describe('request timeout', { skip: true }, () => { client.request({ path: '/', method: 'GET', signal: ee }, (err, response) => { t.ok(err instanceof errors.RequestAbortedError) clock.tick(100) + fastTimerTick(100) }) }) @@ -313,12 +286,6 @@ describe('request timeout', { skip: true }, () => { }) after(() => clock.uninstall()) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) - const abortController = new AbortController() const server = createServer((req, res) => { setTimeout(() => { @@ -326,6 +293,7 @@ describe('request timeout', { skip: true }, () => { }, 100) abortController.abort() clock.tick(50) + fastTimerTick(50) }) after(() => server.close()) @@ -338,6 +306,7 @@ describe('request timeout', { skip: true }, () => { client.request({ path: '/', method: 'GET', signal: abortController.signal }, (err, response) => { t.ok(err instanceof errors.RequestAbortedError) clock.tick(100) + fastTimerTick(100) }) }) @@ -353,17 +322,12 @@ describe('request timeout', { skip: true }, () => { }) after(() => clock.uninstall()) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) - const server = createServer((req, res) => { setTimeout(() => { res.end('hello') }, 100) clock.tick(50) + fastTimerTick(50) }) after(() => server.close()) @@ -399,17 +363,12 @@ describe('request timeout', { skip: true }, () => { }) after(() => clock.uninstall()) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) - const server = createServer((req, res) => { setTimeout(() => { res.end('hello') }, 100) clock.tick(100) + fastTimerTick(100) }) after(() => server.close()) @@ -424,6 +383,7 @@ describe('request timeout', { skip: true }, () => { }) clock.tick(50) + fastTimerTick(50) }) await t.completed @@ -438,17 +398,12 @@ describe('request timeout', { skip: true }, () => { }) after(() => clock.uninstall()) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) - const server = createServer((req, res) => { setTimeout(() => { res.end('hello') }, 100) clock.tick(100) + fastTimerTick(100) }) after(() => server.close()) @@ -463,6 +418,7 @@ describe('request timeout', { skip: true }, () => { }) clock.tick(50) + fastTimerTick(50) }) await t.completed @@ -502,12 +458,6 @@ describe('request timeout', { skip: true }, () => { }) after(() => clock.uninstall()) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) - const server = createServer((req, res) => { }) after(() => server.close()) @@ -529,6 +479,7 @@ describe('request timeout', { skip: true }, () => { client.on('connect', () => { process.nextTick(() => { clock.tick(100) + fastTimerTick(100) }) }) }) @@ -587,17 +538,12 @@ describe('request timeout', { skip: true }, () => { }) after(() => clock.uninstall()) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) - const server = createServer((req, res) => { setTimeout(() => { res.end('hello') }, 32e3) clock.tick(33e3) + fastTimerTick(33e3) }) after(() => server.close()) @@ -620,6 +566,7 @@ describe('request timeout', { skip: true }, () => { }) clock.tick(31e3) + fastTimerTick(31e3) }) await t.completed @@ -634,17 +581,12 @@ describe('request timeout', { skip: true }, () => { }) after(() => clock.uninstall()) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) - const server = createServer((req, res) => { setTimeout(() => { res.end('hello') }, 32e3) clock.tick(33e3) + fastTimerTick(33e3) }) after(() => server.close()) @@ -667,12 +609,13 @@ describe('request timeout', { skip: true }, () => { }) clock.tick(31e3) + fastTimerTick(31e3) }) await t.completed }) - test('stream timeout', async (t) => { + test('stream timeout', { skip: true }, async (t) => { t = tspl(t, { plan: 1 }) const clock = FakeTimers.install({ @@ -681,17 +624,12 @@ describe('request timeout', { skip: true }, () => { }) after(() => clock.uninstall()) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) - const server = createServer((req, res) => { setTimeout(() => { res.end('hello') }, 301e3) clock.tick(301e3) + fastTimerTick(301e3) }) after(() => server.close()) @@ -713,7 +651,7 @@ describe('request timeout', { skip: true }, () => { await t.completed }) - test('stream custom timeout', async (t) => { + test('stream custom timeout', { skip: true }, async (t) => { t = tspl(t, { plan: 1 }) const clock = FakeTimers.install({ @@ -722,17 +660,12 @@ describe('request timeout', { skip: true }, () => { }) after(() => clock.uninstall()) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) - const server = createServer((req, res) => { setTimeout(() => { res.end('hello') }, 31e3) clock.tick(31e3) + fastTimerTick(31e3) }) after(() => server.close()) @@ -756,7 +689,7 @@ describe('request timeout', { skip: true }, () => { await t.completed }) - test('pipeline timeout', async (t) => { + test('pipeline timeout', { skip: true }, async (t) => { t = tspl(t, { plan: 1 }) const clock = FakeTimers.install({ @@ -765,17 +698,12 @@ describe('request timeout', { skip: true }, () => { }) after(() => clock.uninstall()) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) - const server = createServer((req, res) => { setTimeout(() => { req.pipe(res) }, 301e3) clock.tick(301e3) + fastTimerTick(301e3) }) after(() => server.close()) @@ -816,7 +744,7 @@ describe('request timeout', { skip: true }, () => { await t.completed }) - test('pipeline timeout', async (t) => { + test('pipeline timeout', { skip: true }, async (t) => { t = tspl(t, { plan: 1 }) const clock = FakeTimers.install({ @@ -825,17 +753,12 @@ describe('request timeout', { skip: true }, () => { }) after(() => clock.uninstall()) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) - const server = createServer((req, res) => { setTimeout(() => { req.pipe(res) }, 31e3) clock.tick(31e3) + fastTimerTick(31e3) }) after(() => server.close()) @@ -887,12 +810,6 @@ describe('request timeout', { skip: true }, () => { }) after(() => clock.uninstall()) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) - const server = createServer((req, res) => { }) after(() => server.close()) @@ -917,6 +834,7 @@ describe('request timeout', { skip: true }, () => { }) clock.tick(100) + fastTimerTick(100) }) }) await t.completed From 70d4d9fa0b7cce67d78a230debae87feb2eb08b9 Mon Sep 17 00:00:00 2001 From: uzlopak Date: Thu, 5 Sep 2024 14:37:03 +0200 Subject: [PATCH 04/14] also use fastTimers in connect.js --- lib/core/connect.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/core/connect.js b/lib/core/connect.js index 50578534ae8..11b0151fffb 100644 --- a/lib/core/connect.js +++ b/lib/core/connect.js @@ -166,7 +166,7 @@ const setupConnectTimeout = process.platform === 'win32' let s1 = null let s2 = null - const timer = timers.setTimeout(() => { + const fastTimer = timers.setFastTimeout(() => { // setImmediate is added to make sure that we prioritize socket error events over timeouts s1 = setImmediate(() => { // Windows needs an extra setImmediate probably due to implementation differences in the socket logic @@ -174,7 +174,7 @@ const setupConnectTimeout = process.platform === 'win32' }) }, timeout) return () => { - timers.clearTimeout(timer) + timers.clearFastTimeout(fastTimer) clearImmediate(s1) clearImmediate(s2) } @@ -185,14 +185,14 @@ const setupConnectTimeout = process.platform === 'win32' } let s1 = null - const timer = timers.setTimeout(() => { + const fastTimer = timers.setFastTimeout(() => { // setImmediate is added to make sure that we prioritize socket error events over timeouts s1 = setImmediate(() => { onConnectTimeout(socket.deref()) }) }, timeout) return () => { - timers.clearTimeout(timer) + timers.clearFastTimeout(fastTimer) clearImmediate(s1) } } From bb05bd8c5731994a684f1c7a8de5381ca9958f6a Mon Sep 17 00:00:00 2001 From: uzlopak Date: Fri, 6 Sep 2024 00:06:48 +0200 Subject: [PATCH 05/14] fix tests --- lib/util/timers.js | 9 + test/request-timeout.js | 1267 ++++++++++++++++++++------------------- 2 files changed, 644 insertions(+), 632 deletions(-) diff --git a/lib/util/timers.js b/lib/util/timers.js index d840c87967b..3ac15a20a7e 100644 --- a/lib/util/timers.js +++ b/lib/util/timers.js @@ -403,6 +403,15 @@ module.exports = { onTick() onTick() }, + /** + * Clear all FastTimers. + */ + clearAll () { + fastNow = 0 + fastTimers.length = 0 + clearTimeout(fastNowTimeout) + fastNowTimeout = null + }, /** * Exporting for testing purposes only. * Marking as deprecated to discourage any use outside of testing. diff --git a/test/request-timeout.js b/test/request-timeout.js index c5c5bdd996d..5bc78d90352 100644 --- a/test/request-timeout.js +++ b/test/request-timeout.js @@ -1,7 +1,7 @@ 'use strict' const { tspl } = require('@matteo.collina/tspl') -const { describe, test, after } = require('node:test') +const { test, after, beforeEach } = require('node:test') const { createReadStream, writeFileSync, unlinkSync } = require('node:fs') const { Client, errors } = require('..') const { kConnect } = require('../lib/core/symbols') @@ -16,827 +16,830 @@ const { PassThrough } = require('node:stream') const { - tick: fastTimerTick + tick: fastTimerTick, + clearAll: clearAllFastTimers } = require('../lib/util/timers') -describe('request timeout', () => { - test('request timeout', async (t) => { - t = tspl(t, { plan: 1 }) +beforeEach(() => { + clearAllFastTimers() +}) - const server = createServer((req, res) => { - setTimeout(() => { - res.end('hello') - }, 1000) - }) - after(() => server.close()) +test('request timeout', async (t) => { + t = tspl(t, { plan: 1 }) + + const server = createServer((req, res) => { + setTimeout(() => { + res.end('hello') + }, 1000) + }) + after(() => server.close()) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { headersTimeout: 500 }) - after(() => client.destroy()) + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { headersTimeout: 500 }) + after(() => client.destroy()) - client.request({ path: '/', method: 'GET' }, (err, response) => { - t.ok(err instanceof errors.HeadersTimeoutError) - }) + client.request({ path: '/', method: 'GET' }, (err, response) => { + t.ok(err instanceof errors.HeadersTimeoutError) }) + }) - await t.completed + await t.completed +}) + +test('request timeout with readable body', async (t) => { + t = tspl(t, { plan: 1 }) + + const server = createServer((req, res) => { }) + after(() => server.close()) + + const tempfile = `${__filename}.10mb.txt` + writeFileSync(tempfile, Buffer.alloc(10 * 1024 * 1024)) + after(() => unlinkSync(tempfile)) - test('request timeout with readable body', async (t) => { - t = tspl(t, { plan: 1 }) + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { headersTimeout: 1e3 }) + after(() => client.destroy()) - const server = createServer((req, res) => { + const body = createReadStream(tempfile) + client.request({ path: '/', method: 'POST', body }, (err, response) => { + t.ok(err instanceof errors.HeadersTimeoutError) }) - after(() => server.close()) + }) - const tempfile = `${__filename}.10mb.txt` - writeFileSync(tempfile, Buffer.alloc(10 * 1024 * 1024)) - after(() => unlinkSync(tempfile)) + await t.completed +}) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { headersTimeout: 1e3 }) - after(() => client.destroy()) +test('body timeout', async (t) => { + t = tspl(t, { plan: 2 }) - const body = createReadStream(tempfile) - client.request({ path: '/', method: 'POST', body }, (err, response) => { - t.ok(err instanceof errors.HeadersTimeoutError) + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] + }) + after(() => clock.uninstall()) + + const server = createServer((req, res) => { + res.write('hello') + }) + after(() => server.close()) + + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { bodyTimeout: 50 }) + after(() => client.destroy()) + + client.request({ path: '/', method: 'GET' }, (err, { body }) => { + t.ifError(err) + body.on('data', () => { + clock.tick(100) + fastTimerTick(100) + }).on('error', (err) => { + t.ok(err instanceof errors.BodyTimeoutError) }) }) - await t.completed + clock.tick(50) + fastTimerTick(50) }) - test('body timeout', async (t) => { - t = tspl(t, { plan: 2 }) + await t.completed +}) - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] - }) - after(() => clock.uninstall()) +test('overridden request timeout', async (t) => { + t = tspl(t, { plan: 1 }) - const server = createServer((req, res) => { - res.write('hello') - }) - after(() => server.close()) + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] + }) + after(() => clock.uninstall()) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { bodyTimeout: 50 }) - after(() => client.destroy()) + const server = createServer((req, res) => { + setTimeout(() => { + res.end('hello') + }, 100) + clock.tick(100) + fastTimerTick(100) + }) + after(() => server.close()) - client.request({ path: '/', method: 'GET' }, (err, { body }) => { - t.ifError(err) - body.on('data', () => { - clock.tick(100) - fastTimerTick(100) - }).on('error', (err) => { - t.ok(err instanceof errors.BodyTimeoutError) - }) - }) + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { headersTimeout: 500 }) + after(() => client.destroy()) - clock.tick(50) - fastTimerTick(50) + client.request({ path: '/', method: 'GET', headersTimeout: 50 }, (err, response) => { + t.ok(err instanceof errors.HeadersTimeoutError) }) - await t.completed + clock.tick(50) + fastTimerTick(50) }) - test('overridden request timeout', async (t) => { - t = tspl(t, { plan: 1 }) + await t.completed +}) - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] - }) - after(() => clock.uninstall()) +test('overridden body timeout', async (t) => { + t = tspl(t, { plan: 2 }) - const server = createServer((req, res) => { - setTimeout(() => { - res.end('hello') - }, 100) - clock.tick(100) - fastTimerTick(100) - }) - after(() => server.close()) + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] + }) + after(() => clock.uninstall()) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { headersTimeout: 500 }) - after(() => client.destroy()) + const server = createServer((req, res) => { + res.write('hello') + }) + after(() => server.close()) - client.request({ path: '/', method: 'GET', headersTimeout: 50 }, (err, response) => { - t.ok(err instanceof errors.HeadersTimeoutError) - }) + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { bodyTimeout: 500 }) + after(() => client.destroy()) - clock.tick(50) - fastTimerTick(50) + client.request({ path: '/', method: 'GET', bodyTimeout: 50 }, (err, { body }) => { + t.ifError(err) + body.on('data', () => { + fastTimerTick() + fastTimerTick() + }).on('error', (err) => { + t.ok(err instanceof errors.BodyTimeoutError) + }) }) - await t.completed + fastTimerTick() + fastTimerTick() }) - test('overridden body timeout', async (t) => { - t = tspl(t, { plan: 2 }) + await t.completed +}) + +test('With EE signal', async (t) => { + t = tspl(t, { plan: 1 }) - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] + }) + after(() => clock.uninstall()) + + const server = createServer((req, res) => { + setTimeout(() => { + res.end('hello') + }, 100) + clock.tick(100) + fastTimerTick(100) + }) + after(() => server.close()) + + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + headersTimeout: 50 }) - after(() => clock.uninstall()) + const ee = new EventEmitter() + after(() => client.destroy()) - const server = createServer((req, res) => { - res.write('hello') + client.request({ path: '/', method: 'GET', signal: ee }, (err, response) => { + t.ok(err instanceof errors.HeadersTimeoutError) }) - after(() => server.close()) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { bodyTimeout: 500 }) - after(() => client.destroy()) + clock.tick(50) + fastTimerTick(50) + }) - client.request({ path: '/', method: 'GET', bodyTimeout: 50 }, (err, { body }) => { - t.ifError(err) - body.on('data', () => { - fastTimerTick() - fastTimerTick() - }).on('error', (err) => { - t.ok(err instanceof errors.BodyTimeoutError) - }) - }) + await t.completed +}) - fastTimerTick() - fastTimerTick() - }) +test('With abort-controller signal', async (t) => { + t = tspl(t, { plan: 1 }) - await t.completed + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] }) + after(() => clock.uninstall()) - test('With EE signal', async (t) => { - t = tspl(t, { plan: 1 }) + const server = createServer((req, res) => { + setTimeout(() => { + res.end('hello') + }, 100) + clock.tick(100) + fastTimerTick(100) + }) + after(() => server.close()) - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + headersTimeout: 50 }) - after(() => clock.uninstall()) + const abortController = new AbortController() + after(() => client.destroy()) - const server = createServer((req, res) => { - setTimeout(() => { - res.end('hello') - }, 100) - clock.tick(100) - fastTimerTick(100) + client.request({ path: '/', method: 'GET', signal: abortController.signal }, (err, response) => { + t.ok(err instanceof errors.HeadersTimeoutError) }) - after(() => server.close()) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { - headersTimeout: 50 - }) - const ee = new EventEmitter() - after(() => client.destroy()) + clock.tick(50) + fastTimerTick(50) + }) - client.request({ path: '/', method: 'GET', signal: ee }, (err, response) => { - t.ok(err instanceof errors.HeadersTimeoutError) - }) + await t.completed +}) - clock.tick(50) - fastTimerTick(50) - }) +test('Abort before timeout (EE)', async (t) => { + t = tspl(t, { plan: 1 }) - await t.completed + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] }) + after(() => clock.uninstall()) - test('With abort-controller signal', async (t) => { - t = tspl(t, { plan: 1 }) + const ee = new EventEmitter() + const server = createServer((req, res) => { + setTimeout(() => { + res.end('hello') + }, 100) + ee.emit('abort') + clock.tick(50) + fastTimerTick(50) + }) + after(() => server.close()) - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + headersTimeout: 50 }) - after(() => clock.uninstall()) + after(() => client.destroy()) - const server = createServer((req, res) => { - setTimeout(() => { - res.end('hello') - }, 100) + client.request({ path: '/', method: 'GET', signal: ee }, (err, response) => { + t.ok(err instanceof errors.RequestAbortedError) clock.tick(100) fastTimerTick(100) }) - after(() => server.close()) + }) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { - headersTimeout: 50 - }) - const abortController = new AbortController() - after(() => client.destroy()) + await t.completed +}) - client.request({ path: '/', method: 'GET', signal: abortController.signal }, (err, response) => { - t.ok(err instanceof errors.HeadersTimeoutError) - }) +test('Abort before timeout (abort-controller)', async (t) => { + t = tspl(t, { plan: 1 }) - clock.tick(50) - fastTimerTick(50) - }) + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] + }) + after(() => clock.uninstall()) - await t.completed + const abortController = new AbortController() + const server = createServer((req, res) => { + setTimeout(() => { + res.end('hello') + }, 100) + abortController.abort() + clock.tick(50) + fastTimerTick(50) }) + after(() => server.close()) - test('Abort before timeout (EE)', async (t) => { - t = tspl(t, { plan: 1 }) + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + headersTimeout: 50 + }) + after(() => client.destroy()) - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] + client.request({ path: '/', method: 'GET', signal: abortController.signal }, (err, response) => { + t.ok(err instanceof errors.RequestAbortedError) + clock.tick(100) + fastTimerTick(100) }) - after(() => clock.uninstall()) + }) - const ee = new EventEmitter() - const server = createServer((req, res) => { - setTimeout(() => { - res.end('hello') - }, 100) - ee.emit('abort') - clock.tick(50) - fastTimerTick(50) - }) - after(() => server.close()) - - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { - headersTimeout: 50 - }) - after(() => client.destroy()) + await t.completed +}) - client.request({ path: '/', method: 'GET', signal: ee }, (err, response) => { - t.ok(err instanceof errors.RequestAbortedError) - clock.tick(100) - fastTimerTick(100) - }) - }) +test('Timeout with pipelining', async (t) => { + t = tspl(t, { plan: 3 }) - await t.completed + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] }) + after(() => clock.uninstall()) - test('Abort before timeout (abort-controller)', async (t) => { - t = tspl(t, { plan: 1 }) + const server = createServer((req, res) => { + setTimeout(() => { + res.end('hello') + }, 100) + clock.tick(50) + fastTimerTick(50) + }) + after(() => server.close()) - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + pipelining: 10, + headersTimeout: 50 }) - after(() => clock.uninstall()) + after(() => client.destroy()) - const abortController = new AbortController() - const server = createServer((req, res) => { - setTimeout(() => { - res.end('hello') - }, 100) - abortController.abort() - clock.tick(50) - fastTimerTick(50) - }) - after(() => server.close()) - - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { - headersTimeout: 50 - }) - after(() => client.destroy()) + client.request({ path: '/', method: 'GET' }, (err, response) => { + t.ok(err instanceof errors.HeadersTimeoutError) + }) - client.request({ path: '/', method: 'GET', signal: abortController.signal }, (err, response) => { - t.ok(err instanceof errors.RequestAbortedError) - clock.tick(100) - fastTimerTick(100) - }) + client.request({ path: '/', method: 'GET' }, (err, response) => { + t.ok(err instanceof errors.HeadersTimeoutError) }) - await t.completed + client.request({ path: '/', method: 'GET' }, (err, response) => { + t.ok(err instanceof errors.HeadersTimeoutError) + }) }) - test('Timeout with pipelining', async (t) => { - t = tspl(t, { plan: 3 }) + await t.completed +}) - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] - }) - after(() => clock.uninstall()) +test('Global option', async (t) => { + t = tspl(t, { plan: 1 }) - const server = createServer((req, res) => { - setTimeout(() => { - res.end('hello') - }, 100) - clock.tick(50) - fastTimerTick(50) + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] + }) + after(() => clock.uninstall()) + + const server = createServer((req, res) => { + setTimeout(() => { + res.end('hello') + }, 100) + clock.tick(100) + fastTimerTick(100) + }) + after(() => server.close()) + + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + headersTimeout: 50 }) - after(() => server.close()) + after(() => client.destroy()) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { - pipelining: 10, - headersTimeout: 50 - }) - after(() => client.destroy()) + client.request({ path: '/', method: 'GET' }, (err, response) => { + t.ok(err instanceof errors.HeadersTimeoutError) + }) - client.request({ path: '/', method: 'GET' }, (err, response) => { - t.ok(err instanceof errors.HeadersTimeoutError) - }) + clock.tick(50) + fastTimerTick(50) + }) - client.request({ path: '/', method: 'GET' }, (err, response) => { - t.ok(err instanceof errors.HeadersTimeoutError) - }) + await t.completed +}) - client.request({ path: '/', method: 'GET' }, (err, response) => { - t.ok(err instanceof errors.HeadersTimeoutError) - }) - }) +test('Request options overrides global option', async (t) => { + t = tspl(t, { plan: 1 }) - await t.completed + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] }) + after(() => clock.uninstall()) - test('Global option', async (t) => { - t = tspl(t, { plan: 1 }) + const server = createServer((req, res) => { + setTimeout(() => { + res.end('hello') + }, 100) + clock.tick(100) + fastTimerTick(100) + }) + after(() => server.close()) - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + headersTimeout: 50 }) - after(() => clock.uninstall()) + after(() => client.destroy()) - const server = createServer((req, res) => { - setTimeout(() => { - res.end('hello') - }, 100) - clock.tick(100) - fastTimerTick(100) + client.request({ path: '/', method: 'GET' }, (err, response) => { + t.ok(err instanceof errors.HeadersTimeoutError) }) - after(() => server.close()) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { - headersTimeout: 50 - }) - after(() => client.destroy()) + clock.tick(50) + fastTimerTick(50) + }) - client.request({ path: '/', method: 'GET' }, (err, response) => { - t.ok(err instanceof errors.HeadersTimeoutError) - }) + await t.completed +}) - clock.tick(50) - fastTimerTick(50) - }) +test('client.destroy should cancel the timeout', async (t) => { + t = tspl(t, { plan: 2 }) - await t.completed + const server = createServer((req, res) => { + res.end('hello') }) + after(() => server.close()) - test('Request options overrides global option', async (t) => { - t = tspl(t, { plan: 1 }) - - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + headersTimeout: 100 }) - after(() => clock.uninstall()) - const server = createServer((req, res) => { - setTimeout(() => { - res.end('hello') - }, 100) - clock.tick(100) - fastTimerTick(100) + client.request({ path: '/', method: 'GET' }, (err, response) => { + t.ok(err instanceof errors.ClientDestroyedError) }) - after(() => server.close()) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { - headersTimeout: 50 - }) - after(() => client.destroy()) + client.destroy(err => { + t.ifError(err) + }) + }) - client.request({ path: '/', method: 'GET' }, (err, response) => { - t.ok(err instanceof errors.HeadersTimeoutError) - }) + await t.completed +}) - clock.tick(50) - fastTimerTick(50) - }) +test('client.close should wait for the timeout', async (t) => { + t = tspl(t, { plan: 2 }) - await t.completed + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] }) + after(() => clock.uninstall()) - test('client.destroy should cancel the timeout', async (t) => { - t = tspl(t, { plan: 2 }) + const server = createServer((req, res) => { + }) + after(() => server.close()) - const server = createServer((req, res) => { - res.end('hello') + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + headersTimeout: 100 }) - after(() => server.close()) + after(() => client.destroy()) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { - headersTimeout: 100 - }) + client.request({ path: '/', method: 'GET' }, (err, response) => { + t.ok(err instanceof errors.HeadersTimeoutError) + }) - client.request({ path: '/', method: 'GET' }, (err, response) => { - t.ok(err instanceof errors.ClientDestroyedError) - }) + client.close((err) => { + t.ifError(err) + }) - client.destroy(err => { - t.ifError(err) + client.on('connect', () => { + process.nextTick(() => { + clock.tick(100) + fastTimerTick(100) }) }) - - await t.completed }) - test('client.close should wait for the timeout', async (t) => { - t = tspl(t, { plan: 2 }) + await t.completed +}) + +test('Validation', async (t) => { + t = tspl(t, { plan: 4 }) - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] + try { + const client = new Client('http://localhost:3000', { + headersTimeout: 'foobar' }) - after(() => clock.uninstall()) + after(() => client.destroy()) + } catch (err) { + t.ok(err instanceof errors.InvalidArgumentError) + } - const server = createServer((req, res) => { + try { + const client = new Client('http://localhost:3000', { + headersTimeout: -1 }) - after(() => server.close()) + after(() => client.destroy()) + } catch (err) { + t.ok(err instanceof errors.InvalidArgumentError) + } - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { - headersTimeout: 100 - }) - after(() => client.destroy()) + try { + const client = new Client('http://localhost:3000', { + bodyTimeout: 'foobar' + }) + after(() => client.destroy()) + } catch (err) { + t.ok(err instanceof errors.InvalidArgumentError) + } - client.request({ path: '/', method: 'GET' }, (err, response) => { - t.ok(err instanceof errors.HeadersTimeoutError) - }) + try { + const client = new Client('http://localhost:3000', { + bodyTimeout: -1 + }) + after(() => client.destroy()) + } catch (err) { + t.ok(err instanceof errors.InvalidArgumentError) + } - client.close((err) => { - t.ifError(err) - }) + await t.completed +}) - client.on('connect', () => { - process.nextTick(() => { - clock.tick(100) - fastTimerTick(100) - }) - }) - }) +test('Disable request timeout', async (t) => { + t = tspl(t, { plan: 2 }) - await t.completed + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] }) + after(() => clock.uninstall()) - test('Validation', async (t) => { - t = tspl(t, { plan: 4 }) + const server = createServer((req, res) => { + setTimeout(() => { + res.end('hello') + }, 32e3) + clock.tick(33e3) + fastTimerTick(33e3) + }) + after(() => server.close()) - try { - const client = new Client('http://localhost:3000', { - headersTimeout: 'foobar' - }) - after(() => client.destroy()) - } catch (err) { - t.ok(err instanceof errors.InvalidArgumentError) - } - - try { - const client = new Client('http://localhost:3000', { - headersTimeout: -1 - }) - after(() => client.destroy()) - } catch (err) { - t.ok(err instanceof errors.InvalidArgumentError) - } - - try { - const client = new Client('http://localhost:3000', { - bodyTimeout: 'foobar' + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + headersTimeout: 0, + connectTimeout: 0 + }) + after(() => client.destroy()) + + client.request({ path: '/', method: 'GET' }, (err, response) => { + t.ifError(err) + const bufs = [] + response.body.on('data', (buf) => { + bufs.push(buf) }) - after(() => client.destroy()) - } catch (err) { - t.ok(err instanceof errors.InvalidArgumentError) - } - - try { - const client = new Client('http://localhost:3000', { - bodyTimeout: -1 + response.body.on('end', () => { + t.strictEqual('hello', Buffer.concat(bufs).toString('utf8')) }) - after(() => client.destroy()) - } catch (err) { - t.ok(err instanceof errors.InvalidArgumentError) - } + }) - await t.completed + clock.tick(31e3) + fastTimerTick(31e3) }) - test('Disable request timeout', async (t) => { - t = tspl(t, { plan: 2 }) + await t.completed +}) - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] - }) - after(() => clock.uninstall()) +test('Disable request timeout for a single request', async (t) => { + t = tspl(t, { plan: 2 }) + + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] + }) + after(() => clock.uninstall()) - const server = createServer((req, res) => { - setTimeout(() => { - res.end('hello') - }, 32e3) - clock.tick(33e3) - fastTimerTick(33e3) + const server = createServer((req, res) => { + setTimeout(() => { + res.end('hello') + }, 32e3) + clock.tick(33e3) + fastTimerTick(33e3) + }) + after(() => server.close()) + + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + headersTimeout: 0, + connectTimeout: 0 }) - after(() => server.close()) + after(() => client.destroy()) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { - headersTimeout: 0, - connectTimeout: 0 + client.request({ path: '/', method: 'GET' }, (err, response) => { + t.ifError(err) + const bufs = [] + response.body.on('data', (buf) => { + bufs.push(buf) }) - after(() => client.destroy()) - - client.request({ path: '/', method: 'GET' }, (err, response) => { - t.ifError(err) - const bufs = [] - response.body.on('data', (buf) => { - bufs.push(buf) - }) - response.body.on('end', () => { - t.strictEqual('hello', Buffer.concat(bufs).toString('utf8')) - }) + response.body.on('end', () => { + t.strictEqual('hello', Buffer.concat(bufs).toString('utf8')) }) - - clock.tick(31e3) - fastTimerTick(31e3) }) - await t.completed + clock.tick(31e3) + fastTimerTick(31e3) }) - test('Disable request timeout for a single request', async (t) => { - t = tspl(t, { plan: 2 }) + await t.completed +}) - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] - }) - after(() => clock.uninstall()) +test('stream timeout', async (t) => { + t = tspl(t, { plan: 1 }) - const server = createServer((req, res) => { - setTimeout(() => { - res.end('hello') - }, 32e3) - clock.tick(33e3) - fastTimerTick(33e3) - }) - after(() => server.close()) + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] + }) + after(() => clock.uninstall()) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { - headersTimeout: 0, - connectTimeout: 0 - }) - after(() => client.destroy()) + const server = createServer((req, res) => { + setTimeout(() => { + res.end('hello') + }, 301e3) + clock.tick(301e3) + fastTimerTick(301e3) + }) + after(() => server.close()) - client.request({ path: '/', method: 'GET' }, (err, response) => { - t.ifError(err) - const bufs = [] - response.body.on('data', (buf) => { - bufs.push(buf) - }) - response.body.on('end', () => { - t.strictEqual('hello', Buffer.concat(bufs).toString('utf8')) - }) - }) + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { connectTimeout: 0 }) + after(() => client.destroy()) - clock.tick(31e3) - fastTimerTick(31e3) + client.stream({ + path: '/', + method: 'GET', + opaque: new PassThrough() + }, (result) => { + t.fail('Should not be called') + }, (err) => { + t.ok(err instanceof errors.HeadersTimeoutError) }) + }) + + await t.completed +}) + +test('stream custom timeout', async (t) => { + t = tspl(t, { plan: 1 }) - await t.completed + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] }) + after(() => clock.uninstall()) - test('stream timeout', { skip: true }, async (t) => { - t = tspl(t, { plan: 1 }) + const server = createServer((req, res) => { + setTimeout(() => { + res.end('hello') + }, 31e3) + clock.tick(31e3) + fastTimerTick(31e3) + }) + after(() => server.close()) - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + headersTimeout: 30e3 }) - after(() => clock.uninstall()) + after(() => client.destroy()) - const server = createServer((req, res) => { - setTimeout(() => { - res.end('hello') - }, 301e3) - clock.tick(301e3) - fastTimerTick(301e3) + client.stream({ + path: '/', + method: 'GET', + opaque: new PassThrough() + }, (result) => { + t.fail('Should not be called') + }, (err) => { + t.ok(err instanceof errors.HeadersTimeoutError) }) - after(() => server.close()) + }) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { connectTimeout: 0 }) - after(() => client.destroy()) + await t.completed +}) - client.stream({ +test('pipeline timeout', async (t) => { + t = tspl(t, { plan: 1 }) + + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] + }) + after(() => clock.uninstall()) + + const server = createServer((req, res) => { + setTimeout(() => { + req.pipe(res) + }, 301e3) + clock.tick(301e3) + fastTimerTick(301e3) + }) + after(() => server.close()) + + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`) + after(() => client.destroy()) + + const buf = Buffer.alloc(1e6).toString() + pipeline( + new Readable({ + read () { + this.push(buf) + this.push(null) + } + }), + client.pipeline({ path: '/', - method: 'GET', - opaque: new PassThrough() + method: 'PUT' }, (result) => { t.fail('Should not be called') - }, (err) => { + }, (e) => { + t.fail('Should not be called') + }), + new Writable({ + write (chunk, encoding, callback) { + callback() + }, + final (callback) { + callback() + } + }), + (err) => { t.ok(err instanceof errors.HeadersTimeoutError) - }) - }) - - await t.completed + } + ) }) - test('stream custom timeout', { skip: true }, async (t) => { - t = tspl(t, { plan: 1 }) + await t.completed +}) - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] - }) - after(() => clock.uninstall()) +test('pipeline timeout', async (t) => { + t = tspl(t, { plan: 1 }) - const server = createServer((req, res) => { - setTimeout(() => { - res.end('hello') - }, 31e3) - clock.tick(31e3) - fastTimerTick(31e3) - }) - after(() => server.close()) + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] + }) + after(() => clock.uninstall()) + + const server = createServer((req, res) => { + setTimeout(() => { + req.pipe(res) + }, 31e3) + clock.tick(31e3) + fastTimerTick(31e3) + }) + after(() => server.close()) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { - headersTimeout: 30e3 - }) - after(() => client.destroy()) + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + headersTimeout: 30e3 + }) + after(() => client.destroy()) - client.stream({ + const buf = Buffer.alloc(1e6).toString() + pipeline( + new Readable({ + read () { + this.push(buf) + this.push(null) + } + }), + client.pipeline({ path: '/', - method: 'GET', - opaque: new PassThrough() + method: 'PUT' }, (result) => { t.fail('Should not be called') - }, (err) => { - t.ok(err instanceof errors.HeadersTimeoutError) - }) - }) - - await t.completed - }) - - test('pipeline timeout', { skip: true }, async (t) => { - t = tspl(t, { plan: 1 }) - - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] - }) - after(() => clock.uninstall()) - - const server = createServer((req, res) => { - setTimeout(() => { - req.pipe(res) - }, 301e3) - clock.tick(301e3) - fastTimerTick(301e3) - }) - after(() => server.close()) - - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`) - after(() => client.destroy()) - - const buf = Buffer.alloc(1e6).toString() - pipeline( - new Readable({ - read () { - this.push(buf) - this.push(null) - } - }), - client.pipeline({ - path: '/', - method: 'PUT' - }, (result) => { - t.fail('Should not be called') - }, (e) => { - t.fail('Should not be called') - }), - new Writable({ - write (chunk, encoding, callback) { - callback() - }, - final (callback) { - callback() - } - }), - (err) => { - t.ok(err instanceof errors.HeadersTimeoutError) + }, (e) => { + t.fail('Should not be called') + }), + new Writable({ + write (chunk, encoding, callback) { + callback() + }, + final (callback) { + callback() } - ) - }) - - await t.completed + }), + (err) => { + t.ok(err instanceof errors.HeadersTimeoutError) + } + ) }) - test('pipeline timeout', { skip: true }, async (t) => { - t = tspl(t, { plan: 1 }) - - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] - }) - after(() => clock.uninstall()) - - const server = createServer((req, res) => { - setTimeout(() => { - req.pipe(res) - }, 31e3) - clock.tick(31e3) - fastTimerTick(31e3) - }) - after(() => server.close()) + await t.completed +}) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { - headersTimeout: 30e3 - }) - after(() => client.destroy()) - - const buf = Buffer.alloc(1e6).toString() - pipeline( - new Readable({ - read () { - this.push(buf) - this.push(null) - } - }), - client.pipeline({ - path: '/', - method: 'PUT' - }, (result) => { - t.fail('Should not be called') - }, (e) => { - t.fail('Should not be called') - }), - new Writable({ - write (chunk, encoding, callback) { - callback() - }, - final (callback) { - callback() - } - }), - (err) => { - t.ok(err instanceof errors.HeadersTimeoutError) - } - ) - }) +test('client.close should not deadlock', async (t) => { + t = tspl(t, { plan: 2 }) - await t.completed + const clock = FakeTimers.install({ + shouldClearNativeTimers: true, + toFake: ['setTimeout', 'clearTimeout'] }) + after(() => clock.uninstall()) - test('client.close should not deadlock', async (t) => { - t = tspl(t, { plan: 2 }) - - const clock = FakeTimers.install({ - shouldClearNativeTimers: true, - toFake: ['setTimeout', 'clearTimeout'] - }) - after(() => clock.uninstall()) + const server = createServer((req, res) => { + }) + after(() => server.close()) - const server = createServer((req, res) => { + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + bodyTimeout: 200, + headersTimeout: 100 }) - after(() => server.close()) + after(() => client.destroy()) - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`, { - bodyTimeout: 200, - headersTimeout: 100 + client[kConnect](() => { + client.request({ + path: '/', + method: 'GET' + }, (err, response) => { + t.ok(err instanceof errors.HeadersTimeoutError) }) - after(() => client.destroy()) - - client[kConnect](() => { - client.request({ - path: '/', - method: 'GET' - }, (err, response) => { - t.ok(err instanceof errors.HeadersTimeoutError) - }) - - client.close((err) => { - t.ifError(err) - }) - clock.tick(100) - fastTimerTick(100) + client.close((err) => { + t.ifError(err) }) + + clock.tick(100) + fastTimerTick(100) }) - await t.completed }) + await t.completed }) From 875cbddf3864ce37743a25368cc47c54ce30713c Mon Sep 17 00:00:00 2001 From: uzlopak Date: Fri, 6 Sep 2024 00:10:40 +0200 Subject: [PATCH 06/14] fix tests --- test/issue-3356.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/issue-3356.js b/test/issue-3356.js index b7820204dd9..b0a8cb4408d 100644 --- a/test/issue-3356.js +++ b/test/issue-3356.js @@ -43,7 +43,6 @@ test('https://github.com/nodejs/undici/issues/3356', async (t) => { dispatcher: agent }) - fastTimerTick() fastTimerTick() setTimeout(async () => { From 77f7337804fd94f6846a7e1396f799288960819f Mon Sep 17 00:00:00 2001 From: uzlopak Date: Fri, 6 Sep 2024 00:12:41 +0200 Subject: [PATCH 07/14] fix tests --- lib/util/timers.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/util/timers.js b/lib/util/timers.js index 3ac15a20a7e..9bb1c0ec838 100644 --- a/lib/util/timers.js +++ b/lib/util/timers.js @@ -396,6 +396,8 @@ module.exports = { /** * Trigger the onTick function to process the fastTimers array. * Exported for testing purposes only. + * Marking as deprecated to discourage any use outside of testing. + * @deprecated * @param {number} [delay=0] The delay in milliseconds to add to the now value. */ tick (delay = 0) { @@ -405,6 +407,9 @@ module.exports = { }, /** * Clear all FastTimers. + * Exported for testing purposes only. + * Marking as deprecated to discourage any use outside of testing. + * @deprecated */ clearAll () { fastNow = 0 From 5f056406186b21c7c0f46b92134353e3fb513749 Mon Sep 17 00:00:00 2001 From: Aras Abbasi Date: Fri, 6 Sep 2024 00:15:17 +0200 Subject: [PATCH 08/14] fix: use native timeouts for TIMEOUT_IDLE, rename TIMEOUT_IDLE to TIMEOUT_KEEP_ALIVE (#3554) * fix: use native timeouts for TIMEOUT_IDLE, rename TIMEOUT_IDLE to TIMEOUT_KEEP_ALIVE * ensure that fast timers and native timers are set properly * . * . --- lib/dispatcher/client-h1.js | 49 ++++++++++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/lib/dispatcher/client-h1.js b/lib/dispatcher/client-h1.js index 3895fc5874f..cb51ce2fb27 100644 --- a/lib/dispatcher/client-h1.js +++ b/lib/dispatcher/client-h1.js @@ -129,9 +129,17 @@ let currentBufferRef = null let currentBufferSize = 0 let currentBufferPtr = null -const TIMEOUT_HEADERS = 1 -const TIMEOUT_BODY = 2 -const TIMEOUT_IDLE = 3 +const USE_NATIVE_TIMER = 0 +const USE_FAST_TIMER = 1 + +// Use fast timers for headers and body to take eventual event loop +// latency into account. +const TIMEOUT_HEADERS = 2 | USE_FAST_TIMER +const TIMEOUT_BODY = 4 | USE_FAST_TIMER + +// Use native timers to ignore event loop latency for keep-alive +// handling. +const TIMEOUT_KEEP_ALIVE = 8 | USE_NATIVE_TIMER class Parser { constructor (client, socket, { exports }) { @@ -163,14 +171,29 @@ class Parser { } setTimeout (delay, type) { - this.timeoutType = type - if (delay !== this.timeoutValue) { - this.timeout && timers.clearFastTimeout(this.timeout) - if (delay) { - this.timeout = timers.setFastTimeout(onParserTimeout, delay, new WeakRef(this)) - } else { + // If the existing timer and the new timer are of different timer type + // (fast or native) or have different delay, we need to clear the existing + // timer and set a new one. + if ( + delay !== this.timeoutValue || + (type & USE_FAST_TIMER) ^ (this.timeoutType & USE_FAST_TIMER) + ) { + // If a timeout is already set, clear it with clearTimeout of the fast + // timer implementation, as it can clear fast and native timers. + if (this.timeout) { + timers.clearTimeout(this.timeout) this.timeout = null } + + if (delay) { + if (type & USE_FAST_TIMER) { + this.timeout = timers.setFastTimeout(onParserTimeout, delay, new WeakRef(this)) + } else { + this.timeout = setTimeout(onParserTimeout, delay, new WeakRef(this)) + this.timeout.unref() + } + } + this.timeoutValue = delay } else if (this.timeout) { // istanbul ignore else: only for jest @@ -178,6 +201,8 @@ class Parser { this.timeout.refresh() } } + + this.timeoutType = type } resume () { @@ -625,7 +650,7 @@ function onParserTimeout (parser) { if (!paused) { util.destroy(socket, new BodyTimeoutError()) } - } else if (timeoutType === TIMEOUT_IDLE) { + } else if (timeoutType === TIMEOUT_KEEP_ALIVE) { assert(client[kRunning] === 0 && client[kKeepAliveTimeoutValue]) util.destroy(socket, new InformationalError('socket idle timeout')) } @@ -803,8 +828,8 @@ function resumeH1 (client) { } if (client[kSize] === 0) { - if (socket[kParser].timeoutType !== TIMEOUT_IDLE) { - socket[kParser].setTimeout(client[kKeepAliveTimeoutValue], TIMEOUT_IDLE) + if (socket[kParser].timeoutType !== TIMEOUT_KEEP_ALIVE) { + socket[kParser].setTimeout(client[kKeepAliveTimeoutValue], TIMEOUT_KEEP_ALIVE) } } else if (client[kRunning] > 0 && socket[kParser].statusCode < 200) { if (socket[kParser].timeoutType !== TIMEOUT_HEADERS) { From b08e513944071be4c82f3f0454cbc111dcb5e870 Mon Sep 17 00:00:00 2001 From: uzlopak Date: Fri, 6 Sep 2024 00:24:39 +0200 Subject: [PATCH 09/14] rename import --- test/issue-3356.js | 4 +-- test/request-timeout.js | 64 ++++++++++++++++++++--------------------- 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/test/issue-3356.js b/test/issue-3356.js index b0a8cb4408d..fd7bf59656f 100644 --- a/test/issue-3356.js +++ b/test/issue-3356.js @@ -4,7 +4,7 @@ const { tspl } = require('@matteo.collina/tspl') const { test, after } = require('node:test') const { createServer } = require('node:http') const { once } = require('node:events') -const { tick: fastTimerTick } = require('../lib/util/timers') +const { tick: fastTimersTick } = require('../lib/util/timers') const { fetch, Agent, RetryAgent } = require('..') test('https://github.com/nodejs/undici/issues/3356', async (t) => { @@ -43,7 +43,7 @@ test('https://github.com/nodejs/undici/issues/3356', async (t) => { dispatcher: agent }) - fastTimerTick() + fastTimersTick() setTimeout(async () => { try { diff --git a/test/request-timeout.js b/test/request-timeout.js index 5bc78d90352..0f7618c75a0 100644 --- a/test/request-timeout.js +++ b/test/request-timeout.js @@ -16,7 +16,7 @@ const { PassThrough } = require('node:stream') const { - tick: fastTimerTick, + tick: fastTimersTick, clearAll: clearAllFastTimers } = require('../lib/util/timers') @@ -92,14 +92,14 @@ test('body timeout', async (t) => { t.ifError(err) body.on('data', () => { clock.tick(100) - fastTimerTick(100) + fastTimersTick(100) }).on('error', (err) => { t.ok(err instanceof errors.BodyTimeoutError) }) }) clock.tick(50) - fastTimerTick(50) + fastTimersTick(50) }) await t.completed @@ -119,7 +119,7 @@ test('overridden request timeout', async (t) => { res.end('hello') }, 100) clock.tick(100) - fastTimerTick(100) + fastTimersTick(100) }) after(() => server.close()) @@ -132,7 +132,7 @@ test('overridden request timeout', async (t) => { }) clock.tick(50) - fastTimerTick(50) + fastTimersTick(50) }) await t.completed @@ -159,15 +159,15 @@ test('overridden body timeout', async (t) => { client.request({ path: '/', method: 'GET', bodyTimeout: 50 }, (err, { body }) => { t.ifError(err) body.on('data', () => { - fastTimerTick() - fastTimerTick() + fastTimersTick() + fastTimersTick() }).on('error', (err) => { t.ok(err instanceof errors.BodyTimeoutError) }) }) - fastTimerTick() - fastTimerTick() + fastTimersTick() + fastTimersTick() }) await t.completed @@ -187,7 +187,7 @@ test('With EE signal', async (t) => { res.end('hello') }, 100) clock.tick(100) - fastTimerTick(100) + fastTimersTick(100) }) after(() => server.close()) @@ -203,7 +203,7 @@ test('With EE signal', async (t) => { }) clock.tick(50) - fastTimerTick(50) + fastTimersTick(50) }) await t.completed @@ -223,7 +223,7 @@ test('With abort-controller signal', async (t) => { res.end('hello') }, 100) clock.tick(100) - fastTimerTick(100) + fastTimersTick(100) }) after(() => server.close()) @@ -239,7 +239,7 @@ test('With abort-controller signal', async (t) => { }) clock.tick(50) - fastTimerTick(50) + fastTimersTick(50) }) await t.completed @@ -261,7 +261,7 @@ test('Abort before timeout (EE)', async (t) => { }, 100) ee.emit('abort') clock.tick(50) - fastTimerTick(50) + fastTimersTick(50) }) after(() => server.close()) @@ -274,7 +274,7 @@ test('Abort before timeout (EE)', async (t) => { client.request({ path: '/', method: 'GET', signal: ee }, (err, response) => { t.ok(err instanceof errors.RequestAbortedError) clock.tick(100) - fastTimerTick(100) + fastTimersTick(100) }) }) @@ -297,7 +297,7 @@ test('Abort before timeout (abort-controller)', async (t) => { }, 100) abortController.abort() clock.tick(50) - fastTimerTick(50) + fastTimersTick(50) }) after(() => server.close()) @@ -310,7 +310,7 @@ test('Abort before timeout (abort-controller)', async (t) => { client.request({ path: '/', method: 'GET', signal: abortController.signal }, (err, response) => { t.ok(err instanceof errors.RequestAbortedError) clock.tick(100) - fastTimerTick(100) + fastTimersTick(100) }) }) @@ -331,7 +331,7 @@ test('Timeout with pipelining', async (t) => { res.end('hello') }, 100) clock.tick(50) - fastTimerTick(50) + fastTimersTick(50) }) after(() => server.close()) @@ -372,7 +372,7 @@ test('Global option', async (t) => { res.end('hello') }, 100) clock.tick(100) - fastTimerTick(100) + fastTimersTick(100) }) after(() => server.close()) @@ -387,7 +387,7 @@ test('Global option', async (t) => { }) clock.tick(50) - fastTimerTick(50) + fastTimersTick(50) }) await t.completed @@ -407,7 +407,7 @@ test('Request options overrides global option', async (t) => { res.end('hello') }, 100) clock.tick(100) - fastTimerTick(100) + fastTimersTick(100) }) after(() => server.close()) @@ -422,7 +422,7 @@ test('Request options overrides global option', async (t) => { }) clock.tick(50) - fastTimerTick(50) + fastTimersTick(50) }) await t.completed @@ -483,7 +483,7 @@ test('client.close should wait for the timeout', async (t) => { client.on('connect', () => { process.nextTick(() => { clock.tick(100) - fastTimerTick(100) + fastTimersTick(100) }) }) }) @@ -547,7 +547,7 @@ test('Disable request timeout', async (t) => { res.end('hello') }, 32e3) clock.tick(33e3) - fastTimerTick(33e3) + fastTimersTick(33e3) }) after(() => server.close()) @@ -570,7 +570,7 @@ test('Disable request timeout', async (t) => { }) clock.tick(31e3) - fastTimerTick(31e3) + fastTimersTick(31e3) }) await t.completed @@ -590,7 +590,7 @@ test('Disable request timeout for a single request', async (t) => { res.end('hello') }, 32e3) clock.tick(33e3) - fastTimerTick(33e3) + fastTimersTick(33e3) }) after(() => server.close()) @@ -613,7 +613,7 @@ test('Disable request timeout for a single request', async (t) => { }) clock.tick(31e3) - fastTimerTick(31e3) + fastTimersTick(31e3) }) await t.completed @@ -633,7 +633,7 @@ test('stream timeout', async (t) => { res.end('hello') }, 301e3) clock.tick(301e3) - fastTimerTick(301e3) + fastTimersTick(301e3) }) after(() => server.close()) @@ -669,7 +669,7 @@ test('stream custom timeout', async (t) => { res.end('hello') }, 31e3) clock.tick(31e3) - fastTimerTick(31e3) + fastTimersTick(31e3) }) after(() => server.close()) @@ -707,7 +707,7 @@ test('pipeline timeout', async (t) => { req.pipe(res) }, 301e3) clock.tick(301e3) - fastTimerTick(301e3) + fastTimersTick(301e3) }) after(() => server.close()) @@ -762,7 +762,7 @@ test('pipeline timeout', async (t) => { req.pipe(res) }, 31e3) clock.tick(31e3) - fastTimerTick(31e3) + fastTimersTick(31e3) }) after(() => server.close()) @@ -838,7 +838,7 @@ test('client.close should not deadlock', async (t) => { }) clock.tick(100) - fastTimerTick(100) + fastTimersTick(100) }) }) await t.completed From bbf31bc5c92d49f9d513bbcbd844da79e75024f9 Mon Sep 17 00:00:00 2001 From: uzlopak Date: Fri, 6 Sep 2024 13:02:32 +0200 Subject: [PATCH 10/14] more informative connection error --- lib/core/connect.js | 59 +++++++++++++++++++++++++++++++++------------ 1 file changed, 43 insertions(+), 16 deletions(-) diff --git a/lib/core/connect.js b/lib/core/connect.js index 11b0151fffb..403ce1550dd 100644 --- a/lib/core/connect.js +++ b/lib/core/connect.js @@ -6,6 +6,8 @@ const util = require('./util') const { InvalidArgumentError, ConnectTimeoutError } = require('./errors') const timers = require('../util/timers') +function noop () {} + let tls // include tls conditionally since it is not always available // TODO: session re-use does not wait for the first @@ -96,6 +98,8 @@ function buildConnector ({ allowH2, maxCachedSessions, socketPath, timeout, sess const session = customSession || sessionCache.get(sessionKey) || null + port = port || 443 + socket = tls.connect({ highWaterMark: 16384, // TLS in node can't have bigger HWM anyway... ...options, @@ -105,7 +109,7 @@ function buildConnector ({ allowH2, maxCachedSessions, socketPath, timeout, sess // TODO(HTTP/2): Add support for h2c ALPNProtocols: allowH2 ? ['http/1.1', 'h2'] : ['http/1.1'], socket: httpSocket, // upgrade socket connection - port: port || 443, + port, host: hostname }) @@ -116,11 +120,14 @@ function buildConnector ({ allowH2, maxCachedSessions, socketPath, timeout, sess }) } else { assert(!httpSocket, 'httpSocket can only be sent on TLS update') + + port = port || 80 + socket = net.connect({ highWaterMark: 64 * 1024, // Same as nodejs fs streams. ...options, localAddress, - port: port || 80, + port, host: hostname }) } @@ -131,7 +138,7 @@ function buildConnector ({ allowH2, maxCachedSessions, socketPath, timeout, sess socket.setKeepAlive(true, keepAliveInitialDelay) } - const cancelConnectTimeout = setupConnectTimeout(new WeakRef(socket), timeout) + const cancelConnectTimeout = setupConnectTimeout(new WeakRef(socket), { timeout, hostname, port }) socket .setNoDelay(true) @@ -158,10 +165,18 @@ function buildConnector ({ allowH2, maxCachedSessions, socketPath, timeout, sess } } +/** + * @param {net.Socket} socket + * @param {object} opts + * @param {number} opts.timeout + * @param {string} opts.hostname + * @param {number} opts.port + * @returns {() => void} + */ const setupConnectTimeout = process.platform === 'win32' - ? (socket, timeout) => { - if (!timeout) { - return () => { } + ? (socket, opts) => { + if (!opts.timeout) { + return noop } let s1 = null @@ -170,38 +185,50 @@ const setupConnectTimeout = process.platform === 'win32' // setImmediate is added to make sure that we prioritize socket error events over timeouts s1 = setImmediate(() => { // Windows needs an extra setImmediate probably due to implementation differences in the socket logic - s2 = setImmediate(() => onConnectTimeout(socket.deref())) + s2 = setImmediate(() => onConnectTimeout(socket.deref()), opts) }) - }, timeout) + }, opts.timeout) return () => { timers.clearFastTimeout(fastTimer) clearImmediate(s1) clearImmediate(s2) } } - : (socket, timeout) => { - if (!timeout) { - return () => { } + : (socket, opts) => { + if (!opts.timeout) { + return noop } let s1 = null const fastTimer = timers.setFastTimeout(() => { // setImmediate is added to make sure that we prioritize socket error events over timeouts s1 = setImmediate(() => { - onConnectTimeout(socket.deref()) + onConnectTimeout(socket.deref(), opts) }) - }, timeout) + }, opts.timeout) return () => { timers.clearFastTimeout(fastTimer) clearImmediate(s1) } } -function onConnectTimeout (socket) { +/** + * @param {net.Socket} socket + * @param {object} opts + * @param {number} opts.timeout + * @param {string} opts.hostname + * @param {number} opts.port + */ +function onConnectTimeout (socket, opts) { let message = 'Connect Timeout Error' - if (Array.isArray(socket.autoSelectFamilyAttemptedAddresses)) { - message += ` (attempted addresses: ${socket.autoSelectFamilyAttemptedAddresses.join(', ')})` + if (socket.autoSelectFamilyAttemptedAddresses) { + message += ` (attempted addresses: ${socket.autoSelectFamilyAttemptedAddresses.join(', ')}, ` + } else { + message += ` (attempted address: ${opts.hostname}:${opts.port}, ` } + + message += ` timeout: ${opts.timeout}ms)` + util.destroy(socket, new ConnectTimeoutError(message)) } From 454070ebce33d0fd4fcdf68e0115d55782c876c1 Mon Sep 17 00:00:00 2001 From: uzlopak Date: Fri, 6 Sep 2024 13:13:04 +0200 Subject: [PATCH 11/14] ignore request-timeout binary file, rename clearAll to reset --- .gitignore | 3 +++ .npmignore | 3 +++ lib/util/timers.js | 4 ++-- test/request-timeout.js | 7 ++++--- 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/.gitignore b/.gitignore index 60aa663c838..7cba7df889f 100644 --- a/.gitignore +++ b/.gitignore @@ -84,3 +84,6 @@ undici-fetch.js .npmrc .tap + +# File generated by /test/request-timeout.js +test/request-timeout.10mb.bin diff --git a/.npmignore b/.npmignore index 879c6669f03..461d334ef36 100644 --- a/.npmignore +++ b/.npmignore @@ -11,3 +11,6 @@ lib/llhttp/llhttp.wasm !index.d.ts !docs/docs/**/* !scripts/strip-comments.js + +# File generated by /test/request-timeout.js +test/request-timeout.10mb.bin diff --git a/lib/util/timers.js b/lib/util/timers.js index 9bb1c0ec838..36b6bbf985e 100644 --- a/lib/util/timers.js +++ b/lib/util/timers.js @@ -406,12 +406,12 @@ module.exports = { onTick() }, /** - * Clear all FastTimers. + * Reset FastTimers. * Exported for testing purposes only. * Marking as deprecated to discourage any use outside of testing. * @deprecated */ - clearAll () { + reset () { fastNow = 0 fastTimers.length = 0 clearTimeout(fastNowTimeout) diff --git a/test/request-timeout.js b/test/request-timeout.js index 0f7618c75a0..dfe0de1967c 100644 --- a/test/request-timeout.js +++ b/test/request-timeout.js @@ -1,6 +1,7 @@ 'use strict' const { tspl } = require('@matteo.collina/tspl') +const { resolve: pathResolve } = require('node:path') const { test, after, beforeEach } = require('node:test') const { createReadStream, writeFileSync, unlinkSync } = require('node:fs') const { Client, errors } = require('..') @@ -17,11 +18,11 @@ const { } = require('node:stream') const { tick: fastTimersTick, - clearAll: clearAllFastTimers + reset: resetFastTimers } = require('../lib/util/timers') beforeEach(() => { - clearAllFastTimers() + resetFastTimers() }) test('request timeout', async (t) => { @@ -53,7 +54,7 @@ test('request timeout with readable body', async (t) => { }) after(() => server.close()) - const tempfile = `${__filename}.10mb.txt` + const tempfile = pathResolve(__dirname, 'request-timeout.10mb.bin') writeFileSync(tempfile, Buffer.alloc(10 * 1024 * 1024)) after(() => unlinkSync(tempfile)) From 53160091e0e4d58693d5cb676ebb019f684f281b Mon Sep 17 00:00:00 2001 From: uzlopak Date: Fri, 6 Sep 2024 13:21:22 +0200 Subject: [PATCH 12/14] fix --- lib/core/connect.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/core/connect.js b/lib/core/connect.js index 403ce1550dd..869e4ea5c5b 100644 --- a/lib/core/connect.js +++ b/lib/core/connect.js @@ -185,13 +185,13 @@ const setupConnectTimeout = process.platform === 'win32' // setImmediate is added to make sure that we prioritize socket error events over timeouts s1 = setImmediate(() => { // Windows needs an extra setImmediate probably due to implementation differences in the socket logic - s2 = setImmediate(() => onConnectTimeout(socket.deref()), opts) + s2 = setImmediate(() => onConnectTimeout(socket.deref(), opts)) }) }, opts.timeout) return () => { - timers.clearFastTimeout(fastTimer) clearImmediate(s1) clearImmediate(s2) + setImmediate(timers.clearFastTimeout, fastTimer) } } : (socket, opts) => { @@ -207,8 +207,8 @@ const setupConnectTimeout = process.platform === 'win32' }) }, opts.timeout) return () => { - timers.clearFastTimeout(fastTimer) clearImmediate(s1) + setImmediate(timers.clearFastTimeout, fastTimer) } } From 387a750edc7ae8ccb69c347adbc2479af04826d6 Mon Sep 17 00:00:00 2001 From: uzlopak Date: Fri, 6 Sep 2024 13:46:05 +0200 Subject: [PATCH 13/14] add test --- lib/core/connect.js | 16 ++++++++-------- test/connect-timeout.js | 11 ++++++++--- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/lib/core/connect.js b/lib/core/connect.js index 869e4ea5c5b..663b35da450 100644 --- a/lib/core/connect.js +++ b/lib/core/connect.js @@ -166,7 +166,7 @@ function buildConnector ({ allowH2, maxCachedSessions, socketPath, timeout, sess } /** - * @param {net.Socket} socket + * @param {WeakRef} socketWeakRef * @param {object} opts * @param {number} opts.timeout * @param {string} opts.hostname @@ -174,7 +174,7 @@ function buildConnector ({ allowH2, maxCachedSessions, socketPath, timeout, sess * @returns {() => void} */ const setupConnectTimeout = process.platform === 'win32' - ? (socket, opts) => { + ? (socketWeakRef, opts) => { if (!opts.timeout) { return noop } @@ -185,7 +185,7 @@ const setupConnectTimeout = process.platform === 'win32' // setImmediate is added to make sure that we prioritize socket error events over timeouts s1 = setImmediate(() => { // Windows needs an extra setImmediate probably due to implementation differences in the socket logic - s2 = setImmediate(() => onConnectTimeout(socket.deref(), opts)) + s2 = setImmediate(() => onConnectTimeout(socketWeakRef.deref(), opts)) }) }, opts.timeout) return () => { @@ -194,7 +194,7 @@ const setupConnectTimeout = process.platform === 'win32' setImmediate(timers.clearFastTimeout, fastTimer) } } - : (socket, opts) => { + : (socketWeakRef, opts) => { if (!opts.timeout) { return noop } @@ -203,7 +203,7 @@ const setupConnectTimeout = process.platform === 'win32' const fastTimer = timers.setFastTimeout(() => { // setImmediate is added to make sure that we prioritize socket error events over timeouts s1 = setImmediate(() => { - onConnectTimeout(socket.deref(), opts) + onConnectTimeout(socketWeakRef.deref(), opts) }) }, opts.timeout) return () => { @@ -221,10 +221,10 @@ const setupConnectTimeout = process.platform === 'win32' */ function onConnectTimeout (socket, opts) { let message = 'Connect Timeout Error' - if (socket.autoSelectFamilyAttemptedAddresses) { - message += ` (attempted addresses: ${socket.autoSelectFamilyAttemptedAddresses.join(', ')}, ` + if (Array.isArray(socket.autoSelectFamilyAttemptedAddresses)) { + message += ` (attempted addresses: ${socket.autoSelectFamilyAttemptedAddresses.join(', ')},` } else { - message += ` (attempted address: ${opts.hostname}:${opts.port}, ` + message += ` (attempted address: ${opts.hostname}:${opts.port},` } message += ` timeout: ${opts.timeout}ms)` diff --git a/test/connect-timeout.js b/test/connect-timeout.js index ff50eb777a6..186067f80ac 100644 --- a/test/connect-timeout.js +++ b/test/connect-timeout.js @@ -10,12 +10,13 @@ const skip = !!process.env.CITGM // Using describe instead of test to avoid the timeout describe('prioritize socket errors over timeouts', { skip }, async () => { - const t = tspl({ ...assert, after: () => {} }, { plan: 1 }) + const t = tspl({ ...assert, after: () => {} }, { plan: 2 }) const client = new Pool('http://foorbar.invalid:1234', { connectTimeout: 1 }) client.request({ method: 'GET', path: '/foobar' }) .then(() => t.fail()) .catch((err) => { + t.strictEqual(err.code, 'ENOTFOUND') t.strictEqual(err.code !== 'UND_ERR_CONNECT_TIMEOUT', true) }) @@ -32,7 +33,7 @@ net.connect = function (options) { } test('connect-timeout', { skip }, async t => { - t = tspl(t, { plan: 1 }) + t = tspl(t, { plan: 3 }) const client = new Client('http://localhost:9000', { connectTimeout: 1e3 @@ -48,6 +49,8 @@ test('connect-timeout', { skip }, async t => { method: 'GET' }, (err) => { t.ok(err instanceof errors.ConnectTimeoutError) + t.strictEqual(err.code, 'UND_ERR_CONNECT_TIMEOUT') + t.strictEqual(err.message, 'Connect Timeout Error (attempted address: localhost:9000, timeout: 1000ms)') clearTimeout(timeout) }) @@ -55,7 +58,7 @@ test('connect-timeout', { skip }, async t => { }) test('connect-timeout', { skip }, async t => { - t = tspl(t, { plan: 1 }) + t = tspl(t, { plan: 3 }) const client = new Pool('http://localhost:9000', { connectTimeout: 1e3 @@ -71,6 +74,8 @@ test('connect-timeout', { skip }, async t => { method: 'GET' }, (err) => { t.ok(err instanceof errors.ConnectTimeoutError) + t.strictEqual(err.code, 'UND_ERR_CONNECT_TIMEOUT') + t.strictEqual(err.message, 'Connect Timeout Error (attempted address: localhost:9000, timeout: 1000ms)') clearTimeout(timeout) }) From 75ddb34d1ee791185562512e4ad21343895227f5 Mon Sep 17 00:00:00 2001 From: uzlopak Date: Fri, 6 Sep 2024 17:15:35 +0200 Subject: [PATCH 14/14] use queueMicrotask earlier in the socket callbacks --- lib/core/connect.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/core/connect.js b/lib/core/connect.js index 663b35da450..8ab21fcd5fc 100644 --- a/lib/core/connect.js +++ b/lib/core/connect.js @@ -138,12 +138,12 @@ function buildConnector ({ allowH2, maxCachedSessions, socketPath, timeout, sess socket.setKeepAlive(true, keepAliveInitialDelay) } - const cancelConnectTimeout = setupConnectTimeout(new WeakRef(socket), { timeout, hostname, port }) + const clearConnectTimeout = setupConnectTimeout(new WeakRef(socket), { timeout, hostname, port }) socket .setNoDelay(true) .once(protocol === 'https:' ? 'secureConnect' : 'connect', function () { - cancelConnectTimeout() + queueMicrotask(clearConnectTimeout) if (callback) { const cb = callback @@ -152,7 +152,7 @@ function buildConnector ({ allowH2, maxCachedSessions, socketPath, timeout, sess } }) .on('error', function (err) { - cancelConnectTimeout() + queueMicrotask(clearConnectTimeout) if (callback) { const cb = callback @@ -189,9 +189,9 @@ const setupConnectTimeout = process.platform === 'win32' }) }, opts.timeout) return () => { + timers.clearFastTimeout(fastTimer) clearImmediate(s1) clearImmediate(s2) - setImmediate(timers.clearFastTimeout, fastTimer) } } : (socketWeakRef, opts) => { @@ -207,8 +207,8 @@ const setupConnectTimeout = process.platform === 'win32' }) }, opts.timeout) return () => { + timers.clearFastTimeout(fastTimer) clearImmediate(s1) - setImmediate(timers.clearFastTimeout, fastTimer) } }