From 003953cce27765d1a6291507a42f02b808021b8b Mon Sep 17 00:00:00 2001 From: hyj1991 Date: Tue, 5 Mar 2019 16:10:20 +0800 Subject: [PATCH] distinguish between read timeout and connect timeout --- lib/index.js | 93 ++++++++++++++++++++++++++++++++++++---------- test/index.test.js | 52 +++++++++++++++++++++++++- 2 files changed, 124 insertions(+), 21 deletions(-) diff --git a/lib/index.js b/lib/index.js index 6f0c27c..c615400 100644 --- a/lib/index.js +++ b/lib/index.js @@ -14,19 +14,35 @@ const httpsAgent = new https.Agent({ keepAlive: true }); const TIMEOUT = 3000; // 3s +const READ_TIMER = Symbol('TIMER::READ_TIMER'); +const READ_TIME_OUT = Symbol('TIMER::READ_TIME_OUT'); + var append = function (err, name, message) { err.name = name + err.name; err.message = `${message}. ${err.message}`; return err; }; +const isNumber = function (num) { + return num !== null && !isNaN(num); +}; + exports.request = function (url, opts) { // request(url) opts || (opts = {}); const parsed = typeof url === 'string' ? parse(url) : url; - const timeout = opts.timeout || TIMEOUT; + let readTimeout, connectTimeout; + if (isNumber(opts.readTimeout) || isNumber(opts.connectTimeout)) { + readTimeout = isNumber(opts.readTimeout) ? Number(opts.readTimeout) : TIMEOUT; + connectTimeout = isNumber(opts.connectTimeout) ? Number(opts.connectTimeout) : TIMEOUT; + } else if (isNumber(opts.timeout)) { + readTimeout = connectTimeout = Number(opts.timeout); + } else { + readTimeout = connectTimeout = TIMEOUT; + } + const isHttps = parsed.protocol === 'https:'; const method = (opts.method || 'GET').toUpperCase(); const defaultAgent = isHttps ? httpsAgent : httpAgent; @@ -39,6 +55,8 @@ exports.request = function (url, opts) { port: parsed.port || (parsed.protocol === 'https:' ? 443 : 80), agent: agent, headers: opts.headers || {}, + // connect timerout + timeout: connectTimeout }; if (isHttps && typeof opts.rejectUnauthorized !== 'undefined') { @@ -59,17 +77,7 @@ exports.request = function (url, opts) { const request = httplib.request(options); const body = opts.data; - var timer; - - var cleanup = () => { - if (timer) { - clearTimeout(timer); - timer = null; - } - }; - var fulfilled = (response) => { - cleanup(); if (debugHeader.enabled) { const requestHeaders = response.req._header; requestHeaders.split('\r\n').forEach((line) => { @@ -85,8 +93,11 @@ exports.request = function (url, opts) { }; var rejected = (err) => { - cleanup(); err.message += `${method} ${format(parsed)} failed.`; + // clear response timer when error + if (request.socket[READ_TIMER]) { + clearTimeout(request.socket[READ_TIMER]); + } reject(err); }; @@ -95,6 +106,19 @@ exports.request = function (url, opts) { rejected(err); }; + const startResponseTimer = function (socket) { + const timer = setTimeout(() => { + socket[READ_TIMER] = null; + var err = new Error(); + var message = `ReadTimeout(${readTimeout})`; + abort(append(err, 'RequestTimeout', message)); + }, readTimeout); + timer.startTime = Date.now(); + // start read-timer + socket[READ_TIME_OUT] = readTimeout; + socket[READ_TIMER] = timer; + }; + // string if (!body || 'string' === typeof body || body instanceof Buffer) { request.end(body); @@ -107,13 +131,16 @@ exports.request = function (url, opts) { request.on('response', fulfilled); request.on('error', rejected); - // for timeout - timer = setTimeout(() => { - timer = null; - var err = new Error(); - var message = `Timeout(${timeout})`; - abort(append(err, 'RequestTimeout', message)); - }, timeout); + request.once('socket', function (socket) { + // reuse socket + if (socket.readyState === 'opening') { + socket.once('connect', function () { + startResponseTimer(socket); + }); + } else { + startResponseTimer(socket); + } + }); }); }; @@ -132,12 +159,40 @@ exports.read = function (response, encoding) { } return new Promise((resolve, reject) => { + const makeReadTimeoutError = (err) => { + const req = response.req; + err.name = 'RequestTimeoutError'; + err.message = `ReadTimeout: ${response.socket[READ_TIME_OUT]}. ${req.method} ${req.path} failed.`; + return err; + } + // check read-timer + let readTimer; + const oldReadTimer = response.socket[READ_TIMER]; + if (!oldReadTimer) { + reject(makeReadTimeoutError(new Error())); + return; + } + const remainTime = response.socket[READ_TIME_OUT] - (Date.now() - oldReadTimer.startTime); + clearTimeout(oldReadTimer); + if (remainTime <= 0) { + reject(makeReadTimeoutError(new Error())); + return; + } + readTimer = setTimeout(function () { + reject(makeReadTimeoutError(new Error())); + }, remainTime); + + // start reading data var onError, onData, onEnd; var cleanup = function () { // cleanup readable.removeListener('error', onError); readable.removeListener('data', onData); readable.removeListener('end', onEnd); + // clear read timer + if (readTimer) { + clearTimeout(readTimer); + } }; const bufs = []; diff --git a/test/index.test.js b/test/index.test.js index ba9f57d..016026d 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -7,7 +7,15 @@ const assert = require('assert'); const httpx = require('../'); const server = http.createServer((req, res) => { - if (req.url === '/timeout') { + if(req.url === '/readTimeout') { + setTimeout(() => { + res.writeHead(200, {'Content-Type': 'text/plain'}); + res.end('Hello world!'); + }, 200); + } else if (req.url === '/readTimeout2') { + res.writeHead(200, {'Content-Type': 'text/plain'}); + res.end('Hello world!'); + } else if (req.url === '/timeout') { setTimeout(() => { res.writeHead(200, {'Content-Type': 'text/plain'}); res.end('Hello world!'); @@ -63,9 +71,49 @@ describe('httpx', () => { } catch (ex) { assert.equal(ex.name, 'RequestTimeoutError'); const port = server.address().port; - assert.equal(ex.message, `Timeout(100). GET http://127.0.0.1:${port}/timeout failed.`); + assert.equal(ex.message, `ReadTimeout(100). GET http://127.0.0.1:${port}/timeout failed.`); return; } assert.ok(false, 'should not ok'); }); + + it('timeout should ok', async function () { + try { + await make(server)('/readTimeout', {readTimeout: 100, connectTimeout: 50}); + } catch (ex) { + assert.equal(ex.name, 'RequestTimeoutError'); + const port = server.address().port; + assert.equal(ex.message, `ReadTimeout(100). GET http://127.0.0.1:${port}/readTimeout failed.`); + return; + } + assert.ok(false, 'should not ok'); + }); + + it('timeout should ok', async function () { + try { + await make(server)('/readTimeout', {readTimeout: 100, connectTimeout: 50, timeout: 300}); + } catch (ex) { + assert.equal(ex.name, 'RequestTimeoutError'); + const port = server.address().port; + assert.equal(ex.message, `ReadTimeout(100). GET http://127.0.0.1:${port}/readTimeout failed.`); + return; + } + assert.ok(false, 'should not ok'); + }); + + it('read timeout should ok', async function () { + const res = await make(server)('/readTimeout2', {readTimeout: 100, connectTimeout: 50, timeout: 300}); + const err = await new Promise(resolve=> { + setTimeout(async function () { + try { + const data = await httpx.read(res); + resolve(null); + } catch (err) { + resolve(err); + } + }, 200) + }); + assert.ok(err, 'should throw error'); + assert.equal(err.message, 'ReadTimeout: 100. GET /readTimeout2 failed.') + }); });