Repository: mcollina/autocannon
Pull request #317 (2020-12-15 14:18:18)

fix (#315): correctly increment this.cer
Fixes https://github.com/mcollina/autocannon/issues/315


Patch:
diff --git a/lib/httpClient.js b/lib/httpClient.js
index 0250c77..2f6e04e 100644
--- a/lib/httpClient.js
+++ b/lib/httpClient.js
@@ -112,10 +112,14 @@ function Client (opts) {
 
     this.emit('response', resp.headers.statusCode, resp.bytes, resp.duration, this.rate)
 
+    // Separating rip ensures that after getting response for first request, we fire
+    // first one again because second is already running right now (with pipelining=2).
+    // See detailed explaination at https://github.com/mcollina/autocannon/pull/317#discussion_r543678831
+    const rpi = (this.cer + pipelining) % pipelining
     resp.body = ''
     resp.bytes = 0
-    this.cer = this.cer === pipelining - 1 ? 0 : this.cer++
-    this._doRequest(this.cer)
+    this.cer = (this.cer + 1) % pipelining
+    this._doRequest(rpi)
   }
 
   this._connect()


Test patch:
diff --git a/test/helper.js b/test/helper.js
index c691350..37b7b54 100644
--- a/test/helper.js
+++ b/test/helper.js
@@ -36,7 +36,13 @@ function startServer (opts) {
     }
 
     res.statusCode = statusCode
-    res.end(typeof body === 'function' ? body(req) : body)
+    const reply = () => res.end(typeof body === 'function' ? body(req) : body)
+
+    if (opts.delayResponse) {
+      setTimeout(reply, opts.delayResponse)
+    } else {
+      reply()
+    }
   }
 
   server.unref()
diff --git a/test/httpClient.test.js b/test/httpClient.test.js
index 4bd6bb2..2c8923e 100644
--- a/test/httpClient.test.js
+++ b/test/httpClient.test.js
@@ -63,6 +63,25 @@ test('client calls a https server twice', (t) => {
   })
 })
 
+test('client calculates correct duration when using pipelining', (t) => {
+  t.plan(4)
+
+  const lazyServer = helper.startServer({ delayResponse: 500 })
+  const opts = lazyServer.address()
+  opts.pipelining = 2
+  const client = new Client(opts)
+  let count = 0
+
+  client.on('response', (statusCode, length, duration) => {
+    t.equal(statusCode, 200, 'status code matches')
+    t.ok(duration > 500 && duration < 600)
+
+    if (++count === 2) {
+      client.destroy()
+    }
+  })
+})
+
 test('client calls a tls server without SNI servername twice', (t) => {
   t.plan(4)
 
@@ -891,17 +910,17 @@ test('client invokes appropriate onResponse when using pipelining', (t) => {
         t.deepEqual(responses, ['POST'])
         break
       case 2:
-        // 3rd was sent as 1st is finished, receiving 2st
+        // 3rd was sent as 1st is finished, receiving 2nd
         t.same(client.getRequestBuffer().toString(), makeResponseFromBody({ server, method: 'PUT' }), 'current should be third request')
         t.deepEqual(responses, ['POST', 'GET'])
         break
       case 3:
-        // 1st was resent, receiving 3st
+        // 1st was resent, receiving 3rd
         t.same(client.getRequestBuffer().toString(), makeResponseFromBody({ server, method: 'POST' }), 'current should be first request')
         t.deepEqual(responses, ['POST', 'GET', 'PUT'])
         break
       case 4:
-        // 2st was resent, receiving 1st
+        // 2nd was resent, receiving 1st
         t.same(client.getRequestBuffer().toString(), makeResponseFromBody({ server, method: 'GET' }), 'current should be second request')
         t.deepEqual(responses, ['POST', 'GET', 'PUT', 'POST'])
         client.destroy()