Merge pull request #255 from yujidude/axiosFix

Fix for infinite axios-retry on 5xx responses and axios client option leakage

Author: pooyaj

index.js

@@ -45,8 +45,8 @@ class Analytics {
       enumerable: true,
       value: typeof options.enable === 'boolean' ? options.enable : true
     })
-
-    axiosRetry(axios, {
+    this.axiosClient = axios.create()
+    axiosRetry(this.axiosClient, {
       retries: options.retryCount || 3,
       retryCondition: this._isErrorRetryable,
       retryDelay: axiosRetry.exponentialDelay
@@ -279,7 +279,7 @@ class Analytics {
       req.timeout = typeof this.timeout === 'string' ? ms(this.timeout) : this.timeout
     }
 
-    axios(req)
+    this.axiosClient(req)
       .then(() => done())
       .catch(err => {
         if (err.response) {

package.json

@@ -22,7 +22,7 @@
     "circle-lint": ".buildscript/circle.sh",
     "dependencies": "yarn",
     "size": "size-limit",
-    "test": "standard && nyc ava && .buildscript/e2e.sh",
+    "test": "standard && nyc ava --timeout=20s&& .buildscript/e2e.sh",
     "report-coverage": "nyc report --reporter=lcov > coverage.lcov && codecov",
     "np": "np --no-publish",
     "release": "yarn run np"
@@ -41,7 +41,7 @@
   ],
   "dependencies": {
     "@segment/loosely-validate-event": "^2.0.0",
-    "axios": "^0.19.0",
+    "axios": "^0.19.2",
     "axios-retry": "^3.0.2",
     "lodash.isstring": "^4.0.1",
     "md5": "^2.2.1",

test.js

@@ -19,6 +19,8 @@ const context = {
 
 const metadata = { nodeVersion: process.versions.node }
 const port = 4063
+const separateAxiosClientPort = 4064
+const retryCount = 5
 
 const createClient = options => {
   options = Object.assign({
@@ -33,6 +35,7 @@ const createClient = options => {
 }
 
 test.before.cb(t => {
+  let count = 0
   express()
     .use(bodyParser.json())
     .post('/v1/batch', (req, res) => {
@@ -62,6 +65,19 @@ test.before.cb(t => {
         return setTimeout(() => res.end(), 5000)
       }
 
+      if (batch[0] === 'axios-retry') {
+        if (count++ === retryCount) return res.json({})
+        return res.status(503).json({
+          error: { message: 'Service Unavailable' }
+        })
+      }
+
+      if (batch[0] === 'axios-retry-forever') {
+        return res.status(503).json({
+          error: { message: 'Service Unavailable' }
+        })
+      }
+
       res.json({})
     })
     .listen(port, t.end)
@@ -561,3 +577,63 @@ test('allows messages > 32kb', t => {
     client.track(event, noop)
   })
 })
+
+test('ensure that failed requests are retried', async t => {
+  const client = createClient({ retryCount: retryCount })
+  const callback = spy()
+
+  client.queue = [
+    {
+      message: 'axios-retry',
+      callback
+    }
+  ]
+
+  await t.notThrows(client.flush())
+})
+
+test('ensure that failed requests are not retried forever', async t => {
+  const client = createClient()
+  const callback = spy()
+
+  client.queue = [
+    {
+      message: 'axios-retry-forever',
+      callback
+    }
+  ]
+
+  await t.throws(client.flush())
+})
+
+test('ensure other axios clients are not impacted by axios-retry', async t => {
+  let client = createClient() // eslint-disable-line
+  const axios = require('axios')
+
+  let callCounter = 0
+
+  // Client will return a successful response for any requests beyond the first
+  let server = express()
+    .use(bodyParser.json())
+    .get('/v1/anotherEndpoint', (req, res) => {
+      if (callCounter > 0) {
+        res.status(200).send('Ok')
+      } else {
+        callCounter++
+        res.status(503).send('Service down')
+      }
+    })
+    .listen(separateAxiosClientPort)
+
+  await axios.get(`http://localhost:${separateAxiosClientPort}/v1/anotherEndpoint`)
+    .then(response => {
+      t.fail()
+    })
+    .catch(error => {
+      if (error) {
+        t.pass()
+      }
+    })
+
+  server.close()
+})

yarn.lock

@@ -829,13 +829,12 @@ axios-retry@^3.0.2:
   dependencies:
     is-retry-allowed "^1.1.0"
 
-axios@^0.19.0:
-  version "0.19.0"
-  resolved "https://registry.yarnpkg.com/axios/-/axios-0.19.0.tgz#8e09bff3d9122e133f7b8101c8fbdd00ed3d2ab8"
-  integrity sha512-1uvKqKQta3KBxIz14F2v06AEHZ/dIoeKfbTRkK1E5oqjDnuEerLmYTgJB5AiQZHJcljpg1TuRzdjDR06qNk0DQ==
+axios@^0.19.2:
+  version "0.19.2"
+  resolved "https://registry.yarnpkg.com/axios/-/axios-0.19.2.tgz#3ea36c5d8818d0d5f8a8a97a6d36b86cdc00cb27"
+  integrity sha512-fjgm5MvRHLhx+osE2xoekY70AhARk3a6hkN+3Io1jc00jtquGvxYlKlsFUhmUET0V5te6CcZI7lcv2Ym61mjHA==
   dependencies:
     follow-redirects "1.5.10"
-    is-buffer "^2.0.2"
 
 babel-code-frame@^6.26.0:
   version "6.26.0"
@@ -4218,11 +4217,6 @@ is-buffer@^1.0.2, is-buffer@^1.1.5, is-buffer@~1.1.1:
   resolved "https://registry.yarnpkg.com/is-buffer/-/is-buffer-1.1.6.tgz#efaa2ea9daa0d7ab2ea13a97b2b8ad51fefbe8be"
   integrity sha512-NcdALwpXkTm5Zvvbk7owOUSvVvBKDgKP5/ewfXEznmQFfs4ZRmanOeKBTjRVjka3QFoN6XJ+9F3USqfHqTaU5w==
 
-is-buffer@^2.0.2:
-  version "2.0.3"
-  resolved "https://registry.yarnpkg.com/is-buffer/-/is-buffer-2.0.3.tgz#4ecf3fcf749cbd1e472689e109ac66261a25e725"
-  integrity sha512-U15Q7MXTuZlrbymiz95PJpZxu8IlipAp4dtS3wOdgPXx3mqBnslrWU14kxfHB+Py/+2PVKSr37dMAgM2A4uArw==
-
 is-builtin-module@^1.0.0:
   version "1.0.0"
   resolved "https://registry.yarnpkg.com/is-builtin-module/-/is-builtin-module-1.0.0.tgz#540572d34f7ac3119f8f76c30cbc1b1e037affbe"