Merge pull request #1714 from commercetools/feat/add-retry-on-abort

Author: jenschude

docs/sdk/api/sdkMiddlewareHttp.md

@@ -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 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.
 
 #### Retrying requests
 
@@ -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,
     }),
   ],

packages/sdk-middleware-http/src/http.js

@@ -65,6 +65,7 @@ export default function createHttpMiddleware({
     backoff = true,
     retryDelay = 200,
     maxDelay = Infinity,
+    retryOnAbort = false,
   } = {},
   fetch: fetcher,
   abortController: _abortController,
@@ -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)
@@ -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)

packages/sdk-middleware-http/test/http.spec.js

@@ -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)
+    })
+  })
 })

types/sdk.js

@@ -241,6 +241,7 @@ export type HttpMiddlewareOptions = {
     retryDelay?: number,
     backoff?: boolean,
     maxDelay?: number,
+    retryOnAbort: boolean
   },
   fetch?: typeof fetch,
   /**