Repository: mcollina/autocannon
Pull request #301 (2020-11-17 14:22:54)

fix: handle large response body correctly
Supersedes https://github.com/mcollina/autocannon/pull/276/files
Fixes https://github.com/mcollina/autocannon/issues/275

Coincidently this fix also over-rides the code changes one done in https://github.com/mcollina/autocannon/pull/287 but the fix still works.

- Refactored response body parsing. It now collects body chunks until `kOnMessageComplete` fires and then it calls `onResponse` and emit `response`
- Added test to cover the same



Patch:
diff --git a/lib/httpClient.js b/lib/httpClient.js
index f50dd96..0e7ef80 100644
--- a/lib/httpClient.js
+++ b/lib/httpClient.js
@@ -49,6 +49,7 @@ function Client (opts) {
   for (let i = 0; i < pipelining; i++) {
     this.resData[i] = {
       bytes: 0,
+      body: '',
       headers: {},
       startTime: [0, 0]
     }
@@ -88,19 +89,8 @@ function Client (opts) {
   }
 
   this.parser[HTTPParser.kOnBody] = (body, start, len) => {
-    this.bodyRecorded = true
-
+    this.resData[this.cer].body += body.slice(start, start + len)
     this.emit('body', body)
-    const bodyString = '' + body.slice(start, start + len)
-
-    if (this.opts.expectBody) {
-      if (this.opts.expectBody !== bodyString) {
-        return this.emit('mismatch', bodyString)
-      }
-    }
-
-    const resp = this.resData[this.cer]
-    this.requestIterator.recordBody(resp.req, resp.headers.statusCode, bodyString)
   }
 
   this.parser[HTTPParser.kOnMessageComplete] = () => {
@@ -112,11 +102,15 @@ function Client (opts) {
       return this._resetConnection()
     }
 
-    if (!this.bodyRecorded) {
-      this.requestIterator.recordBody(resp.req, resp.headers.statusCode, undefined)
+    if (this.opts.expectBody && this.opts.expectBody !== resp.body) {
+      return this.emit('mismatch', resp.body)
     }
 
+    this.requestIterator.recordBody(resp.req, resp.headers.statusCode, resp.body)
+
     this.emit('response', resp.headers.statusCode, resp.bytes, resp.duration, this.rate)
+
+    resp.body = ''
     resp.bytes = 0
     this.cer = this.cer === pipelining - 1 ? 0 : this.cer++
     this._doRequest(this.cer)
@@ -181,7 +175,6 @@ Client.prototype._doRequest = function (rpi) {
         this.emit('reset')
       }
     }
-    this.bodyRecorded = false
     this.resData[rpi].req = this.requestIterator.currentRequest
     this.resData[rpi].startTime = process.hrtime()
     this.conn.write(this.getRequestBuffer())


Test patch:
diff --git a/test/httpClient.test.js b/test/httpClient.test.js
index cd9813d..60873f6 100644
--- a/test/httpClient.test.js
+++ b/test/httpClient.test.js
@@ -1,6 +1,7 @@
 'use strict'
 
 const os = require('os')
+const http = require('http')
 const path = require('path')
 const test = require('tap').test
 const Client = require('../lib/httpClient')
@@ -886,3 +887,36 @@ test('client invokes appropriate onResponse when using pipelining', (t) => {
     }
   })
 })
+
+test('client supports receiving large response body', (t) => {
+  t.plan(2)
+
+  const mockBody = Array.from({ length: 1024 * 10 }, (_, i) => `str-${i}`).join('\n')
+  const server = http.createServer((req, res) => {
+    res.end(mockBody)
+  })
+  server.listen(0)
+  server.unref()
+
+  let onResponseCalled = 0
+  const opts = server.address()
+  opts.method = 'POST'
+  opts.body = Buffer.from('hello world')
+  opts.requests = [
+    {
+      path: '/',
+      method: 'GET',
+      onResponse: (...args) => {
+        onResponseCalled++
+      }
+    }
+  ]
+
+  const client = new Client(opts)
+
+  client.on('response', (statusCode, length) => {
+    t.equal(onResponseCalled, 1, 'onResponse should be called only once')
+    t.equal(statusCode, 200, 'status code matches')
+    client.destroy()
+  })
+})
diff --git a/test/run.test.js b/test/run.test.js
index e92c4e7..533f2fe 100644
--- a/test/run.test.js
+++ b/test/run.test.js
@@ -718,7 +718,7 @@ test('should get onResponse callback invoked even when there is no body', async
         method: 'GET',
         onResponse (status, body) {
           t.same(status, 204)
-          t.same(body, undefined)
+          t.same(body, '')
         }
       }
     ]