From efc5e83c6e0c5e726bd03b73d705ff4fc9e2df00 Mon Sep 17 00:00:00 2001 From: uzlopak Date: Fri, 23 Aug 2024 22:25:09 +0200 Subject: [PATCH] make the tests pass --- lib/core/connect.js | 65 +++++++++++++++++++++++-------------- lib/dispatcher/client-h1.js | 12 +++++++ lib/util/timers.js | 18 +++++++++- test/socket-timeout.js | 7 ---- 4 files changed, 69 insertions(+), 33 deletions(-) diff --git a/lib/core/connect.js b/lib/core/connect.js index b388f022298..2d955489e8f 100644 --- a/lib/core/connect.js +++ b/lib/core/connect.js @@ -4,6 +4,7 @@ const net = require('node:net') const assert = require('node:assert') const util = require('./util') const { InvalidArgumentError, ConnectTimeoutError } = require('./errors') +const timers = require('../util/timers') let tls // include tls conditionally since it is not always available @@ -130,12 +131,12 @@ function buildConnector ({ allowH2, maxCachedSessions, socketPath, timeout, sess socket.setKeepAlive(true, keepAliveInitialDelay) } - const cancelTimeout = setupTimeout(() => onConnectTimeout(socket), timeout) + const cancelConnectTimeout = setupConnectTimeout(new WeakRef(socket), timeout) socket .setNoDelay(true) .once(protocol === 'https:' ? 'secureConnect' : 'connect', function () { - cancelTimeout() + cancelConnectTimeout() if (callback) { const cb = callback @@ -144,7 +145,7 @@ function buildConnector ({ allowH2, maxCachedSessions, socketPath, timeout, sess } }) .on('error', function (err) { - cancelTimeout() + cancelConnectTimeout() if (callback) { const cb = callback @@ -157,35 +158,49 @@ function buildConnector ({ allowH2, maxCachedSessions, socketPath, timeout, sess } } -function setupTimeout (onConnectTimeout, timeout) { - if (!timeout) { - return () => {} - } +const setupConnectTimeout = process.platform === 'win32' + ? (socket, timeout) => { + if (!timeout) { + return () => { } + } - let s1 = null - let s2 = null - const timeoutId = setTimeout(() => { - // setImmediate is added to make sure that we prioritize socket error events over timeouts - s1 = setImmediate(() => { - if (process.platform === 'win32') { + let s1 = null + let s2 = null + const timer = timers.setTimeout(() => { + // 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()) - } else { - onConnectTimeout() + s2 = setImmediate(() => onConnectTimeout(socket.deref())) + }) + }, timeout) + return () => { + timers.clearTimeout(timer) + clearImmediate(s1) + clearImmediate(s2) } - }) - }, timeout) - return () => { - clearTimeout(timeoutId) - clearImmediate(s1) - clearImmediate(s2) - } -} + } + : (socket, timeout) => { + if (!timeout) { + return () => { } + } + + let s1 = null + const timer = timers.setTimeout(() => { + // setImmediate is added to make sure that we prioritize socket error events over timeouts + s1 = setImmediate(() => { + onConnectTimeout(socket.deref()) + }) + }, timeout) + return () => { + timers.clearTimeout(timer) + clearImmediate(s1) + } + } function onConnectTimeout (socket) { let message = 'Connect Timeout Error' if (Array.isArray(socket.autoSelectFamilyAttemptedAddresses)) { - message += ` (attempted addresses: ${socket.autoSelectFamilyAttemptedAddresses.join(', ')})` + message += ` (attempted addresses: ${socket.deref().autoSelectFamilyAttemptedAddresses.join(', ')})` } util.destroy(socket, new ConnectTimeoutError(message)) } diff --git a/lib/dispatcher/client-h1.js b/lib/dispatcher/client-h1.js index 6d024402a5f..af3af17841e 100644 --- a/lib/dispatcher/client-h1.js +++ b/lib/dispatcher/client-h1.js @@ -156,6 +156,7 @@ class Parser { this.bytesRead = 0 + this.timeoutDeadline = 0 this.keepAlive = '' this.contentLength = '' this.connection = '' @@ -168,6 +169,7 @@ class Parser { this.timeout && timers.clearTimeout(this.timeout) if (delay) { this.timeout = timers.setTimeout(onParserTimeout, delay, this) + this.timeoutDeadline = this.timeoutValue ? timers.now() + this.timeoutValue : 0 // istanbul ignore else: only for jest if (this.timeout.unref) { this.timeout.unref() @@ -184,6 +186,12 @@ class Parser { } } + updateTimeout () { + if (this.timeoutDeadline && this.timeoutDeadline < timers.now()) { + onParserTimeout(this) + } + } + resume () { if (this.socket.destroyed || !this.paused) { return @@ -613,6 +621,8 @@ class Parser { function onParserTimeout (parser) { const { socket, timeoutType, client } = parser + parser.timeoutDeadline = 0 + /* istanbul ignore else */ if (timeoutType === TIMEOUT_HEADERS) { if (!socket[kWriting] || socket.writableNeedDrain || client[kRunning] > 1) { @@ -813,6 +823,8 @@ function resumeH1 (client) { socket[kParser].setTimeout(headersTimeout, TIMEOUT_HEADERS) } } + + socket[kParser].updateTimeout() } } diff --git a/lib/util/timers.js b/lib/util/timers.js index ccc679bb89d..18ec4496c2b 100644 --- a/lib/util/timers.js +++ b/lib/util/timers.js @@ -14,6 +14,14 @@ const nativeSetTimeout = global.setTimeout const nativeClearTimeout = global.clearTimeout +/** + * The fastNow variable is used to store the current time in milliseconds + * since the process started. + * + * @type {number} + */ +let fastNow = 0 + /** * The RESOLUTION_MS is the desired time in milliseconds. * @@ -115,7 +123,7 @@ function onTick () { * * @type {number} */ - const fastNow = Math.trunc(performance.now()) + fastNow = Math.trunc(performance.now()) /** * The idx variable is used to iterate over the fastTimers array. @@ -358,6 +366,14 @@ module.exports = { nativeClearTimeout(timeout) } }, + /** + * The now method returns the value of the cached performance.now() value. + * + * @returns {number} + */ + now () { + return fastNow + }, /** * Exporting for testing purposes only. * Marking as deprecated to discourage any use outside of testing. diff --git a/test/socket-timeout.js b/test/socket-timeout.js index 5be62fe4119..43ee1576615 100644 --- a/test/socket-timeout.js +++ b/test/socket-timeout.js @@ -3,7 +3,6 @@ const { tspl } = require('@matteo.collina/tspl') const { test, after } = require('node:test') const { Client, errors } = require('..') -const timers = require('../lib/util/timers') const { createServer } = require('node:http') const FakeTimers = require('@sinonjs/fake-timers') @@ -68,12 +67,6 @@ test('Disable socket timeout', async (t) => { const clock = FakeTimers.install() after(clock.uninstall.bind(clock)) - const orgTimers = { ...timers } - Object.assign(timers, { setTimeout, clearTimeout }) - after(() => { - Object.assign(timers, orgTimers) - }) - server.once('request', (req, res) => { setTimeout(() => { res.end('hello')