Skip to content

Commit

Permalink
fix(retry): do only retry axios related errors
Browse files Browse the repository at this point in the history
fixes #49
  • Loading branch information
Benedikt Rötsch committed Feb 21, 2018
1 parent 01deb24 commit d328452
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 4 deletions.
9 changes: 5 additions & 4 deletions lib/rate-limit.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,17 @@ export default function rateLimit (instance, maxRetry = 5) {
return response
}, function (error) {
let { response, config } = error
if (!instance.defaults.retryOnError) {

// Do not retry if it is disabled or no request config exists (not an axios error)
if (!config || !instance.defaults.retryOnError) {
return Promise.reject(error)
}

let retryErrorType = null
let wait = 0

// Errors without response or config did not recieve anything from the server
// Should be a network related error
if (!response || !config) {
// Errors without response did not recieve anything from the server
if (!response) {
retryErrorType = 'Connection'
networkErrorAttempts++

Expand Down
34 changes: 34 additions & 0 deletions test/unit/rate-limit-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,17 @@ function setupWithOneRetry () {
rateLimit(client, 1)
return { client }
}
function setupWithNonAxiosError () {
const client = axios.create({
logHandler: logHandlerStub,
retryOnError: true
})
client.interceptors.response.use(function (response) {
return Promise.reject(new Error('some non-axios error'))
})
rateLimit(client)
return { client }
}
function teardown () {
logHandlerStub.reset()
mock.reset()
Expand Down Expand Up @@ -110,6 +121,27 @@ test('no retry when automatic handling flag is disabled', (t) => {
teardown()
})
})

test('no retry with non-axios error', (t) => {
const { client } = setupWithNonAxiosError()
mock.onGet('/rate-limit-me').replyOnce(200, 'worked but will fail due to interceptor', {'x-contentful-request-id': 4})

return client.get('/rate-limit-me')
.then((response) => {
t.fail('Promise should reject not resolve')
teardown()
})
.catch((error) => {
// Check if right error is returned:
t.equals(error.message, 'some non-axios error')
// Ensure no retry happened:
t.equals(logHandlerStub.callCount, 0, 'did not log anything')
t.equals(error.response, undefined)
t.equals(error.attempts, undefined)
teardown()
})
})

test('Should Fail if it hits maxRetries', (t) => {
const { client } = setupWithOneRetry()
mock.onGet('/error').replyOnce(500, 'error attempt #1', {'x-contentful-request-id': 4})
Expand All @@ -131,6 +163,7 @@ test('Should Fail if it hits maxRetries', (t) => {
teardown()
})
})

test('Rejects error straight away when X-Contentful-Request-Id header is missing', (t) => {
const { client } = setupWithOneRetry()
mock.onGet('/error').replyOnce(500, 'error attempt')
Expand All @@ -149,6 +182,7 @@ test('Rejects error straight away when X-Contentful-Request-Id header is missing
teardown()
})
})

test('Rejects errors with strange status codes', (t) => {
const { client } = setup()
mock.onGet('/error').replyOnce(765, 'error attempt')
Expand Down

0 comments on commit d328452

Please sign in to comment.