fix: ACNA-2890 - fix error handling in library

lib/AdobeState.js

@@ -72,37 +72,41 @@ async function _wrap (promise, params) {
   try {
     response = await promise
     logger.debug('response', response)
-    // reuse code in exception handler, for any other network exceptions
-    if (!response.ok) {
-      // no exception on 404
-      if (response.status === 404) {
-        return null
-      } else {
-        const e = new Error(response.statusText)
-        e.status = response.status
-        e.internal = response
-        throw e
-      }
-    }
   } catch (e) {
-    const status = e.status || e.code
-    const copyParams = cloneDeep(params)
-    logger.debug(`got internal error with status ${status}: ${e.message} `)
-    switch (status) {
-      case 401:
-        return logAndThrow(new codes.ERROR_UNAUTHORIZED({ messageValues: ['underlying DB provider'], sdkDetails: copyParams }))
-      case 403:
-        return logAndThrow(new codes.ERROR_BAD_CREDENTIALS({ messageValues: ['underlying DB provider'], sdkDetails: copyParams }))
-      case 413:
-        return logAndThrow(new codes.ERROR_PAYLOAD_TOO_LARGE({ messageValues: ['underlying DB provider'], sdkDetails: copyParams }))
-      case 429:
-        return logAndThrow(new codes.ERROR_REQUEST_RATE_TOO_HIGH({ sdkDetails: copyParams }))
-      default:
-        // NOTE: we should throw a different error if its not a response error
-        return logAndThrow(new codes.ERROR_INTERNAL({ messageValues: [`unexpected response from provider with status: ${status}`], sdkDetails: { ...cloneDeep(params), _internal: e.internal || e.message } }))
-    }
+    logAndThrow(e)
+  }
+
+  return handleResponse(response, params)
+}
+
+/**
+ * Handle a network response.
+ *
+ * @param {Response} response a fetch Response
+ * @param {object} params the params to the network call
+ * @returns {void}
+ */
+async function handleResponse (response, params) {
+  if (response.ok) {
+    return response
+  }
+
+  const copyParams = cloneDeep(params)
+  switch (response.status) {
+    case 404:
+      // no exception on 404
+      return response
+    case 401:
+      return logAndThrow(new codes.ERROR_UNAUTHORIZED({ messageValues: ['underlying DB provider'], sdkDetails: copyParams }))
+    case 403:
+      return logAndThrow(new codes.ERROR_BAD_CREDENTIALS({ messageValues: ['underlying DB provider'], sdkDetails: copyParams }))
+    case 413:
+      return logAndThrow(new codes.ERROR_PAYLOAD_TOO_LARGE({ messageValues: ['underlying DB provider'], sdkDetails: copyParams }))
+    case 429:
+      return logAndThrow(new codes.ERROR_REQUEST_RATE_TOO_HIGH({ sdkDetails: copyParams }))
+    default: // 500 errors
+      return logAndThrow(new codes.ERROR_INTERNAL({ messageValues: [`unexpected response from provider with status: ${response.status} body: ${await response.text()}`], sdkDetails: copyParams }))
   }
-  return response
 }
 
 /**
@@ -293,7 +297,7 @@ class AdobeState {
     logger.debug('get', requestOptions)
     const promise = this.fetchRetry.exponentialBackoff(this.createRequestUrl(key), requestOptions)
     const response = await _wrap(promise, { key })
-    if (response) {
+    if (response.ok) {
       // we only expect string values
       const value = await response.text()
       const expiration = new Date(Number(response.headers.get(HEADER_KEY_EXPIRES))).toISOString()
@@ -357,7 +361,7 @@ class AdobeState {
    * Deletes a state key-value pair
    *
    * @param {string} key state key identifier
-   * @returns {Promise<string>} key of deleted state or `null` if state does not exists
+   * @returns {Promise<string>} key of deleted state or `null` if state does not exist
    * @memberof AdobeState
    */
   async delete (key) {
@@ -373,8 +377,12 @@ class AdobeState {
     logger.debug('delete', requestOptions)
 
     const promise = this.fetchRetry.exponentialBackoff(this.createRequestUrl(key), requestOptions)
-    const ret = await _wrap(promise, { key })
-    return ret && key
+    const response = await _wrap(promise, { key })
+    if (response.status === 404) {
+      return null
+    } else {
+      return key
+    }
   }
 
   /**
@@ -395,7 +403,7 @@ class AdobeState {
 
     const promise = this.fetchRetry.exponentialBackoff(this.createRequestUrl(), requestOptions)
     const response = await _wrap(promise, {})
-    return response !== null
+    return (response.status !== 404)
   }
 
   /**
@@ -416,7 +424,7 @@ class AdobeState {
 
     const promise = this.fetchRetry.exponentialBackoff(this.createRequestUrl(), requestOptions)
     const response = await _wrap(promise, {})
-    return response !== null
+    return (response.status !== 404)
   }
 
   /**
@@ -437,7 +445,11 @@ class AdobeState {
 
     const promise = this.fetchRetry.exponentialBackoff(this.createRequestUrl(), requestOptions)
     const response = await _wrap(promise, {})
-    return !!response && response.json()
+    if (response.status === 404) {
+      return false
+    } else {
+      return response.json()
+    }
   }
 }
 

test/AdobeState.test.js

@@ -55,14 +55,14 @@ const wrapInFetchResponse = (body, options = {}) => {
   }
 }
 
-const wrapInFetchError = (status) => {
+const wrapInFetchError = (status, body) => {
   return {
     ok: false,
     headers: {
       get: () => 'fake req id'
     },
-    json: async () => 'error',
-    text: async () => 'error',
+    json: async () => JSON.parse(body),
+    text: async () => body,
     status
   }
 }
@@ -253,19 +253,19 @@ describe('put', () => {
   test('coverage: unknown server error', async () => {
     const key = 'some-key'
     const value = 'some-value'
+    const responseBody = 'error: this is the response body'
 
-    mockExponentialBackoff.mockResolvedValue(wrapInFetchError(500))
-    await expect(store.put(key, value)).rejects.toThrow('[AdobeStateLib:ERROR_INTERNAL] unexpected response from provider with status: 500')
+    mockExponentialBackoff.mockResolvedValue(wrapInFetchError(500, responseBody))
+    await expect(store.put(key, value)).rejects.toThrow(`[AdobeStateLib:ERROR_INTERNAL] unexpected response from provider with status: 500 body: ${responseBody}`)
   })
 
   test('coverage: unknown error (fetch network failure)', async () => {
     const key = 'some-key'
     const value = 'some-value'
 
     const error = new Error('some network error')
-    error.code = 502
     mockExponentialBackoff.mockRejectedValue(error)
-    await expect(store.put(key, value)).rejects.toThrow('[AdobeStateLib:ERROR_INTERNAL] unexpected response from provider with status: 502')
+    await expect(store.put(key, value)).rejects.toThrow(error.message)
   })
 })
 
@@ -319,7 +319,7 @@ describe('deleteAll', () => {
   })
 })
 
-describe('any', () => {
+describe('stats()', () => {
   let store
 
   beforeEach(async () => {
@@ -342,7 +342,7 @@ describe('any', () => {
   })
 })
 
-describe('stats()', () => {
+describe('any', () => {
   let store
 
   beforeEach(async () => {