diff --git a/lib/_http_agent.js b/lib/_http_agent.js index 3db42174d73004..b9d2c0c2c04a7c 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -242,7 +242,12 @@ Agent.prototype.addRequest = function addRequest(req, options, port/* legacy */, } else if (sockLen < this.maxSockets) { debug('call onSocket', sockLen, freeLen); // If we are under maxSockets create a new one. - this.createSocket(req, options, handleSocketCreation(this, req, true)); + this.createSocket(req, options, (err, socket) => { + if (err) + req.onSocket(socket, err); + else + setRequestSocket(this, req, socket); + }); } else { debug('wait for socket'); // We are over limit so we'll add it to the queue. @@ -388,8 +393,12 @@ Agent.prototype.removeSocket = function removeSocket(s, options) { debug('removeSocket, have a request, make a socket'); const req = this.requests[name][0]; // If we have pending requests and a socket gets closed make a new one - const socketCreationHandler = handleSocketCreation(this, req, false); - this.createSocket(req, options, socketCreationHandler); + this.createSocket(req, options, (err, socket) => { + if (err) + req.onSocket(socket, err); + else + socket.emit('free'); + }); } }; @@ -422,19 +431,6 @@ Agent.prototype.destroy = function destroy() { } }; -function handleSocketCreation(agent, request, informRequest) { - return function handleSocketCreation_Inner(err, socket) { - if (err) { - process.nextTick(emitErrorNT, request, err); - return; - } - if (informRequest) - setRequestSocket(agent, request, socket); - else - socket.emit('free'); - }; -} - function setRequestSocket(agent, req, socket) { req.onSocket(socket); const agentTimeout = agent.options.timeout || 0; @@ -444,10 +440,6 @@ function setRequestSocket(agent, req, socket) { socket.setTimeout(req.timeout); } -function emitErrorNT(emitter, err) { - emitter.emit('error', err); -} - module.exports = { Agent, globalAgent: new Agent() diff --git a/lib/_http_client.js b/lib/_http_client.js index 73a6409c411cc1..c78289138e756a 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -371,10 +371,12 @@ function _destroy(req, socket, err) { // TODO (ronag): Check if socket was used at all (e.g. headersSent) and // re-use it in that case. `req.socket` just checks whether the socket was // assigned to the request and *might* have been used. - if (!req.agent || req.socket) { + if (socket && (!req.agent || req.socket)) { socket.destroy(err); } else { - socket.emit('free'); + if (socket) { + socket.emit('free'); + } if (!req.aborted && !err) { err = connResetException('socket hang up'); } @@ -776,15 +778,18 @@ function listenSocketTimeout(req) { } } -ClientRequest.prototype.onSocket = function onSocket(socket) { +ClientRequest.prototype.onSocket = function onSocket(socket, err) { // TODO(ronag): Between here and onSocketNT the socket // has no 'error' handler. - process.nextTick(onSocketNT, this, socket); + process.nextTick(onSocketNT, this, socket, err); }; -function onSocketNT(req, socket) { +function onSocketNT(req, socket, err) { if (req.destroyed) { _destroy(req, socket, req[kError]); + } else if (err) { + req.destroyed = true; + _destroy(req, null, err); } else { tickOnSocket(req, socket); } diff --git a/test/parallel/test-http-agent-close.js b/test/parallel/test-http-agent-close.js new file mode 100644 index 00000000000000..84ed5e57c5fb86 --- /dev/null +++ b/test/parallel/test-http-agent-close.js @@ -0,0 +1,21 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); + +const agent = new http.Agent(); +const _err = new Error('kaboom'); +agent.createSocket = function(req, options, cb) { + cb(_err); +}; + +const req = http + .request({ + agent + }) + .on('error', common.mustCall((err) => { + assert.strictEqual(err, _err); + })) + .on('close', common.mustCall(() => { + assert.strictEqual(req.destroyed, true); + }));