From 25bc8453510f0115157babffd08b29aa72a1bf3f Mon Sep 17 00:00:00 2001
From: Robert Nagy <ronagy@icloud.com>
Date: Thu, 30 Apr 2020 23:36:19 +0200
Subject: [PATCH 1/2] http: ensure client request emits close

If socket creation failed then an error would be
emitted on the client request object, but not
'close' nor would destroyed be set to true.
---
 lib/_http_agent.js                     | 32 ++++++++++----------------
 lib/_http_client.js                    | 15 ++++++++----
 test/parallel/test-http-agent-close.js | 22 ++++++++++++++++++
 3 files changed, 44 insertions(+), 25 deletions(-)
 create mode 100644 test/parallel/test-http-agent-close.js

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..8df5e59a1e22dd
--- /dev/null
+++ b/test/parallel/test-http-agent-close.js
@@ -0,0 +1,22 @@
+
+'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);
+  }));

From a7ba5fa7ab8c434c70b1d29f571957cd395e6013 Mon Sep 17 00:00:00 2001
From: Robert Nagy <ronagy@icloud.com>
Date: Mon, 4 May 2020 00:09:07 +0200
Subject: [PATCH 2/2] fixup

---
 test/parallel/test-http-agent-close.js | 1 -
 1 file changed, 1 deletion(-)

diff --git a/test/parallel/test-http-agent-close.js b/test/parallel/test-http-agent-close.js
index 8df5e59a1e22dd..84ed5e57c5fb86 100644
--- a/test/parallel/test-http-agent-close.js
+++ b/test/parallel/test-http-agent-close.js
@@ -1,4 +1,3 @@
-
 'use strict';
 const common = require('../common');
 const assert = require('assert');