fix: wait a macrotick to resume without pipelining (nodejs#1465)

* Wait a macrotick to resume without pipelining

This change makes the Client wait for a full macrotick before
executing up the next request if pipelining is disabled.
This is to account for socket errors events that might be waiting
to be processed in the event queue. This is the expected behavior
without pipelining.

This will slow us down a bit without pipelinig.

Fixes: nodejs#1415
Signed-off-by: Matteo Collina <[email protected]>

* fixup

Signed-off-by: Matteo Collina <[email protected]>

Author: mcollina

lib/client.js

@@ -873,6 +873,11 @@ class Parser {
       // have been queued since then.
       util.destroy(socket, new InformationalError('reset'))
       return constants.ERROR.PAUSED
+    } else if (client[kPipelining] === 1) {
+      // We must wait a full event loop cycle to reuse this socket to make sure
+      // that non-spec compliant servers are not closing the connection even if they
+      // said they won't.
+      setImmediate(resume, client)
     } else {
       resume(client)
     }

test/inflight-and-close.js

@@ -0,0 +1,31 @@
+'use strict'
+
+const t = require('tap')
+const { request } = require('..')
+const http = require('http')
+
+const server = http.createServer((req, res) => {
+  res.writeHead(200)
+  res.end('Response body')
+  res.socket.end() // Close the connection immediately with every response
+}).listen(0, '127.0.0.1', function () {
+  const url = `http://127.0.0.1:${this.address().port}`
+  request(url)
+    .then(({ statusCode, headers, body }) => {
+      t.pass('first response')
+      body.resume()
+      body.on('close', function () {
+        t.pass('first body closed')
+      })
+      return request(url)
+        .then(({ statusCode, headers, body }) => {
+          t.pass('second response')
+          body.resume()
+          body.on('close', function () {
+            server.close()
+          })
+        })
+    }).catch((err) => {
+      t.error(err)
+    })
+})