diff --git a/lib/core/connect.js b/lib/core/connect.js index 57667a1314a..e9b456f8831 100644 --- a/lib/core/connect.js +++ b/lib/core/connect.js @@ -75,14 +75,12 @@ function buildConnector ({ maxCachedSessions, socketPath, timeout, ...opts }) { }) } - const timeoutId = timeout - ? setTimeout(onConnectTimeout, timeout, socket) - : null + const cancelTimeout = setupTimeout(() => onConnectTimeout(socket), timeout) socket .setNoDelay(true) .once(protocol === 'https:' ? 'secureConnect' : 'connect', function () { - clearTimeout(timeoutId) + cancelTimeout() if (callback) { const cb = callback @@ -91,7 +89,7 @@ function buildConnector ({ maxCachedSessions, socketPath, timeout, ...opts }) { } }) .on('error', function (err) { - clearTimeout(timeoutId) + cancelTimeout() if (callback) { const cb = callback @@ -104,6 +102,31 @@ function buildConnector ({ maxCachedSessions, socketPath, timeout, ...opts }) { } } +function setupTimeout (onConnectTimeout, timeout) { + if (!timeout) { + return () => {} + } + + let s1 = null + let s2 = null + const timeoutId = setTimeout(() => { + // setImmediate is added to make sure that we priotorise socket error events over timeouts + s1 = setImmediate(() => { + if (process.platform === 'win32') { + // Windows needs an extra setImmediate probably due to implementation differences in the socket logic + s2 = setImmediate(() => onConnectTimeout()) + } else { + onConnectTimeout() + } + }) + }, timeout) + return () => { + clearTimeout(timeoutId) + clearImmediate(s1) + clearImmediate(s2) + } +} + function onConnectTimeout (socket) { util.destroy(socket, new ConnectTimeoutError()) } diff --git a/package.json b/package.json index 1fde040055f..42d6d530103 100644 --- a/package.json +++ b/package.json @@ -67,6 +67,7 @@ "@sinonjs/fake-timers": "^9.1.2", "@types/node": "^17.0.29", "abort-controller": "^3.0.0", + "atomic-sleep": "^1.0.0", "busboy": "^1.6.0", "chai": "^4.3.4", "chai-as-promised": "^7.1.1", diff --git a/test/connect-timeout.js b/test/connect-timeout.js index eb6f02dbb1b..b2af6a3965d 100644 --- a/test/connect-timeout.js +++ b/test/connect-timeout.js @@ -3,8 +3,24 @@ const { test } = require('tap') const { Client, Pool, errors } = require('..') const net = require('net') +const sleep = require('atomic-sleep') -// Never connect +test('priotorise socket errors over timeouts', (t) => { + t.plan(1) + const connectTimeout = 1000 + const client = new Pool('http://foobar.bar:1234', { connectTimeout }) + + client.request({ method: 'GET', path: '/foobar' }) + .then(() => t.fail()) + .catch((err) => { + t.equal(err.code, 'ENOTFOUND') + }) + + // block for 1001ms which is enough for the dns lookup to complete and TO to fire + sleep(connectTimeout + 1) +}) + +// never connect net.connect = function (options) { return new net.Socket(options) }