ULMS-2596 Added timeout and retry logic

__tests__/basic-client.test.js

@@ -1,12 +1,15 @@
 import BasicClient from '../src/basic-client'
 
+const methodFunctionMock = jest.fn(() =>
+  Promise.resolve({ data: {}, status: 200 }),
+)
 const baseUrl = 'https://test.url'
 const httpClient = {
-  get: jest.fn(),
-  put: jest.fn(),
-  post: jest.fn(),
-  patch: jest.fn(),
-  delete: jest.fn(),
+  get: methodFunctionMock,
+  put: methodFunctionMock,
+  post: methodFunctionMock,
+  patch: methodFunctionMock,
+  delete: methodFunctionMock,
 }
 const tokenProvider = {
   getToken: jest.fn().mockImplementation(() => 'token'),

__tests__/retry.test.js

@@ -0,0 +1,58 @@
+import retry from '../src/retry'
+
+const resolvedData = {}
+const error = new Error('example error')
+const taskThatResolves = () => Promise.resolve(resolvedData)
+const taskThatRejects = () => Promise.reject(error)
+const taskThatThrows = () => {
+  throw error
+}
+const taskThatResolvesAfter = (n) => {
+  let count = 0
+
+  return () => {
+    count += 1
+
+    if (count < n) {
+      return taskThatRejects()
+    }
+
+    return taskThatResolves()
+  }
+}
+
+describe('retry', () => {
+  it('resolves when task resolves', async () => {
+    await expect(retry(taskThatResolves)).resolves.toBe(resolvedData)
+  })
+
+  it('rejects when task rejects', async () => {
+    await expect(retry(taskThatRejects)).rejects.toBe(error)
+  })
+
+  it('rejects when task throws', async () => {
+    await expect(retry(taskThatThrows)).rejects.toBe(error)
+  })
+
+  it('resolves when task resolves after 2 calls', async () => {
+    const task = taskThatResolvesAfter(2)
+
+    await expect(retry(task)).resolves.toBe(resolvedData)
+  })
+
+  it('onRetry have not been called', async () => {
+    const onRetry = jest.fn()
+
+    await expect(retry(taskThatResolves, onRetry)).resolves.toBe(resolvedData)
+
+    expect(onRetry).not.toHaveBeenCalled()
+  })
+
+  it('onRetry have been called 2 times', async () => {
+    const onRetry = jest.fn()
+
+    await expect(retry(taskThatRejects, onRetry)).rejects.toBe(error)
+
+    expect(onRetry).toHaveBeenCalledTimes(2)
+  })
+})

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@ulms/api-clients",
-  "version": "7.5.0",
+  "version": "7.5.0-dev.0-ulms-2596",
   "description": "JavaScript API clients for ULMS platform",
   "keywords": [],
   "homepage": "https://github.com/foxford/ulms-api-clients-js#readme",

src/basic-client.js

@@ -1,3 +1,7 @@
+import retry, { isErrorRetryable } from './retry'
+
+const onRetry = (error) => !isErrorRetryable(error)
+
 const parseParameter = (key, value) => {
   if (Array.isArray(value)) {
     // eslint-disable-next-line unicorn/no-array-reduce
@@ -22,6 +26,8 @@ const parseParameter = (key, value) => {
   return `${key}=${value}`
 }
 
+const responseTransformer = (_) => _.data
+
 class BasicClient {
   constructor(baseUrl, httpClient, tokenProvider) {
     this.baseUrl = baseUrl
@@ -86,14 +92,33 @@ class BasicClient {
   }
 
   async get(url, options = {}) {
+    const { retry: retryEnabled } = options
+
+    if (retryEnabled) {
+      const task = async () => {
+        const token = await this.tokenProvider.getToken()
+        const headers = {
+          ...options.headers,
+          ...BasicClient.headers(token, this.labels, this.customHeaders),
+        }
+        const requestOptions = { ...options, headers }
+
+        return this.httpClient
+          .get(url, requestOptions)
+          .then(responseTransformer)
+      }
+
+      return retry(task, onRetry)
+    }
+
     const token = await this.tokenProvider.getToken()
     const headers = {
       ...options.headers,
       ...BasicClient.headers(token, this.labels, this.customHeaders),
     }
     const requestOptions = { ...options, headers }
 
-    return this.httpClient.get(url, requestOptions)
+    return this.httpClient.get(url, requestOptions).then(responseTransformer)
   }
 
   async put(url, data, options = {}) {
@@ -104,7 +129,9 @@ class BasicClient {
     }
     const requestOptions = { ...options, headers }
 
-    return this.httpClient.put(url, data, requestOptions)
+    return this.httpClient
+      .put(url, data, requestOptions)
+      .then(responseTransformer)
   }
 
   async post(url, data, options = {}) {
@@ -122,7 +149,9 @@ class BasicClient {
       const expiresAtLocal = this.tokenProvider.tokenData
         ? this.tokenProvider.tokenData.expires_ts
         : undefined
-      const result = this.httpClient.post(url, data, requestOptions)
+      const result = this.httpClient
+        .post(url, data, requestOptions)
+        .then(responseTransformer)
 
       result.catch((error) => {
         const responseEnd = Date.now()
@@ -143,7 +172,9 @@ class BasicClient {
     }
     // [debug section] end
 
-    return this.httpClient.post(url, data, requestOptions)
+    return this.httpClient
+      .post(url, data, requestOptions)
+      .then(responseTransformer)
   }
 
   async patch(url, data, options = {}) {
@@ -154,7 +185,9 @@ class BasicClient {
     }
     const requestOptions = { ...options, headers }
 
-    return this.httpClient.patch(url, data, requestOptions)
+    return this.httpClient
+      .patch(url, data, requestOptions)
+      .then(responseTransformer)
   }
 
   async delete(url, options = {}) {
@@ -165,7 +198,7 @@ class BasicClient {
     }
     const requestOptions = { ...options, headers }
 
-    return this.httpClient.delete(url, requestOptions)
+    return this.httpClient.delete(url, requestOptions).then(responseTransformer)
   }
 }
 

src/http-client.js

@@ -1,65 +1,92 @@
 /* eslint-disable class-methods-use-this */
 
+function createTimeoutSignal(timeout) {
+  const controller = new AbortController()
+  const { signal } = controller
+  const id = setTimeout(() => controller.abort(), timeout)
+  const cleanup = () => clearTimeout(id)
+
+  return { cleanup, signal }
+}
+
 class FetchHttpClient {
-  static handleResponse(response) {
+  static async handleResponse(response) {
+    const bodyAsText = await response.text()
+    let data
+
+    try {
+      data = JSON.parse(bodyAsText)
+    } catch {
+      data = { message: bodyAsText }
+    }
+
+    const result = {
+      data,
+      status: response.status,
+    }
+
     if (!response.ok) {
-      return response
-        .json()
-        .catch(() => response.text())
-        .catch(() => ({
-          status: response.status,
-          statusText: response.statusText,
-        }))
-        .then((result) => {
-          throw result
-        })
+      throw result
+    }
+
+    return result
+  }
+
+  request(url, config) {
+    const { timeout, ...requestConfig } = config
+    const requestOptions = {
+      ...requestConfig,
+    }
+    let onFinally
+
+    if (timeout !== undefined) {
+      const { cleanup, signal } = createTimeoutSignal(timeout)
+
+      requestOptions.signal = signal
+      onFinally = cleanup
     }
 
-    return response
-      .json()
-      .catch(() => response.text())
-      .catch(() => ({
-        status: response.status,
-        statusText: response.statusText,
-      }))
+    return fetch(url, requestOptions)
+      .then(FetchHttpClient.handleResponse)
+      .finally(() => (onFinally ? onFinally() : undefined))
   }
 
   get(url, config) {
-    return fetch(url, {
+    return this.request(url, {
       ...config,
       method: 'GET',
-    }).then(FetchHttpClient.handleResponse)
+    })
   }
 
   put(url, data, config) {
-    return fetch(url, {
+    return this.request(url, {
       ...config,
       method: 'PUT',
       body: JSON.stringify(data),
-    }).then(FetchHttpClient.handleResponse)
+    })
   }
 
   post(url, data, config) {
-    return fetch(url, {
+    return this.request(url, {
       ...config,
       method: 'POST',
       body: JSON.stringify(data),
-    }).then(FetchHttpClient.handleResponse)
+    })
   }
 
   patch(url, data, config) {
-    return fetch(url, {
+    return this.request(url, {
       ...config,
       method: 'PATCH',
       body: JSON.stringify(data),
-    }).then(FetchHttpClient.handleResponse)
+    })
   }
 
   delete(url, config) {
-    return fetch(url, {
+    return this.request(url, {
       ...config,
       method: 'DELETE',
-    }).then(FetchHttpClient.handleResponse)
+    })
   }
 }
 

src/profile.js

@@ -1,3 +1,7 @@
+/* eslint-disable promise/no-nesting */
+const responseTransformer = (_) => _.data
+
+// todo: extend from BasicClient
 class HttpProfileResource {
   constructor(host, endpoint, httpClient, tokenProvider) {
     this.baseUrl = `${host}/${endpoint}`
@@ -25,29 +29,35 @@ class HttpProfileResource {
     }
 
     return this.tokenProvider.getToken().then((token) =>
-      this.httpClient.get(`${this.baseUrl}/users/${id}${qs}`, {
-        headers: HttpProfileResource.headers({ token }),
-      }),
+      this.httpClient
+        .get(`${this.baseUrl}/users/${id}${qs}`, {
+          headers: HttpProfileResource.headers({ token }),
+        })
+        .then(responseTransformer),
     )
   }
 
   listProfiles(ids, scope) {
     const qs = `?ids=${ids.join(',')}&scope=${scope}`
 
     return this.tokenProvider.getToken().then((token) =>
-      this.httpClient.get(`${this.baseUrl}/users${qs}`, {
-        headers: HttpProfileResource.headers({ token }),
-      }),
+      this.httpClient
+        .get(`${this.baseUrl}/users${qs}`, {
+          headers: HttpProfileResource.headers({ token }),
+        })
+        .then(responseTransformer),
     )
   }
 
   updateAttributes(id, scope, data) {
     const qs = `?scope=${scope}`
 
     return this.tokenProvider.getToken().then((token) =>
-      this.httpClient.patch(`${this.baseUrl}/users/${id}${qs}`, data, {
-        headers: HttpProfileResource.headers({ token }),
-      }),
+      this.httpClient
+        .patch(`${this.baseUrl}/users/${id}${qs}`, data, {
+          headers: HttpProfileResource.headers({ token }),
+        })
+        .then(responseTransformer),
     )
   }
 }