Repository: mcollina/autocannon (js)

Issue:
client.setHeaders only sets headers for first request
I'm not sure if this is intended behavior or a bug, but in the `setupClient`, while doing a `client.setHeaders`, it only affects the first request in the `requests` array.

Example:


```js
'use strict'

const autocannon = require('autocannon')

autocannon({
    url: `https://example.com`,
    connections: 1,
    duration: 1,
    amount: 1,
    setupClient: (client) => {
        const userInfo = getNextUserInfo()
        // I expect this to set the headers for all requests in the list below
        client.setHeaders({
            'Authorization': userInfo.authorizationToken,
            'Content-Type': 'application/json',
        })
    },
    requests: [
        {
            path: `/someEndpoint`,
            method: 'POST',
            setupRequest: (req, context) => {
                // I do this hack to get the headers forwarded to the next request;
                // context.authHeaders = req.headers // left out to show bug
                return {
                    ...req,
                    body: JSON.stringify({
                        command: 'startTournamentLevel',
                    }),
                }
            },
            onResponse: (status, body, context) => {
                if (status === 200) {
                    context.data = JSON.parse(body)
                }
            },
        },
        {
            path: `/someOtherEndpoint`,
            method: 'POST',
            setupRequest: (req, context) => {
                console.log('req.headers is not set', JSON.stringify(req.headers))
                return {
                    ...req,
                    // headers: context.authHeaders, // I do this hack to get the headers forwarded. Left out to expose bug
                    body: JSON.stringify({
                        someData: context.data.some,
                    }),
                }
            },
        }],
}, console.log)
```

I think I have found the reason in the source code;

The method on the `httpClient` passes the to the requestIterator

https://github.com/mcollina/autocannon/blob/53643534b935e81970244ea1814cd74572b9e17d/lib/httpClient.js#L211

It seems this method only sets the headers for the `currentRequest`
https://github.com/mcollina/autocannon/blob/53643534b935e81970244ea1814cd74572b9e17d/lib/requestIterator.js#L54

I'm not sure what is the correct behavior, but I would expect that perhaps the `httpClient` saved the headers, and added it to all requests as the new default.

Again, not sure if it is a bug, but it feels like it. I was able to work around it by adding the headers to the context object on the first request, in case anyone else has this issue.

Pull request:
fix: Reuse headers when set - fixes #280
Thanks for creating and maintaining `autocannon`. I really enjoy using this software.

I was running tests on an AWS API Gateway where I needed to configure a fixed set of headers for a couple of requests when I encountered the same issue as in #280 .

I went ahead and tried to fix it. All feedback is welcome and I'll try to update as quick as possible.

Patch:
diff --git a/lib/requestIterator.js b/lib/requestIterator.js
index 20d0275..0a1c4eb 100644
--- a/lib/requestIterator.js
+++ b/lib/requestIterator.js
@@ -11,6 +11,7 @@ function RequestIterator (opts) {
   }
 
   this.resetted = false
+  this.headers = {}
   this.initialContext = opts.initialContext || {}
   this.resetContext()
   this.reqDefaults = opts
@@ -61,7 +62,8 @@ RequestIterator.prototype.setRequests = function (newRequests) {
 }
 
 RequestIterator.prototype.setHeaders = function (newHeaders) {
-  this.currentRequest.headers = newHeaders || {}
+  this.headers = newHeaders || {}
+  this.currentRequest.headers = this.headers
   this.rebuildRequest()
 }
 
@@ -71,7 +73,8 @@ RequestIterator.prototype.setBody = function (newBody) {
 }
 
 RequestIterator.prototype.setHeadersAndBody = function (newHeaders, newBody) {
-  this.currentRequest.headers = newHeaders || {}
+  this.headers = newHeaders || {}
+  this.currentRequest.headers = this.headers
   this.currentRequest.body = newBody || Buffer.alloc(0)
   this.rebuildRequest()
 }
@@ -85,6 +88,7 @@ RequestIterator.prototype.rebuildRequest = function () {
   let data
   this.resetted = false
   if (this.currentRequest) {
+    this.currentRequest.headers = this.headers
     data = this.requestBuilder(this.currentRequest, this.context)
     if (data) {
       this.currentRequest.requestBuffer = this.reqDefaults.idReplacement


Test patch:
diff --git a/test/requestIterator.test.js b/test/requestIterator.test.js
index e27ef18..0b4bc4f 100644
--- a/test/requestIterator.test.js
+++ b/test/requestIterator.test.js
@@ -136,7 +136,7 @@ test('request iterator should allow for rebuilding the current request', (t) =>
   const request2Res = Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 8\r\n\r\nmodified`)
   const request3Res = Buffer.from(`GET / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 8\r\n\r\nmodified`)
   const request4Res = Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nheader: modifiedHeader\r\nContent-Length: 8\r\n\r\nmodified`)
-  const request5Res = Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\n\r\n`)
+  const request5Res = Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nheader: modifiedHeader\r\n\r\n`)
 
   opts.requests = requests1
 
@@ -258,7 +258,7 @@ test('request iterator should properly mutate requests if a setupRequest functio
   const request3Res = Buffer.from(`GET / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 8\r\n\r\nmodified`)
   const request4Res = Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nContent-Length: 9\r\n\r\nmodified2`)
   const request5Res = Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nheader: modifiedHeader\r\nContent-Length: 9\r\n\r\nmodified3`)
-  const request6Res = Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\n\r\n`)
+  const request6Res = Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nheader: modifiedHeader\r\n\r\n`)
 
   const iterator = new RequestIterator(opts)
   t.same(iterator.currentRequest.requestBuffer, request1Res, 'request was okay')
@@ -435,3 +435,49 @@ test('request iterator should initialize context from options', (t) => {
   // will reset and reinit context
   iterator.nextRequest()
 })
+
+test('request iterator should use the same headers when set', (t) => {
+  t.plan(6)
+
+  const opts = server.address()
+  opts.method = 'POST'
+
+  let i = 0
+
+  opts.requests = [
+    {
+      body: 'hello world',
+      setupRequest: req => {
+        req.body += i++
+        return req
+      }
+    },
+    {
+      method: 'POST',
+      body: 'modified',
+      setupRequest: req => {
+        req.method = 'GET'
+        return req
+      }
+    }
+  ]
+
+  const request1Res = Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nAccess-Control-Allow-Credentials: true\r\nContent-Length: 12\r\n\r\nhello world1`)
+  const request2Res = Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nAccess-Control-Allow-Credentials: true\r\nContent-Length: 12\r\n\r\nhello world1`)
+  const request3Res = Buffer.from(`GET / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nAccess-Control-Allow-Credentials: true\r\nContent-Length: 8\r\n\r\nmodified`)
+  const request4Res = Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nAccess-Control-Allow-Credentials: true\r\nContent-Length: 12\r\n\r\nhello world2`)
+  const request5Res = Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nAccess-Control-Allow-Credentials: true\r\nContent-Length: 12\r\n\r\nhello world2`)
+  const request6Res = Buffer.from(`POST / HTTP/1.1\r\nHost: localhost:${server.address().port}\r\nConnection: keep-alive\r\nAccess-Control-Allow-Credentials: true\r\n\r\n`)
+
+  const iterator = new RequestIterator(opts)
+  iterator.setHeaders({ 'Access-Control-Allow-Credentials': 'true' })
+  t.same(iterator.currentRequest.requestBuffer, request1Res, iterator.currentRequest.requestBuffer.toString())
+  t.same(iterator.currentRequest.requestBuffer, request2Res, iterator.currentRequest.requestBuffer.toString())
+  iterator.nextRequest()
+  t.same(iterator.currentRequest.requestBuffer, request3Res, iterator.currentRequest.requestBuffer.toString())
+  iterator.nextRequest()
+  t.same(iterator.currentRequest.requestBuffer, request4Res, iterator.currentRequest.requestBuffer.toString())
+  t.same(iterator.currentRequest.requestBuffer, request5Res, iterator.currentRequest.requestBuffer.toString())
+  iterator.setRequest()
+  t.same(iterator.currentRequest.requestBuffer, request6Res, iterator.currentRequest.requestBuffer.toString())
+})