diff --git a/lib/_http_agent.js b/lib/_http_agent.js index dc46c0e0bc7147..66e8632c9d1285 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -57,6 +57,13 @@ class ReusedHandle { } } +function freeSocketErrorListener(err) { + const socket = this; + debug('SOCKET ERROR on FREE socket:', err.message, err.stack); + socket.destroy(); + socket.emit('agentRemove'); +} + function Agent(options) { if (!(this instanceof Agent)) return new Agent(options); @@ -82,6 +89,11 @@ function Agent(options) { const name = this.getName(options); debug('agent.on(free)', name); + // TODO(ronag): socket.destroy(err) might have been called + // before coming here and have an 'error' scheduled. In the + // case of socket.destroy() below this 'error' has no handler + // and could cause unhandled exception. + if (socket.writable && this.requests[name] && this.requests[name].length) { const req = this.requests[name].shift(); @@ -118,6 +130,7 @@ function Agent(options) { socket.setTimeout(agentTimeout); } + socket.once('error', freeSocketErrorListener); freeSockets.push(socket); } else { // Implementation doesn't want to keep socket alive @@ -393,6 +406,7 @@ Agent.prototype.keepSocketAlive = function keepSocketAlive(socket) { Agent.prototype.reuseSocket = function reuseSocket(socket, req) { debug('have free socket'); + socket.removeListener('error', freeSocketErrorListener); req.reusedSocket = true; socket.ref(); }; diff --git a/lib/_http_client.js b/lib/_http_client.js index 3f335aa6eaa7ce..0c7ae0045ec9f5 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -473,13 +473,6 @@ function socketErrorListener(err) { socket.destroy(); } -function freeSocketErrorListener(err) { - const socket = this; - debug('SOCKET ERROR on FREE socket:', err.message, err.stack); - socket.destroy(); - socket.emit('agentRemove'); -} - function socketOnEnd() { const socket = this; const req = this._httpMessage; @@ -658,8 +651,11 @@ function responseKeepAlive(req) { socket.removeListener('error', socketErrorListener); socket.removeListener('data', socketOnData); socket.removeListener('end', socketOnEnd); - socket.once('error', freeSocketErrorListener); - // There are cases where _handle === null. Avoid those. Passing null to + + // TODO(ronag): Between here and emitFreeNT the socket + // has no 'error' handler. + + // There are cases where _handle === null. Avoid those. Passing undefined to // nextTick() will call getDefaultTriggerAsyncId() to retrieve the id. const asyncId = socket._handle ? socket._handle.getAsyncId() : undefined; // Mark this socket as available, AFTER user-added end @@ -741,7 +737,6 @@ function tickOnSocket(req, socket) { } parser.onIncoming = parserOnIncomingClient; - socket.removeListener('error', freeSocketErrorListener); socket.on('error', socketErrorListener); socket.on('data', socketOnData); socket.on('end', socketOnEnd); @@ -781,6 +776,8 @@ function listenSocketTimeout(req) { } ClientRequest.prototype.onSocket = function onSocket(socket) { + // TODO(ronag): Between here and onSocketNT the socket + // has no 'error' handler. process.nextTick(onSocketNT, this, socket); };