Merge pull request #498 from contentstack/fix/run-plugin-onresponse-on-error

Enhance ConcurrencyQueue to support plugin error handling and update …

Author: harshithad0703

.talismanrc

@@ -9,7 +9,7 @@ fileignoreconfig:
     ignore_detectors:
       - filecontent
   - filename: package-lock.json
-    checksum: d6e0739053d0068a31f75ef462d9e80a68a165d7715115cc2f33cdf65d9084dd
+    checksum: 751efa34d2f832c7b99771568b5125d929dab095784b6e4ea659daaa612994c8
   - filename: .husky/pre-commit
     checksum: 52a664f536cf5d1be0bea19cb6031ca6e8107b45b6314fe7d47b7fad7d800632
   - filename: test/sanity-check/api/user-test.js

lib/core/concurrency-queue.js

@@ -52,7 +52,7 @@ const defaultConfig = {
  * @returns {Object} ConcurrencyQueue instance with request/response interceptors attached to Axios.
  * @throws {Error} If axios instance is not provided or configuration is invalid.
  */
-export function ConcurrencyQueue ({ axios, config }) {
+export function ConcurrencyQueue ({ axios, config, plugins = [] }) {
   if (!axios) {
     throw Error(ERROR_MESSAGES.AXIOS_INSTANCE_MISSING)
   }
@@ -436,6 +436,30 @@ export function ConcurrencyQueue ({ axios, config }) {
     return response
   }
 
+  // Run plugin onResponse hooks for errors so plugins see every error (including
+  // those that are rejected without going through the plugin response interceptor).
+  const runPluginOnResponseForError = (error) => {
+    if (!plugins || !plugins.length) return error
+    let currentError = error
+    for (const plugin of plugins) {
+      try {
+        if (typeof plugin.onResponse === 'function') {
+          const result = plugin.onResponse(currentError)
+          if (result !== undefined) currentError = result
+        }
+      } catch (e) {
+        if (this.config.logHandler) {
+          this.config.logHandler('error', {
+            name: 'PluginError',
+            message: `Error in plugin onResponse (error handler): ${e.message}`,
+            error: e
+          })
+        }
+      }
+    }
+    return currentError
+  }
+
   const responseErrorHandler = error => {
     let networkError = error.config.retryCount
     let retryErrorType = null
@@ -449,7 +473,8 @@ export function ConcurrencyQueue ({ axios, config }) {
 
     // Original retry logic for non-network errors
     if (!this.config.retryOnError || networkError > this.config.retryLimit) {
-      return Promise.reject(responseHandler(error))
+      const err = runPluginOnResponseForError(error)
+      return Promise.reject(responseHandler(err))
     }
 
     // Check rate limit remaining header before retrying
@@ -465,20 +490,23 @@ export function ConcurrencyQueue ({ axios, config }) {
         }
         response = error.response
       } else {
-        return Promise.reject(responseHandler(error))
+        const err = runPluginOnResponseForError(error)
+        return Promise.reject(responseHandler(err))
       }
     } else if ((response.status === 401 && this.config.refreshToken)) {
       // If error_code is 294 (2FA required), don't retry/refresh - pass through the error as-is
       const apiErrorCode = response.data?.error_code
       if (apiErrorCode === 294) {
-        return Promise.reject(error)
+        const err = runPluginOnResponseForError(error)
+        return Promise.reject(responseHandler(err))
       }
 
       retryErrorType = `Error with status: ${response.status}`
       networkError++
 
       if (networkError > this.config.retryLimit) {
-        return Promise.reject(responseHandler(error))
+        const err = runPluginOnResponseForError(error)
+        return Promise.reject(responseHandler(err))
       }
       this.running.shift()
       // Cool down the running requests
@@ -492,19 +520,22 @@ export function ConcurrencyQueue ({ axios, config }) {
       networkError++
       return this.retry(error, retryErrorType, networkError, wait)
     }
-    return Promise.reject(responseHandler(error))
+    const err = runPluginOnResponseForError(error)
+    return Promise.reject(responseHandler(err))
   }
 
   this.retry = (error, retryErrorType, retryCount, waittime) => {
     let delaytime = waittime
     if (retryCount > this.config.retryLimit) {
-      return Promise.reject(responseHandler(error))
+      const err = runPluginOnResponseForError(error)
+      return Promise.reject(responseHandler(err))
     }
     if (this.config.retryDelayOptions) {
       if (this.config.retryDelayOptions.customBackoff) {
         delaytime = this.config.retryDelayOptions.customBackoff(retryCount, error)
         if (delaytime && delaytime <= 0) {
-          return Promise.reject(responseHandler(error))
+          const err = runPluginOnResponseForError(error)
+          return Promise.reject(responseHandler(err))
         }
       } else if (this.config.retryDelayOptions.base) {
         delaytime = this.config.retryDelayOptions.base * retryCount

lib/core/contentstackHTTPClient.js

@@ -109,10 +109,11 @@ export default function contentstackHttpClient (options) {
   }
   const instance = axios.create(axiosOptions)
   instance.httpClientParams = options
-  instance.concurrencyQueue = new ConcurrencyQueue({ axios: instance, config })
 
-  // Normalize and store plugins
-  const plugins = normalizePlugins(config.plugins)
+  // Normalize and store plugins before ConcurrencyQueue so plugin interceptors
+  // run after the queue's (plugin sees responses/errors before they reach the queue).
+  // Use options.plugins so hooks run against the same plugin references (spies work in tests).
+  const plugins = normalizePlugins(options.plugins || config.plugins)
 
   // Request interceptor for versioning strategy (must run first)
   instance.interceptors.request.use((request) => {
@@ -235,5 +236,7 @@ export default function contentstackHttpClient (options) {
     )
   }
 
+  instance.concurrencyQueue = new ConcurrencyQueue({ axios: instance, config, plugins })
+
   return instance
 }

package-lock.json

@@ -251,11 +251,13 @@ describe('Contentstack HTTP Client', () => {
     })
 
     it('should call onResponse hook after error response', (done) => {
-      const onResponseSpy = sinon.spy()
+      let onResponseCalled = false
+      let receivedError = null
       const plugin = {
         onRequest: () => {},
         onResponse: (error) => {
-          onResponseSpy(error)
+          onResponseCalled = true
+          receivedError = error
           return error
         }
       }
@@ -275,11 +277,13 @@ describe('Contentstack HTTP Client', () => {
       axiosInstance.get('/test').catch(() => {
         // Plugin should be called for the error
         // eslint-disable-next-line no-unused-expressions
-        expect(onResponseSpy.called).to.be.true
-        if (onResponseSpy.called) {
-          // eslint-disable-next-line no-unused-expressions
-          expect(onResponseSpy.calledWith(sinon.match.object)).to.be.true
-        }
+        expect(onResponseCalled).to.be.true
+        // eslint-disable-next-line no-unused-expressions
+        expect(receivedError).to.exist
+        // eslint-disable-next-line no-unused-expressions
+        expect(receivedError.response).to.exist
+        // eslint-disable-next-line no-unused-expressions
+        expect(receivedError.response.status).to.equal(500)
         done()
       }).catch((err) => {
         // Ensure done is called even if there's an unexpected error