Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(rate-limit): do not store instances/defaultconfigs globally #90

Merged
merged 1 commit into from Apr 27, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/create-http-client.js
Expand Up @@ -130,6 +130,6 @@ export default function createHttpClient (axios, options) {
...newParams
})
}
rateLimit(instance, axiosOptions, config.retryLimit)
rateLimit(instance, config.retryLimit)
return instance
}
11 changes: 4 additions & 7 deletions lib/rate-limit.js
@@ -1,12 +1,9 @@

const attempts = {}
const defaultsByInstance = new Map()
let networkErrorAttempts = 0

export default function rateLimit (instance, defaults, maxRetry = 5) {
defaultsByInstance.set(instance, defaults)
const instanceDefaults = defaultsByInstance.get(instance)
const { responseLogger = () => undefined, requestLogger = () => undefined } = instanceDefaults
export default function rateLimit (instance, maxRetry = 5) {
const { responseLogger = () => undefined, requestLogger = () => undefined } = instance.defaults

instance.interceptors.request.use(function (config) {
requestLogger(config)
Expand All @@ -22,7 +19,7 @@ export default function rateLimit (instance, defaults, maxRetry = 5) {
}, function (error) {
let { response, config } = error
// Do not retry if it is disabled or no request config exists (not an axios error)
if (!config || !instanceDefaults.retryOnError) {
if (!config || !instance.defaults.retryOnError) {
return Promise.reject(error)
}

Expand Down Expand Up @@ -75,7 +72,7 @@ export default function rateLimit (instance, defaults, maxRetry = 5) {
if (retryErrorType) {
// convert to ms and add jitter
wait = Math.floor(wait * 1000 + (Math.random() * 200) + 500)
instanceDefaults.logHandler('warning', `${retryErrorType} error occurred. Waiting for ${wait} ms before retrying...`)
instance.defaults.logHandler('warning', `${retryErrorType} error occurred. Waiting for ${wait} ms before retrying...`)

/* Somehow between the interceptor and retrying the request the httpAgent/httpsAgent gets transformed from an Agent-like object
to a regular object, causing failures on retries after rate limits. Removing these properties here fixes the error, but retry
Expand Down
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -48,7 +48,7 @@
"@babel/node": "^7.2.2",
"@babel/preset-env": "^7.2.3",
"@babel/register": "^7.0.0",
"axios": "^0.19.0",
"axios": "^0.19.1",
"axios-mock-adapter": "^1.15.0",
"babel-plugin-rewire": "^1.2.0",
"blue-tape": "^1.0.0",
Expand Down
20 changes: 7 additions & 13 deletions test/unit/rate-limit-test.js
Expand Up @@ -14,27 +14,24 @@ function setup (options = {}) {
logHandler: logHandlerStub,
retryOnError: true
}, options))
rateLimit(client, {
logHandler: logHandlerStub,
retryOnError: true
}, options.retryLimit)
rateLimit(client, options.retryLimit)
return { client }
}

function setupWithoutErrorRetry () {
const client = axios.create()
rateLimit(client, {
const client = axios.create({
retryOnError: false,
logHandler: logHandlerStub
})
rateLimit(client)
return { client }
}
function setupWithOneRetry () {
const client = axios.create()
rateLimit(client, {
const client = axios.create({
retryOnError: true,
logHandler: logHandlerStub
}, 1)
})
rateLimit(client, 1)
return { client }
}
function setupWithNonAxiosError () {
Expand All @@ -45,10 +42,7 @@ function setupWithNonAxiosError () {
client.interceptors.response.use(function (response) {
return Promise.reject(new Error('some non-axios error'))
})
rateLimit(client, {
retryOnError: true,
logHandler: logHandlerStub
})
rateLimit(client)
return { client }
}
function teardown () {
Expand Down