Skip to content
This repository has been archived by the owner on Oct 10, 2022. It is now read-only.

Commit

Permalink
feat!: stop retrying on 5xx responses (#726)
Browse files Browse the repository at this point in the history
  • Loading branch information
eduardoboucas authored Jul 21, 2022
1 parent 1e76962 commit d0328e9
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 9 deletions.
27 changes: 24 additions & 3 deletions src/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -494,17 +494,38 @@ test('Can timeout access token polling', async (t) => {
t.false(scope.isDone())
})

test('Retries on server errors', async (t) => {
test('Does not retry on server errors', async (t) => {
const errorMessage = 'Something went zap!'
const accountId = uuidv4()
const expectedResponse = { test: 'test' }
const scope = nock(origin)
.get(`${pathPrefix}/accounts/${accountId}`)
.reply(500)
.reply(500, errorMessage)
.get(`${pathPrefix}/accounts/${accountId}`)
.reply(200, expectedResponse)

const client = getClient()
const response = await client.getAccount({ account_id: accountId })
const error = await t.throwsAsync(client.getAccount({ account_id: accountId }))

t.is(error.status, 500)
t.is(error.message, errorMessage)
t.false(scope.isDone())
})

test('Retries on server errors for the `getLatestPluginRuns` endpoint', async (t) => {
const packages = 'foo'
const siteId = uuidv4()
const expectedResponse = { test: 'test' }
const scope = nock(origin)
.get(`${pathPrefix}/sites/${siteId}/plugin_runs/latest`)
.query({ packages })
.reply(500)
.get(`${pathPrefix}/sites/${siteId}/plugin_runs/latest`)
.query({ packages })
.reply(200, expectedResponse)

const client = getClient()
const response = await client.getLatestPluginRuns({ site_id: siteId, packages })

t.deepEqual(response, expectedResponse)
t.true(scope.isDone())
Expand Down
2 changes: 1 addition & 1 deletion src/methods/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ const makeRequestOrRetry = async function ({ url, method, defaultHeaders, agent,
const optsA = getOpts({ method, defaultHeaders, agent, requestParams, opts })
const { response, error } = await makeRequest(url, optsA)

if (shouldRetry({ response, error }) && index !== MAX_RETRY) {
if (shouldRetry({ response, error, method }) && index !== MAX_RETRY) {
await waitForRetry(response)
continue
}
Expand Down
16 changes: 11 additions & 5 deletions src/methods/retry.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
// We retry:
// - when receiving a rate limiting response
// - on network failures due to timeouts
export const shouldRetry = function ({ response = {}, error = {} }) {
return isRetryStatus(response) || RETRY_ERROR_CODES.has(error.code)
}
export const shouldRetry = function ({ response = {}, error = {}, method = {} }) {
if (response.status === RATE_LIMIT_STATUS || RETRY_ERROR_CODES.has(error.code)) {
return true
}

// Special case for the `getLatestPluginRuns` endpoint.
// See https://github.com/netlify/bitballoon/issues/9616.
if (method.operationId === 'getLatestPluginRuns' && response.status === 500) {
return true
}

const isRetryStatus = function ({ status }) {
return typeof status === 'number' && (status === RATE_LIMIT_STATUS || String(status).startsWith('5'))
return false
}

export const waitForRetry = async function (response) {
Expand Down

0 comments on commit d0328e9

Please sign in to comment.