Skip to content

Commit

Permalink
Merge pull request #7 from hyj1991/master
Browse files Browse the repository at this point in the history
distinguish between read timeout and connect timeout
  • Loading branch information
JacksonTian committed Mar 15, 2019
2 parents 25963d1 + 003953c commit 9f4b0dc
Show file tree
Hide file tree
Showing 2 changed files with 124 additions and 21 deletions.
93 changes: 74 additions & 19 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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') {
Expand All @@ -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) => {
Expand All @@ -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);
};

Expand All @@ -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);
Expand All @@ -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);
}
});
});
};

Expand All @@ -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 = [];
Expand Down
52 changes: 50 additions & 2 deletions test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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!');
Expand Down Expand Up @@ -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.')
});
});

0 comments on commit 9f4b0dc

Please sign in to comment.