diff --git a/lib/adapters/http.js b/lib/adapters/http.js index 754a15c1..057ae811 100755 --- a/lib/adapters/http.js +++ b/lib/adapters/http.js @@ -390,6 +390,7 @@ export default isHttpAdapterSupported && let isDone; let rejected = false; let req; + let connectPhaseTimer; httpVersion = +httpVersion; @@ -434,9 +435,34 @@ export default isHttpAdapterSupported && } } + function clearConnectPhaseTimer() { + if (connectPhaseTimer) { + clearTimeout(connectPhaseTimer); + connectPhaseTimer = null; + } + } + + function createTimeoutError() { + let timeoutErrorMessage = config.timeout + ? 'timeout of ' + config.timeout + 'ms exceeded' + : 'timeout exceeded'; + const transitional = config.transitional || transitionalDefaults; + if (config.timeoutErrorMessage) { + timeoutErrorMessage = config.timeoutErrorMessage; + } + return new AxiosError( + timeoutErrorMessage, + transitional.clarifyTimeoutError ? AxiosError.ETIMEDOUT : AxiosError.ECONNABORTED, + config, + req + ); + } + abortEmitter.once('abort', reject); const onFinished = () => { + clearConnectPhaseTimer(); + if (config.cancelToken) { config.cancelToken.unsubscribe(abort); } @@ -457,6 +483,7 @@ export default isHttpAdapterSupported && onDone((response, isRejected) => { isDone = true; + clearConnectPhaseTimer(); if (isRejected) { rejected = true; @@ -755,6 +782,7 @@ export default isHttpAdapterSupported && ); } let transport; + let isNativeTransport = false; const isHttpsRequest = isHttps.test(options.protocol); options.agent = isHttpsRequest ? config.httpsAgent : config.httpAgent; @@ -766,6 +794,7 @@ export default isHttpAdapterSupported && transport = configTransport; } else if (config.maxRedirects === 0) { transport = isHttpsRequest ? https : http; + isNativeTransport = true; } else { if (config.maxRedirects) { options.maxRedirects = config.maxRedirects; @@ -792,6 +821,8 @@ export default isHttpAdapterSupported && // Create the request req = transport.request(options, function handleResponse(res) { + clearConnectPhaseTimer(); + if (req.destroyed) return; const streams = [res]; @@ -1017,6 +1048,8 @@ export default isHttpAdapterSupported && }); req.once('close', function clearCurrentReq() { + clearConnectPhaseTimer(); + for (const socket of boundSockets) { if (socket[kAxiosCurrentReq] === req) { socket[kAxiosCurrentReq] = null; @@ -1043,29 +1076,24 @@ export default isHttpAdapterSupported && return; } + const handleTimeout = function handleTimeout() { + if (isDone) return; + abort(createTimeoutError()); + }; + + if (isNativeTransport && timeout > 0) { + // Native ClientRequest#setTimeout starts from the socket lifecycle and + // may not fire while TCP connect is still pending. Mirror the + // follow-redirects wall-clock timer for the maxRedirects === 0 path. + connectPhaseTimer = setTimeout(handleTimeout, timeout); + } + // Sometime, the response will be very slow, and does not respond, the connect event will be block by event loop system. // And timer callback will be fired, and abort() will be invoked before connection, then get "socket hang up" and code ECONNRESET. // At this time, if we have a large number of request, nodejs will hang up some socket on background. and the number will up and up. // And then these socket which be hang up will devouring CPU little by little. // ClientRequest.setTimeout will be fired on the specify milliseconds, and can make sure that abort() will be fired after connect. - req.setTimeout(timeout, function handleRequestTimeout() { - if (isDone) return; - let timeoutErrorMessage = config.timeout - ? 'timeout of ' + config.timeout + 'ms exceeded' - : 'timeout exceeded'; - const transitional = config.transitional || transitionalDefaults; - if (config.timeoutErrorMessage) { - timeoutErrorMessage = config.timeoutErrorMessage; - } - abort( - new AxiosError( - timeoutErrorMessage, - transitional.clarifyTimeoutError ? AxiosError.ETIMEDOUT : AxiosError.ECONNABORTED, - config, - req - ) - ); - }); + req.setTimeout(timeout, handleTimeout); } else { // explicitly reset the socket timeout value for a possible `keep-alive` request req.setTimeout(0); diff --git a/tests/unit/adapters/http.test.js b/tests/unit/adapters/http.test.js index d93c551d..1041f452 100644 --- a/tests/unit/adapters/http.test.js +++ b/tests/unit/adapters/http.test.js @@ -52,6 +52,37 @@ describe('supports http with nodejs', () => { }; } + class HangingConnectSocket extends stream.Duplex { + constructor() { + super(); + this.connecting = true; + } + + _read() {} + + _write(_chunk, _encoding, callback) { + callback(); + } + + setKeepAlive() { + return this; + } + + setNoDelay() { + return this; + } + + setTimeout() { + return this; + } + } + + class HangingConnectAgent extends http.Agent { + createConnection() { + return new HangingConnectSocket(); + } + } + it('should support IPv4 literal strings', async () => { const data = { firstName: 'Fred', @@ -206,6 +237,69 @@ describe('supports http with nodejs', () => { } }); + it('should respect the timeout property during TCP connect with maxRedirects set to 0', async () => { + const timeout = 100; + const guardTimeout = 1000; + const started = Date.now(); + const controller = new AbortController(); + const agent = new HangingConnectAgent(); + let guardTimer; + const request = axios.get('http://connect-timeout.test/', { + httpAgent: agent, + maxRedirects: 0, + proxy: false, + signal: controller.signal, + timeout, + }); + const guard = new Promise((_resolve, reject) => { + guardTimer = setTimeout(() => { + controller.abort(); + reject(new Error('request did not honor timeout during connect')); + }, guardTimeout); + }); + + try { + await assert.rejects(Promise.race([request, guard]), (error) => { + const elapsed = Date.now() - started; + assert.strictEqual(error.code, 'ECONNABORTED'); + assert.strictEqual(error.message, `timeout of ${timeout}ms exceeded`); + assert.ok(elapsed < guardTimeout, `request timed out after ${elapsed}ms`); + return true; + }); + } finally { + clearTimeout(guardTimer); + controller.abort(); + agent.destroy(); + } + }); + + it('should not time out immediately for timeout set to zero during TCP connect', async () => { + const controller = new AbortController(); + const agent = new HangingConnectAgent(); + const request = axios + .get('http://connect-timeout.test/', { + httpAgent: agent, + maxRedirects: 0, + proxy: false, + signal: controller.signal, + timeout: '0', + }) + .then( + () => null, + (error) => error + ); + + try { + await setTimeoutAsync(50); + controller.abort(); + const error = await request; + assert.strictEqual(error.code, AxiosError.ERR_CANCELED); + } finally { + controller.abort(); + agent.destroy(); + } + }); + it('should respect the timeoutErrorMessage property', async () => { const server = await startHTTPServer( (req, res) => {