feat: add retry to callApi (#384)

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/package.json

@@ -32,6 +32,7 @@
   },
   "dependencies": {
     "@babel/runtime": "^7.1.2",
+    "fetch-retry": "^3.1.0",
     "whatwg-fetch": "^3.0.0"
   },
   "publishConfig": {

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/SupersetClientClass.ts

@@ -5,18 +5,21 @@ import {
   Credentials,
   CsrfPromise,
   CsrfToken,
+  FetchRetryOptions,
   Headers,
   Host,
   Mode,
   Protocol,
   RequestConfig,
   SupersetClientResponse,
 } from './types';
+import { DEFAULT_FETCH_RETRY_OPTIONS } from './constants';
 
 export default class SupersetClientClass {
   credentials: Credentials;
   csrfToken?: CsrfToken;
   csrfPromise?: CsrfPromise;
+  fetchRetryOptions?: FetchRetryOptions;
   protocol: Protocol;
   host: Host;
   headers: Headers;
@@ -27,6 +30,7 @@ export default class SupersetClientClass {
     protocol = 'http:',
     host = 'localhost',
     headers = {},
+    fetchRetryOptions = {},
     mode = 'same-origin',
     timeout,
     credentials = undefined,
@@ -40,6 +44,7 @@ export default class SupersetClientClass {
     this.credentials = credentials;
     this.csrfToken = csrfToken;
     this.csrfPromise = undefined;
+    this.fetchRetryOptions = { ...DEFAULT_FETCH_RETRY_OPTIONS, ...fetchRetryOptions };
 
     if (typeof this.csrfToken === 'string') {
       this.headers = { ...this.headers, 'X-CSRFToken': this.csrfToken };
@@ -80,6 +85,7 @@ export default class SupersetClientClass {
     body,
     credentials,
     endpoint,
+    fetchRetryOptions,
     headers,
     host,
     method,
@@ -95,6 +101,7 @@ export default class SupersetClientClass {
       callApi({
         body,
         credentials: credentials ?? this.credentials,
+        fetchRetryOptions,
         headers: { ...this.headers, ...headers },
         method,
         mode: mode ?? this.mode,

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/callApi/callApi.ts

@@ -1,4 +1,5 @@
 import 'whatwg-fetch';
+import fetchRetry from 'fetch-retry';
 import { CallApi } from '../types';
 import { CACHE_AVAILABLE, CACHE_KEY, HTTP_STATUS_NOT_MODIFIED, HTTP_STATUS_OK } from '../constants';
 
@@ -7,6 +8,7 @@ export default function callApi({
   body,
   cache = 'default',
   credentials = 'same-origin',
+  fetchRetryOptions,
   headers,
   method = 'GET',
   mode = 'same-origin',
@@ -16,6 +18,8 @@ export default function callApi({
   stringify = true,
   url,
 }: CallApi): Promise<Response> {
+  const fetchWithRetry = fetchRetry(fetch, fetchRetryOptions);
+
   const request = {
     body,
     cache,
@@ -43,7 +47,7 @@ export default function callApi({
             request.headers = { ...request.headers, 'If-None-Match': etag };
           }
 
-          return fetch(url, request);
+          return fetchWithRetry(url, request);
         })
         .then(response => {
           if (response.status === HTTP_STATUS_NOT_MODIFIED) {
@@ -82,5 +86,5 @@ export default function callApi({
     request.body = formData;
   }
 
-  return fetch(url, request);
+  return fetchWithRetry(url, request);
 }

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/constants.ts

@@ -1,7 +1,15 @@
+import { FetchRetryOptions } from './types';
+
 // HTTP status codes
 export const HTTP_STATUS_OK = 200;
 export const HTTP_STATUS_NOT_MODIFIED = 304;
 
 // Namespace for Cache API
 export const CACHE_AVAILABLE = 'caches' in self;
 export const CACHE_KEY = '@SUPERSET-UI/CONNECTION';
+
+export const DEFAULT_FETCH_RETRY_OPTIONS: FetchRetryOptions = {
+  retries: 3,
+  retryDelay: 1000,
+  retryOn: [503],
+};

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/types.ts

@@ -4,6 +4,11 @@ export type Body = RequestInit['body'];
 export type Cache = RequestInit['cache'];
 export type Credentials = RequestInit['credentials'];
 export type Endpoint = string;
+export type FetchRetryOptions = {
+  retries?: number;
+  retryDelay?: number | ((attempt: number, error: Error, response: Response) => number);
+  retryOn?: number[] | ((attempt: number, error: Error, response: Response) => boolean);
+};
 export type Headers = { [k: string]: string };
 export type Host = string;
 export type Json = { [k: string]: any };
@@ -21,6 +26,7 @@ export interface CallApi {
   body?: Body;
   cache?: Cache;
   credentials?: Credentials;
+  fetchRetryOptions?: FetchRetryOptions;
   headers?: Headers;
   method?: Method;
   mode?: Mode;
@@ -34,6 +40,7 @@ export interface CallApi {
 export interface RequestBase {
   body?: Body;
   credentials?: Credentials;
+  fetchRetryOptions?: FetchRetryOptions;
   headers?: Headers;
   host?: Host;
   mode?: Mode;
@@ -70,6 +77,7 @@ export type Protocol = 'http:' | 'https:';
 export interface ClientConfig {
   credentials?: Credentials;
   csrfToken?: CsrfToken;
+  fetchRetryOptions?: FetchRetryOptions;
   headers?: Headers;
   host?: Host;
   protocol?: Protocol;

superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/callApi/callApi.test.ts

@@ -6,6 +6,7 @@ import * as constants from '../../src/constants';
 import { LOGIN_GLOB } from '../fixtures/constants';
 import throwIfCalled from '../utils/throwIfCalled';
 import { CallApi } from '../../src/types';
+import { DEFAULT_FETCH_RETRY_OPTIONS } from '../../src/constants';
 
 describe('callApi()', () => {
   beforeAll(() => {
@@ -20,6 +21,8 @@ describe('callApi()', () => {
   const mockPatchUrl = '/mock/patch/url';
   const mockCacheUrl = '/mock/cache/url';
   const mockNotFound = '/mock/notfound';
+  const mockErrorUrl = '/mock/error/url';
+  const mock503 = '/mock/503';
 
   const mockGetPayload = { get: 'payload' };
   const mockPostPayload = { post: 'payload' };
@@ -30,13 +33,16 @@ describe('callApi()', () => {
     body: 'BODY',
     headers: { Etag: 'etag' },
   };
+  const mockErrorPayload = { status: 500, statusText: 'Internal error' };
 
   fetchMock.get(mockGetUrl, mockGetPayload);
   fetchMock.post(mockPostUrl, mockPostPayload);
   fetchMock.put(mockPutUrl, mockPutPayload);
   fetchMock.patch(mockPatchUrl, mockPatchPayload);
   fetchMock.get(mockCacheUrl, mockCachePayload);
   fetchMock.get(mockNotFound, { status: 404 });
+  fetchMock.get(mock503, { status: 503 });
+  fetchMock.get(mockErrorUrl, () => Promise.reject(mockErrorPayload));
 
   afterEach(fetchMock.reset);
 
@@ -437,19 +443,47 @@ describe('callApi()', () => {
     });
   });
 
-  it('rejects if the request throws', () => {
-    const mockErrorUrl = '/mock/error/url';
-    const mockErrorPayload = { status: 500, statusText: 'Internal error' };
-    fetchMock.get(mockErrorUrl, () => Promise.reject(mockErrorPayload));
+  it('rejects after retrying thrice if the request throws', () => {
+    expect.assertions(3);
 
+    return callApi({
+      fetchRetryOptions: DEFAULT_FETCH_RETRY_OPTIONS,
+      url: mockErrorUrl,
+      method: 'GET',
+    })
+      .then(throwIfCalled)
+      .catch(error => {
+        expect(fetchMock.calls(mockErrorUrl)).toHaveLength(4);
+        expect(error.status).toBe(mockErrorPayload.status);
+        expect(error.statusText).toBe(mockErrorPayload.statusText);
+      });
+  });
+
+  it('rejects without retries if the config is set to 0 retries', () => {
     expect.assertions(3);
 
-    return callApi({ url: mockErrorUrl, method: 'GET' })
+    return callApi({
+      fetchRetryOptions: { retries: 0 },
+      url: mockErrorUrl,
+      method: 'GET',
+    })
       .then(throwIfCalled)
       .catch(error => {
         expect(fetchMock.calls(mockErrorUrl)).toHaveLength(1);
         expect(error.status).toBe(mockErrorPayload.status);
         expect(error.statusText).toBe(mockErrorPayload.statusText);
       });
   });
+
+  it('rejects after retrying thrice if the request returns a 503', async () => {
+    const url = mock503;
+    const response = await callApi({
+      fetchRetryOptions: DEFAULT_FETCH_RETRY_OPTIONS,
+      url,
+      method: 'GET',
+    });
+    const calls = fetchMock.calls(url);
+    expect(calls).toHaveLength(4);
+    expect(response.status).toEqual(503);
+  });
 });

superset-frontend/temporary_superset_ui/superset-ui/yarn.lock

@@ -8001,7 +8001,7 @@ es5-shim@^4.5.13:
   resolved "https://registry.yarnpkg.com/es5-shim/-/es5-shim-4.5.14.tgz#90009e1019d0ea327447cb523deaff8fe45697ef"
   integrity sha512-7SwlpL+2JpymWTt8sNLuC2zdhhc+wrfe5cMPI2j0o6WsPdfAiPwmFy2f0AocPB4RQVBOZ9kNTgi5YF7TdhkvEg==
 
-es6-promise@^4.0.3:
+es6-promise@^4.0.3, es6-promise@^4.2.8:
   version "4.2.8"
   resolved "https://registry.yarnpkg.com/es6-promise/-/es6-promise-4.2.8.tgz#4eb21594c972bc40553d276e510539143db53e0a"
   integrity sha512-HJDGx5daxeIvxdBxvG2cb9g4tEvwIk3i8+nhX0yGrYmZUzbkdg8QbDevheDB8gd0//uPj4c1EQua8Q+MViT0/w==
@@ -8679,6 +8679,13 @@ fetch-mock@^7.2.5:
     path-to-regexp "^2.2.1"
     whatwg-url "^6.5.0"
 
+fetch-retry@^3.1.0:
+  version "3.1.0"
+  resolved "https://registry.yarnpkg.com/fetch-retry/-/fetch-retry-3.1.0.tgz#53a25652adf56def526f9b6f6d749a5bebffd319"
+  integrity sha512-pHCYCq7g854KkebphR3tKb4M7TJK91ZI0K2BU82cWv+vNkFQn0PZZFrQd/mL+Ra/mj2HLZNvzkTRjPEq2Dh/Bg==
+  dependencies:
+    es6-promise "^4.2.8"
+
 figgy-pudding@^3.4.1, figgy-pudding@^3.5.1:
   version "3.5.2"
   resolved "https://registry.yarnpkg.com/figgy-pudding/-/figgy-pudding-3.5.2.tgz#b4eee8148abb01dcf1d1ac34367d59e12fa61d6e"