Skip to content

Commit

Permalink
fix: should autofix socket timeout by request.timeout (#300)
Browse files Browse the repository at this point in the history
  • Loading branch information
fengmk2 committed Nov 1, 2018
1 parent 1a23576 commit 36c24c3
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 0 deletions.
15 changes: 15 additions & 0 deletions lib/urllib.js
Original file line number Diff line number Diff line change
Expand Up @@ -939,6 +939,15 @@ function requestWithCallback(url, args, callback) {
socket = socket.socket;
}

var orginalSocketTimeout = getSocketTimeout(socket);
if (orginalSocketTimeout && orginalSocketTimeout < responseTimeout) {
// make sure socket live longer than the response timer
var socketTimeout = responseTimeout + 500;
debug('Request#%d socket.timeout(%s) < responseTimeout(%s), reset socket timeout to %s',
reqId, orginalSocketTimeout, responseTimeout, socketTimeout);
socket.setTimeout(socketTimeout);
}

socketHandledRequests = socket[SOCKET_REQUEST_COUNT] = (socket[SOCKET_REQUEST_COUNT] || 0) + 1;
if (socket[SOCKET_RESPONSE_COUNT]) {
socketHandledResponses = socket[SOCKET_RESPONSE_COUNT];
Expand Down Expand Up @@ -1131,3 +1140,9 @@ function addLongStackTrace(err, req) {
err.stack += LONG_STACK_DELIMITER + callSiteStack.substr(index + 1);
}
}

// node 8 don't has timeout attribute on socket
// https://github.com/nodejs/node/pull/21204/files#diff-e6ef024c3775d787c38487a6309e491dR408
function getSocketTimeout(socket) {
return socket.timeout || socket._idleTimeout;
}
42 changes: 42 additions & 0 deletions test/request-timeout.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
'use strict';

var assert = require('assert');
var Agent = require('agentkeepalive');
var urllib = require('..');
var server = require('./fixtures/server');

describe('test/request-timeout.test.js', function() {
var host = 'http://127.0.0.1:';
var port;

before(function(done) {
server.listen(0, function() {
port = server.address().port;
host += port;
done();
});
});

after(function(done) {
setTimeout(function() {
server.close();
done();
}, 1000);
});

it('should work on request timeout bigger than agent timeout', function(done) {
var agent = new Agent({
timeout: 1000,
});
var url = host + '/response_timeout_10s';
urllib.request(url, {
timeout: 1500,
agent: agent,
}, function(err) {
assert(err);
assert(err.name === 'ResponseTimeoutError');
assert(err.message.indexOf('Response timeout for 1500ms') === 0);
done();
});
});
});

0 comments on commit 36c24c3

Please sign in to comment.