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

Add retryCodes option in HttpMiddlewareOptions #1764

Merged
merged 7 commits into from
Mar 7, 2022
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
9 changes: 8 additions & 1 deletion docs/sdk/api/sdkMiddlewareHttp.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ The HTTP middleware can run in either a browser or Node.js environment. For Node
4. `includeOriginalRequest` _(Boolean)_: flag whether to include the original request sent in the response. Can be useful if you want to see the final request being sent.
5. `maskSensitiveHeaderData` _(Boolean)_: flag to mask sensitie data in the header. e.g. Authorization token
6. `enableRetry` _(Boolean)_: flag to enable retry on network errors and `500` response. (Default: false)
7. `retryConfig` _(Object)_: Field required in the object listed below
7. `retryCodes` - _(Array)_: array of `statusCodes` [`numbers`] and error code or messages [`string`] for retring requests when the statusCodes and/or error message/code matches an entry in the list.
8. `maxRetries` _(Number)_: number of times to retry the request before failing the request. (Default: 50)
9. `retryDelay` _(Number)_: amount of milliseconds to wait before retrying the next request. (Default: 200)
10. `backoff` _(Boolean)_: activates exponential backoff. Recommended to prevent spamming of the server. (Default: true)
Expand All @@ -42,6 +42,7 @@ The HTTP middleware can run in either a browser or Node.js environment. For Node
13. `fetch` _(Function)_: A `fetch` implementation which can be e.g. `node-fetch` or `unfetch` but also the native browser `fetch` function
14. `timeout` _(Number)_: Request/response timeout in ms. Must be globally available or passed in `AbortController`
15. `abortController` or `getAbortController` depending on what you chose to handle the timeout (_abortController_): This property accepts the `AbortController` instance. Could be [abort-controller](https://www.npmjs.com/package/abort-controller) or a globally available one.
16. `retryConfig` _(Object)_: Field required in the object listed below

#### Retrying requests

Expand Down Expand Up @@ -72,6 +73,12 @@ const client = createClient({
retryDelay: 300, //milliseconds
maxDelay: 5000, //milliseconds
retryOnAbort: false,
retryCodes: [
504,
'ETIMEDOUT',
'ECONNREFUSED',
503
]
},

// Optional if not globally available
Expand Down
73 changes: 45 additions & 28 deletions packages/sdk-middleware-http/src/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ function calcDelayDuration(
if (backoff)
return retryCount !== 0 // do not increase if it's the first retry
? Math.min(
Math.round((Math.random() + 1) * retryDelay * 2 ** retryCount),
maxDelay
)
Math.round((Math.random() + 1) * retryDelay * 2 ** retryCount),
maxDelay
)
: retryDelay
return retryDelay
}
Expand Down Expand Up @@ -66,6 +66,7 @@ export default function createHttpMiddleware({
retryDelay = 200,
maxDelay = Infinity,
retryOnAbort = false,
retryCodes = [503],
} = {},
fetch: fetcher,
abortController: _abortController,
Expand Down Expand Up @@ -94,6 +95,12 @@ export default function createHttpMiddleware({
fetchFunction = fetch
}

if (!Array.isArray(retryCodes)) {
throw new Error(
'`retryCodes` option must be an array of retry status (error) codes.'
)
}

return (next: Next): Next =>
(request: MiddlewareRequest, response: MiddlewareResponse) => {
let abortController: any
Expand All @@ -114,9 +121,9 @@ export default function createHttpMiddleware({
// Ensure body is a string if content type is application/json
const body =
requestHeader['Content-Type'] === 'application/json' &&
typeof request.body !== 'string'
typeof request.body !== 'string'
? // NOTE: `stringify` of `null` gives the String('null')
JSON.stringify(request.body || undefined)
JSON.stringify(request.body || undefined)
: request.body

if (body && (typeof body === 'string' || Buffer.isBuffer(body))) {
Expand All @@ -132,15 +139,16 @@ export default function createHttpMiddleware({
}

if (!retryOnAbort) {
if (timeout || getAbortController || _abortController)
if (timeout || getAbortController || _abortController) {
// eslint-disable-next-line
abortController =
abortController =
(getAbortController ? getAbortController() : null) ||
_abortController ||
new AbortController()

if (abortController) {
fetchOptions.signal = abortController.signal
if (abortController) {
fetchOptions.signal = abortController.signal
}
}
}

Expand All @@ -151,16 +159,18 @@ export default function createHttpMiddleware({
// wrap in a fn so we can retry if error occur
function executeFetch() {
if (retryOnAbort) {
if (timeout || getAbortController || _abortController)
if (timeout || getAbortController || _abortController) {
// eslint-disable-next-line
abortController =
abortController =
(getAbortController ? getAbortController() : null) ||
_abortController ||
new AbortController()

if (abortController) {
fetchOptions.signal = abortController.signal
if (abortController) {
fetchOptions.signal = abortController.signal
}
}

}
// Kick off timer for abortController directly before fetch.
let timer
Expand Down Expand Up @@ -225,21 +235,6 @@ export default function createHttpMiddleware({
})
return
}
if (res.status === 503 && enableRetry)
if (retryCount < maxRetries) {
setTimeout(
executeFetch,
calcDelayDuration(
retryCount,
retryDelay,
maxRetries,
backoff,
maxDelay
)
)
retryCount += 1
return
}

// Server responded with an error. Try to parse it as JSON, then
// return a proper error type with all necessary meta information.
Expand All @@ -261,6 +256,28 @@ export default function createHttpMiddleware({
? { message: parsed.message, body: parsed }
: { message: parsed, body: parsed }),
})

if (
enableRetry &&
(retryCodes.indexOf(error.statusCode) !== -1 ||
retryCodes?.indexOf(error.message) !== -1)
) {
if (retryCount < maxRetries) {
setTimeout(
executeFetch,
calcDelayDuration(
retryCount,
retryDelay,
maxRetries,
backoff,
maxDelay
)
)
retryCount += 1
return
}
}

maskAuthData(error.originalRequest, maskSensitiveHeaderData)
// Let the final resolver to reject the promise
const parsedResponse = {
Expand Down
210 changes: 210 additions & 0 deletions packages/sdk-middleware-http/test/http.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,16 @@ describe('Http', () => {
)
})

test('throw when a non-array option is passed as retryCodes in the httpMiddlewareOptions', () => {
expect(() => {
createHttpMiddleware({ host: testHost, retryConfig: { retryCodes: null }, fetch })
}).toThrow(
new Error(
'`retryCodes` option must be an array of retry status (error) codes.'
)
)
})

test('execute a get request (success)', () =>
new Promise((resolve, reject) => {
const request = createTestRequest({
Expand Down Expand Up @@ -677,6 +687,206 @@ describe('Http', () => {
httpMiddleware(next)(request, response)
}))

test('should retry when status (error) code is part of retryCodes', () =>
new Promise((resolve, reject) => {
const request = createTestRequest({
uri: '/foo/bar',
})
const response = { resolve, reject }
const next = (req, res) => {
expect(res.error.name).toBe('InternalServerError')
expect(res.error.originalRequest).toBeDefined()
expect(res.body).toBeUndefined()
expect(res.statusCode).toBe(500)
expect(res.error.retryCount).toBe(2)
resolve()
}
const options = {
host: testHost,
enableRetry: true,
retryConfig: {
maxRetries: 2,
retryDelay: 300,
retryCodes: [
500, 501, 502
],
},
fetch,
}
const httpMiddleware = createHttpMiddleware(options)
nock(testHost).get('/foo/bar').times(3).reply(500)

httpMiddleware(next)(request, response)
}))

test('should retry when status (error) message is part of retryCodes', () =>
new Promise((resolve, reject) => {
const request = createTestRequest({
uri: '/foo/bar',
})
const response = { resolve, reject }
const next = (req, res) => {
expect(res.error.name).toBe('InternalServerError')
expect(res.error.message).toBe('ETIMEDOUT')
expect(res.error.retryCount).toBe(2)
expect(res.error.statusCode).toBe(500)
resolve()
}

const httpMiddleware = createHttpMiddleware({
host: testHost,
enableRetry: true,
retryConfig: {
maxRetries: 2,
retryDelay: 300,
retryCodes: [
'ETIMEDOUT', 'ECONNREFUSED', 'write EPIPE'
]
},
fetch,
})
nock(testHost)
.defaultReplyHeaders({
'Content-Type': 'application/json'
})
.persist()
.get('/foo/bar')
.reply(500, 'ETIMEDOUT')

httpMiddleware(next)(request, response)
}))

test('should retry when status (error) code and message are both part of retryCodes', () =>
new Promise((resolve, reject) => {
const request = createTestRequest({
uri: '/foo/bar',
})
const response = { resolve, reject }
const next = (req, res) => {
expect(res.error.name).toBe('HttpError')
expect(res.error.message).toBe('ETIMEDOUT')
expect(res.error.retryCount).toBe(2)
expect(res.error.statusCode).toBe(502)
resolve()
}

const httpMiddleware = createHttpMiddleware({
host: testHost,
enableRetry: true,
retryConfig: {
maxRetries: 2,
retryDelay: 300,
retryCodes: [
'ETIMEDOUT', 503, 502, 'ECONNREFUSED', 'write EPIPE'
]
},
fetch,
})
nock(testHost)
.defaultReplyHeaders({
'Content-Type': 'application/json'
})
.persist()
.get('/foo/bar')
.reply(502, 'ETIMEDOUT')

httpMiddleware(next)(request, response)
}))

test('should not retry when status (error) message is not part of retryCodes', () =>
new Promise((resolve, reject) => {
const request = createTestRequest({
uri: '/foo/bar',
})
const response = { resolve, reject }
const next = (req, res) => {
expect(res.error.name).toBe('InternalServerError')
expect(res.error.originalRequest).toBeDefined()
expect(res.body).toBeUndefined()
expect(res.statusCode).toBe(500)
expect(res.error.retryCount).toBe(0)
resolve()
}
const options = {
host: testHost,
enableRetry: true,
retryCodes: ['Not Included'],
retryConfig: {
maxRetries: 2,
retryDelay: 300,
},
fetch,
}
const httpMiddleware = createHttpMiddleware(options)
nock(testHost)
.get('/foo/bar')
.times(3)
.reply(500, 'Internal Server Error')

httpMiddleware(next)(request, response)
}))

test('should not retry when status (error) code is not part of retryCodes', () =>
new Promise((resolve, reject) => {
const request = createTestRequest({
uri: '/foo/bar',
})
const response = { resolve, reject }
const next = (req, res) => {
expect(res.error.name).toBe('InternalServerError')
expect(res.error.originalRequest).toBeDefined()
expect(res.body).toBeUndefined()
expect(res.statusCode).toBe(500)
expect(res.error.retryCount).toBe(0)
resolve()
}
const options = {
host: testHost,
enableRetry: true,
retryCodes: [501, 502],
retryConfig: {
maxRetries: 2,
retryDelay: 300,
},
fetch,
}
const httpMiddleware = createHttpMiddleware(options)
nock(testHost).get('/foo/bar').times(3).reply(500)

httpMiddleware(next)(request, response)
}))

test('should not retry when enableRetry is set to false ', () =>
new Promise((resolve, reject) => {
const request = createTestRequest({
uri: '/foo/bar',
})
const response = { resolve, reject }
const next = (req, res) => {
expect(res.error.name).toBe('InternalServerError')
expect(res.error.originalRequest).toBeDefined()
expect(res.body).toBeUndefined()
expect(res.statusCode).toBe(500)
expect(res.error.retryCount).toBe(0)
resolve()
}
const options = {
host: testHost,
enableRetry: false,
retryCodes: [500, 502],
retryConfig: {
maxRetries: 2,
retryDelay: 300,
},
fetch,
}
const httpMiddleware = createHttpMiddleware(options)
nock(testHost).get('/foo/bar').times(3).reply(500)

httpMiddleware(next)(request, response)
}))


test(
'should toggle `exponential backoff` off',
() =>
Expand Down
Loading