Skip to content
This repository has been archived by the owner on Dec 10, 2021. It is now read-only.

Commit

Permalink
feat: add retry to callApi
Browse files Browse the repository at this point in the history
  • Loading branch information
erik_ritter committed Apr 21, 2020
1 parent fb9cce3 commit d5e6de2
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 8 deletions.
1 change: 1 addition & 0 deletions packages/superset-ui-connection/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
},
"dependencies": {
"@babel/runtime": "^7.1.2",
"fetch-retry": "^3.1.0",
"whatwg-fetch": "^3.0.0"
},
"publishConfig": {
Expand Down
7 changes: 7 additions & 0 deletions packages/superset-ui-connection/src/SupersetClientClass.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -27,6 +30,7 @@ export default class SupersetClientClass {
protocol = 'http:',
host = 'localhost',
headers = {},
fetchRetryOptions = DEFAULT_FETCH_RETRY_OPTIONS,
mode = 'same-origin',
timeout,
credentials = undefined,
Expand All @@ -40,6 +44,7 @@ export default class SupersetClientClass {
this.credentials = credentials;
this.csrfToken = csrfToken;
this.csrfPromise = undefined;
this.fetchRetryOptions = fetchRetryOptions;

if (typeof this.csrfToken === 'string') {
this.headers = { ...this.headers, 'X-CSRFToken': this.csrfToken };
Expand Down Expand Up @@ -80,6 +85,7 @@ export default class SupersetClientClass {
body,
credentials,
endpoint,
fetchRetryOptions,
headers,
host,
method,
Expand All @@ -95,6 +101,7 @@ export default class SupersetClientClass {
callApi({
body,
credentials: credentials ?? this.credentials,
fetchRetryOptions,
headers: { ...this.headers, ...headers },
method,
mode: mode ?? this.mode,
Expand Down
8 changes: 6 additions & 2 deletions packages/superset-ui-connection/src/callApi/callApi.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -7,6 +8,7 @@ export default function callApi({
body,
cache = 'default',
credentials = 'same-origin',
fetchRetryOptions,
headers,
method = 'GET',
mode = 'same-origin',
Expand All @@ -16,6 +18,8 @@ export default function callApi({
stringify = true,
url,
}: CallApi): Promise<Response> {
const fetchWithRetry = fetchRetry(fetch, fetchRetryOptions);

const request = {
body,
cache,
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -82,5 +86,5 @@ export default function callApi({
request.body = formData;
}

return fetch(url, request);
return fetchWithRetry(url, request);
}
8 changes: 8 additions & 0 deletions packages/superset-ui-connection/src/constants.ts
Original file line number Diff line number Diff line change
@@ -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],
};
8 changes: 8 additions & 0 deletions packages/superset-ui-connection/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand All @@ -21,6 +26,7 @@ export interface CallApi {
body?: Body;
cache?: Cache;
credentials?: Credentials;
fetchRetryOptions?: FetchRetryOptions;
headers?: Headers;
method?: Method;
mode?: Mode;
Expand All @@ -34,6 +40,7 @@ export interface CallApi {
export interface RequestBase {
body?: Body;
credentials?: Credentials;
fetchRetryOptions?: FetchRetryOptions;
headers?: Headers;
host?: Host;
mode?: Mode;
Expand Down Expand Up @@ -70,6 +77,7 @@ export type Protocol = 'http:' | 'https:';
export interface ClientConfig {
credentials?: Credentials;
csrfToken?: CsrfToken;
fetchRetryOptions?: FetchRetryOptions;
headers?: Headers;
host?: Host;
protocol?: Protocol;
Expand Down
44 changes: 39 additions & 5 deletions packages/superset-ui-connection/test/callApi/callApi.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand All @@ -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' };
Expand All @@ -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);

Expand Down Expand Up @@ -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);
});
});
9 changes: 8 additions & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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==
Expand Down Expand Up @@ -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"
Expand Down

0 comments on commit d5e6de2

Please sign in to comment.