Skip to content
This repository was archived by the owner on Feb 13, 2024. It is now read-only.

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

Merged
merged 4 commits into from
Oct 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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",
Expand Down
76 changes: 76 additions & 0 deletions test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -33,6 +35,7 @@ const createClient = options => {
}

test.before.cb(t => {
let count = 0
express()
.use(bodyParser.json())
.post('/v1/batch', (req, res) => {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()
})
14 changes: 4 additions & 10 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down