http: ensure client request emits close

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()

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);
   }

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);
+  }));