Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding parseInt to config.timeout #3781

Merged
merged 2 commits into from May 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 16 additions & 2 deletions lib/adapters/http.js
Expand Up @@ -279,15 +279,29 @@ module.exports = function httpAdapter(config) {

// Handle request timeout
if (config.timeout) {
// This is forcing a int timeout to avoid problems if the `req` interface doesn't handle other types.
var timeout = parseInt(config.timeout, 10);
jasonsaayman marked this conversation as resolved.
Show resolved Hide resolved

if (isNaN(timeout)) {
reject(createError(
'error trying to parse `config.timeout` to int',
config,
'ERR_PARSE_TIMEOUT',
req
));

return;
}

// 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 devoring 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(config.timeout, function handleRequestTimeout() {
req.setTimeout(timeout, function handleRequestTimeout() {
req.abort();
reject(createError(
'timeout of ' + config.timeout + 'ms exceeded',
'timeout of ' + timeout + 'ms exceeded',
config,
config.transitional && config.transitional.clarifyTimeoutError ? 'ETIMEDOUT' : 'ECONNABORTED',
req
Expand Down
58 changes: 58 additions & 0 deletions test/unit/adapters/http.js
Expand Up @@ -29,6 +29,64 @@ describe('supports http with nodejs', function () {
}
});

it('should throw an error if the timeout property is not parsable as a number', function (done) {

server = http.createServer(function (req, res) {
setTimeout(function () {
res.end();
}, 1000);
}).listen(4444, function () {
var success = false, failure = false;
var error;

axios.get('http://localhost:4444/', {
timeout: { strangeTimeout: 250 }
}).then(function (res) {
success = true;
}).catch(function (err) {
error = err;
failure = true;
});

setTimeout(function () {
assert.equal(success, false, 'request should not succeed');
assert.equal(failure, true, 'request should fail');
assert.equal(error.code, 'ERR_PARSE_TIMEOUT');
assert.equal(error.message, 'error trying to parse `config.timeout` to int');
done();
}, 300);
});
});

it('should parse the timeout property', function (done) {

server = http.createServer(function (req, res) {
setTimeout(function () {
res.end();
}, 1000);
}).listen(4444, function () {
var success = false, failure = false;
var error;

axios.get('http://localhost:4444/', {
timeout: '250'
}).then(function (res) {
success = true;
}).catch(function (err) {
error = err;
failure = true;
});

setTimeout(function () {
assert.equal(success, false, 'request should not succeed');
assert.equal(failure, true, 'request should fail');
assert.equal(error.code, 'ECONNABORTED');
assert.equal(error.message, 'timeout of 250ms exceeded');
done();
}, 300);
});
});

it('should respect the timeout property', function (done) {

server = http.createServer(function (req, res) {
Expand Down