Fix issue with intermittent promise rejection error (#1859)

* fix(sdk-middleware-http): issue with intermittent promise rejection error - add an error catch block to handle serialization errors * Create khaki-radios-destroy.md * chore(tests): write tests - DEVX-180 write tests for new bug fix * chore(feedback): implement feedback - use correct error class for error handling. - change network error to server error
commercetools · Jul 3, 2023 · 0ffe724 · 0ffe724
1 parent 60ba4f8
commit 0ffe724
Show file tree

Hide file tree

Showing 3 changed files with 188 additions and 66 deletions.
diff --git a/.changeset/khaki-radios-destroy.md b/.changeset/khaki-radios-destroy.md
@@ -0,0 +1,5 @@
+---
+"@commercetools/sdk-middleware-http": patch
+---
+
+Fix intermittent unhandled promise rejection error
diff --git a/packages/sdk-middleware-http/src/http.js b/packages/sdk-middleware-http/src/http.js
@@ -10,7 +10,11 @@ import type {
   Next,
 } from 'types/sdk'
 
-import getErrorByCode, { NetworkError, HttpError } from './errors'
+import getErrorByCode, {
+  NetworkError,
+  HttpError,
+  InternalServerError,
+} from './errors'
 import parseHeaders from './parse-headers'
 
 function createError({ statusCode, message, ...rest }: Object): HttpErrorType {
@@ -35,9 +39,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
 }
@@ -122,10 +126,14 @@ export default function createHttpMiddleware({
         delete requestHeader['Content-Type']
       }
 
-      const body = (['application/json', 'application/graphql']
-        .indexOf(requestHeader['Content-Type']) > -1 && typeof request.body === 'string') ||
-        Buffer.isBuffer(request.body) ? request.body : JSON.stringify(request.body || undefined)
-
+      const body =
+        (['application/json', 'application/graphql'].indexOf(
+          requestHeader['Content-Type']
+        ) > -1 &&
+          typeof request.body === 'string') ||
+        Buffer.isBuffer(request.body)
+          ? request.body
+          : JSON.stringify(request.body || undefined)
 
       if (body && (typeof body === 'string' || Buffer.isBuffer(body))) {
         requestHeader['Content-Length'] = Buffer.byteLength(body).toString()
@@ -171,7 +179,6 @@ export default function createHttpMiddleware({
               fetchOptions.signal = abortController.signal
             }
           }
-
         }
         // Kick off timer for abortController directly before fetch.
         let timer
@@ -239,55 +246,81 @@ export default function createHttpMiddleware({
 
               // Server responded with an error. Try to parse it as JSON, then
               // return a proper error type with all necessary meta information.
-              res.text().then((text: any) => {
-                // Try to parse the error response as JSON
-                let parsed
-                try {
-                  parsed = JSON.parse(text)
-                } catch (error) {
-                  parsed = text
-                }
+              res
+                .text()
+                .then((text: any) => {
+                  // Try to parse the error response as JSON
+                  let parsed
+                  try {
+                    parsed = JSON.parse(text)
+                  } catch (error) {
+                    parsed = text
+                  }
 
-                const error: HttpErrorType = createError({
-                  statusCode: res.status,
-                  originalRequest: request,
-                  retryCount,
-                  headers: parseHeaders(res.headers),
-                  ...(typeof parsed === 'object'
-                    ? { message: parsed.message, body: parsed }
-                    : { message: parsed, body: parsed }),
-                })
+                  const error: HttpErrorType = createError({
+                    statusCode: res.status,
+                    originalRequest: request,
+                    retryCount,
+                    headers: parseHeaders(res.headers),
+                    ...(typeof parsed === 'object'
+                      ? { 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
+                  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
+                      retryCount += 1
+                      return
+                    }
                   }
-                }
 
-                maskAuthData(error.originalRequest, maskSensitiveHeaderData)
-                // Let the final resolver to reject the promise
-                const parsedResponse = {
-                  ...response,
-                  error,
-                  statusCode: res.status,
-                }
-                next(request, parsedResponse)
-              })
+                  maskAuthData(error.originalRequest, maskSensitiveHeaderData)
+                  // Let the final resolver to reject the promise
+                  const parsedResponse = {
+                    ...response,
+                    error,
+                    statusCode: res.status,
+                  }
+                  next(request, parsedResponse)
+                })
+                .catch((err: Error) => {
+                  if (enableRetry)
+                    if (retryCount < maxRetries) {
+                      setTimeout(
+                        executeFetch,
+                        calcDelayDuration(
+                          retryCount,
+                          retryDelay,
+                          maxRetries,
+                          backoff,
+                          maxDelay
+                        )
+                      )
+                      retryCount += 1
+                      return
+                    }
+
+                  const error = new InternalServerError(err.message, {
+                    originalRequest: request,
+                    retryCount,
+                  })
+                  maskAuthData(error.originalRequest, maskSensitiveHeaderData)
+                  next(request, { ...response, error, statusCode: 500 })
+                })
             },
             // We know that this is a "network" error thrown by the `fetch` library
             (e: Error) => {

diff --git a/packages/sdk-middleware-http/test/http.spec.js b/packages/sdk-middleware-http/test/http.spec.js
@@ -47,7 +47,11 @@ describe('Http', () => {
 
   test('throw when a non-array option is passed as retryCodes in the httpMiddlewareOptions', () => {
     expect(() => {
-      createHttpMiddleware({ host: testHost, retryConfig: { retryCodes: null }, fetch })
+      createHttpMiddleware({
+        host: testHost,
+        retryConfig: { retryCodes: null },
+        fetch,
+      })
     }).toThrow(
       new Error(
         '`retryCodes` option must be an array of retry status (error) codes.'
@@ -707,9 +711,7 @@ describe('Http', () => {
           retryConfig: {
             maxRetries: 2,
             retryDelay: 300,
-            retryCodes: [
-              500, 501, 502
-            ],
+            retryCodes: [500, 501, 502],
           },
           fetch,
         }
@@ -739,15 +741,13 @@ describe('Http', () => {
           retryConfig: {
             maxRetries: 2,
             retryDelay: 300,
-            retryCodes: [
-              'ETIMEDOUT', 'ECONNREFUSED', 'write EPIPE'
-            ]
+            retryCodes: ['ETIMEDOUT', 'ECONNREFUSED', 'write EPIPE'],
           },
           fetch,
         })
         nock(testHost)
           .defaultReplyHeaders({
-            'Content-Type': 'application/json'
+            'Content-Type': 'application/json',
           })
           .persist()
           .get('/foo/bar')
@@ -776,15 +776,13 @@ describe('Http', () => {
           retryConfig: {
             maxRetries: 2,
             retryDelay: 300,
-            retryCodes: [
-              'ETIMEDOUT', 503, 502, 'ECONNREFUSED', 'write EPIPE'
-            ]
+            retryCodes: ['ETIMEDOUT', 503, 502, 'ECONNREFUSED', 'write EPIPE'],
           },
           fetch,
         })
         nock(testHost)
           .defaultReplyHeaders({
-            'Content-Type': 'application/json'
+            'Content-Type': 'application/json',
           })
           .persist()
           .get('/foo/bar')
@@ -886,7 +884,6 @@ describe('Http', () => {
         httpMiddleware(next)(request, response)
       }))
 
-
     test(
       'should toggle `exponential backoff` off',
       () =>
@@ -965,6 +962,93 @@ describe('Http', () => {
       }))
   })
 
+  describe('promise rejection error', () => {
+    test('handle failed response - (server error)', () =>
+      new Promise((resolve, reject) => {
+        const request = createTestRequest({
+          uri: '/foo/bar',
+        })
+        const response = { resolve, reject }
+        const next = (req, res) => {
+          const expectedError = new Error('oops')
+          expectedError.body = {
+            message: 'oops',
+          }
+          expectedError.code = 500
+          expectedError.statusCode = 500
+          expectedError.headers = {
+            'content-type': 'application/json',
+          }
+
+          expect(res.error.statusCode).toEqual(0)
+          expect(res.error.retryCount).toEqual(2)
+          expect(res.error.name).toEqual('NetworkError')
+
+          resolve()
+        }
+        const httpMiddleware = createHttpMiddleware({
+          host: testHost,
+          enableRetry: true,
+          credentialsMode: 'include',
+          retryConfig: {
+            maxRetries: 2,
+            retryDelay: 300,
+          },
+          fetch,
+        })
+        nock(testHost)
+          .defaultReplyHeaders({
+            'Content-Type': 'application/json',
+          })
+          .get('/foo/bar')
+          .reply(500, { message: 'Internal server error' })
+
+        httpMiddleware(next)(request, { ...response })
+      }))
+
+    test('handle failed response - (server error) - ', () =>
+      new Promise((resolve, reject) => {
+        const request = createTestRequest({
+          uri: '/foo/bar',
+        })
+        const response = { resolve, reject }
+        const next = (req, res) => {
+          const expectedError = new Error('oops')
+          expectedError.body = {
+            message: 'oops',
+          }
+          expectedError.code = 0
+          expectedError.statusCode = 0
+          expectedError.headers = {
+            'content-type': 'application/json',
+          }
+
+          expect(res.error.code).toEqual(500)
+          expect(res.error.name).toEqual('InternalServerError')
+
+          resolve()
+        }
+        const httpMiddleware = createHttpMiddleware({
+          host: testHost,
+          enableRetry: false,
+          credentialsMode: 'include',
+          retryConfig: {
+            maxRetries: 2,
+            retryDelay: 300,
+          },
+          fetch,
+        })
+        nock(testHost)
+          .defaultReplyHeaders({
+            'Content-Type': 'application/json',
+          })
+          .get('/foo/bar')
+          .reply(400, { message: 'Internal server error' })
+
+        httpMiddleware(next)(request, { ...response, text: (e) => reject(e) })
+      }))
+  })
+
   test('handle failed response (api error)', () =>
     new Promise((resolve, reject) => {
       const request = createTestRequest({