From b711d3eba7e8a946fb82904c5a9f958471c0b284 Mon Sep 17 00:00:00 2001 From: Erik Ritter Date: Mon, 20 Apr 2020 18:37:13 -0700 Subject: [PATCH] feat: add retry to callApi (#384) --- .../superset-ui-connection/package.json | 1 + .../src/SupersetClientClass.ts | 7 +++ .../src/callApi/callApi.ts | 8 +++- .../superset-ui-connection/src/constants.ts | 8 ++++ .../superset-ui-connection/src/types.ts | 8 ++++ .../test/callApi/callApi.test.ts | 44 ++++++++++++++++--- .../superset-ui/yarn.lock | 9 +++- 7 files changed, 77 insertions(+), 8 deletions(-) diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/package.json b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/package.json index 4d85bcf7f279..b11607caf626 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/package.json +++ b/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": { diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/SupersetClientClass.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/SupersetClientClass.ts index 7e0c635292dc..4fa52d245301 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/SupersetClientClass.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/SupersetClientClass.ts @@ -5,6 +5,7 @@ import { Credentials, CsrfPromise, CsrfToken, + FetchRetryOptions, Headers, Host, Mode, @@ -12,11 +13,13 @@ import { 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, diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/callApi/callApi.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/callApi/callApi.ts index 0fce0b0887e2..957be1563ee6 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/callApi/callApi.ts +++ b/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 { + 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); } diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/constants.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/constants.ts index 0428fc494e98..e856f4889866 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/constants.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/constants.ts @@ -1,3 +1,5 @@ +import { FetchRetryOptions } from './types'; + // HTTP status codes export const HTTP_STATUS_OK = 200; export const HTTP_STATUS_NOT_MODIFIED = 304; @@ -5,3 +7,9 @@ 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], +}; diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/types.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/types.ts index b986788403e5..0b4467ffd387 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/types.ts +++ b/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; diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/callApi/callApi.test.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/callApi/callApi.test.ts index 3847a4c98710..1d5fcd8db0f6 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/callApi/callApi.test.ts +++ b/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,6 +33,7 @@ describe('callApi()', () => { body: 'BODY', headers: { Etag: 'etag' }, }; + const mockErrorPayload = { status: 500, statusText: 'Internal error' }; fetchMock.get(mockGetUrl, mockGetPayload); fetchMock.post(mockPostUrl, mockPostPayload); @@ -37,6 +41,8 @@ describe('callApi()', () => { 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,14 +443,30 @@ 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); @@ -452,4 +474,16 @@ describe('callApi()', () => { 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); + }); }); diff --git a/superset-frontend/temporary_superset_ui/superset-ui/yarn.lock b/superset-frontend/temporary_superset_ui/superset-ui/yarn.lock index 7fb3edeade29..fee87fda46ce 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/yarn.lock +++ b/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"