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

Retry Aborted Requests #1714

Merged
merged 4 commits into from
Oct 26, 2021
Merged
Show file tree
Hide file tree
Changes from 3 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: 6 additions & 3 deletions docs/sdk/api/sdkMiddlewareHttp.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,10 @@ The HTTP middleware can run in either a browser or Node.js environment. For Node
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)
11. `maxDelay` _(Number)_: The maximum duration (milliseconds) to wait before retrying, useful if the delay time grew exponentially more than reasonable
12. `fetch` _(Function)_: A `fetch` implementation which can be e.g. `node-fetch` or `unfetch` but also the native browser `fetch` function
13. `timeout` _(Number)_: Request/response timeout in ms. Must have globally available or passed in `AbortController`
14. `abortController` or `getAbortController` depending on 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 globally available one.
12. `retryOnAbort` _(Boolean)_: Configure the client to retry an aborted request or not. Defaults to false.
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 have globally available or passed in `AbortController`
jenschude marked this conversation as resolved.
Show resolved Hide resolved
15. `abortController` or `getAbortController` depending on 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 globally available one.
jenschude marked this conversation as resolved.
Show resolved Hide resolved

#### Retrying requests

Expand Down Expand Up @@ -70,9 +71,11 @@ const client = createClient({
maxRetries: 2,
retryDelay: 300, //milliseconds
maxDelay: 5000, //milliseconds
retryOnAbort: false,
},

// Optional if not globally available
timeout: 1000,
fetch,
}),
],
Expand Down
35 changes: 26 additions & 9 deletions packages/sdk-middleware-http/src/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export default function createHttpMiddleware({
backoff = true,
retryDelay = 200,
maxDelay = Infinity,
retryOnAbort = false,
} = {},
fetch: fetcher,
abortController: _abortController,
Expand Down Expand Up @@ -98,13 +99,6 @@ export default function createHttpMiddleware({
response: MiddlewareResponse
) => {
let abortController: any
if (timeout || getAbortController || _abortController)
// eslint-disable-next-line
abortController =
(getAbortController ? getAbortController() : null) ||
_abortController ||
new AbortController()

const url = host.replace(/\/$/, '') + request.uri
const body =
typeof request.body === 'string' || Buffer.isBuffer(request.body)
Expand All @@ -126,15 +120,38 @@ export default function createHttpMiddleware({
if (credentialsMode) {
fetchOptions.credentials = credentialsMode
}
if (abortController) {
fetchOptions.signal = abortController.signal

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

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

if (body) {
fetchOptions.body = body
}
let retryCount = 0
// wrap in a fn so we can retry if error occur
function executeFetch() {
if (retryOnAbort) {
if (timeout || getAbortController || _abortController)
// eslint-disable-next-line
abortController =
(getAbortController ? getAbortController() : null) ||
_abortController ||
new AbortController()

if (abortController) {
fetchOptions.signal = abortController.signal
}
}
// Kick off timer for abortController directly before fetch.
let timer
if (timeout)
Expand Down
128 changes: 128 additions & 0 deletions packages/sdk-middleware-http/test/http.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1116,4 +1116,132 @@ describe('Http', () => {

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

test('should retry when request is aborted (success)', () => {
expect.assertions(1)
return new Promise((resolve, reject) => {
const request = createTestRequest({
uri: '/foo/bar',
})
const response = { resolve, reject }
const next = (req, res) => {
expect(res).toEqual({
...response,
body: { foo: 'bar' },
statusCode: 200,
})
resolve()
}
// Use default options
const httpMiddleware = createHttpMiddleware({
host: testHost,
timeout: 100,
fetch,
enableRetry: true,
retryConfig: {
retryOnAbort: true,
},
getAbortController: () => new AbortController(),
})
nock(testHost)
.defaultReplyHeaders({
'Content-Type': 'application/json',
})
.get('/foo/bar')
.once()
.delay(200) // delay response to fail
.reply(200, { foo: 'bar' })
.get('/foo/bar')
.delay(50) // delay lower then timeout
.reply(200, { foo: 'bar' })

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

test('should retry when request is aborted (fail)', () => {
expect.assertions(1)
return new Promise((resolve, reject) => {
const request = createTestRequest({
uri: '/foo/bar',
})
const response = { resolve, reject }
const next = (req, res) => {
expect(res).toEqual({
...response,
error: expect.any(Error),
statusCode: 0,
})
resolve()
}
// Use default options
const httpMiddleware = createHttpMiddleware({
host: testHost,
timeout: 100,
fetch,
enableRetry: true,
retryConfig: {
maxRetries: 2,
retryOnAbort: false,
},
getAbortController: () => new AbortController(),
})
nock(testHost)
.defaultReplyHeaders({
'Content-Type': 'application/json',
})
.get('/foo/bar')
.once()
.delay(200) // delay response to fail
.reply(200, { foo: 'bar' })
.get('/foo/bar')
.delay(150) // delay higher then timeout
.reply(200, { foo: 'bar' })

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

test('should retry when requests are aborted (fail)', () => {
expect.assertions(1)
return new Promise((resolve, reject) => {
const request = createTestRequest({
uri: '/foo/bar',
})
const response = { resolve, reject }
const next = (req, res) => {
expect(res).toEqual({
...response,
error: expect.any(Error),
statusCode: 0,
})
resolve()
}
// Use default options
const httpMiddleware = createHttpMiddleware({
host: testHost,
timeout: 100, // time out after 10ms
fetch,
enableRetry: true,
retryConfig: {
maxRetries: 1,
retryOnAbort: true,
},
getAbortController: () => new AbortController(),
})
nock(testHost)
.defaultReplyHeaders({
'Content-Type': 'application/json',
})
.get('/foo/bar')
.once()
.delay(150) // delay response to fail (higher than timeout)
.reply(200, { foo: 'bar' })
.get('/foo/bar')
.delay(150) // delay response to fail (higher than timeout)
.reply(200, { foo: 'bar' })

httpMiddleware(next)(request, response)
})
})
})
1 change: 1 addition & 0 deletions types/sdk.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ export type HttpMiddlewareOptions = {
retryDelay?: number,
backoff?: boolean,
maxDelay?: number,
retryOnAbort: boolean
},
fetch?: typeof fetch,
/**
Expand Down