task: Use make-fetch-happen (#1500)

closes #739

package.json

@@ -52,9 +52,6 @@
   "jest": {
     "automock": false,
     "maxWorkers": 4,
-    "setupFiles": [
-      "./setupJest.js"
-    ],
     "transform": {
       "^.+\\.tsx?$": "ts-jest"
     },
@@ -101,11 +98,11 @@
     "knex": "1.0.4",
     "json-schema-to-ts": "^2.0.0",
     "log4js": "^6.0.0",
+    "make-fetch-happen": "^10.1.2",
     "memoizee": "^0.4.15",
     "mime": "^3.0.0",
     "multer": "^1.4.1",
     "mustache": "^4.1.0",
-    "node-fetch": "^2.6.7",
     "nodemailer": "^6.5.0",
     "openapi-types": "^11.0.0",
     "owasp-password-strength-test": "^1.3.0",
@@ -131,10 +128,10 @@
     "@types/faker": "5.5.9",
     "@types/jest": "27.4.1",
     "@types/js-yaml": "4.0.5",
+    "@types/make-fetch-happen": "^9.0.2",
     "@types/memoizee": "0.4.7",
     "@types/mime": "2.0.3",
     "@types/node": "16.6.1",
-    "@types/node-fetch": "2.6.1",
     "@types/nodemailer": "6.4.4",
     "@types/owasp-password-strength-test": "1.3.0",
     "@types/stoppable": "1.1.1",
@@ -156,8 +153,8 @@
     "fetch-mock": "9.11.0",
     "husky": "7.0.4",
     "jest": "27.5.1",
-    "jest-fetch-mock": "3.0.3",
-    "lint-staged": "12.4.1",
+    "lint-staged": "12.3.7",
+    "nock": "^13.2.4",
     "prettier": "2.6.2",
     "proxyquire": "2.1.3",
     "source-map-support": "0.5.21",

src/lib/addons/addon.test.ts

@@ -1,40 +1,71 @@
-import fetchMock from 'jest-fetch-mock';
+import nock from 'nock';
 import noLogger from '../../test/fixtures/no-logger';
 
 import SlackAddon from './slack';
 
 beforeEach(() => {
-    fetchMock.resetMocks();
+    nock.disableNetConnect();
 });
 
-test('Retries if fetch throws', async () => {
+test('Does not retry if request succeeds', async () => {
     const url = 'https://test.some.com';
-    jest.useFakeTimers('modern');
     const addon = new SlackAddon({
         getLogger: noLogger,
         unleashUrl: url,
     });
-    fetchMock.mockIf('https://test.some.com', 'OK', {
-        status: 201,
-        statusText: 'ACCEPTED',
+    nock(url).get('/').reply(201);
+    const res = await addon.fetchRetry(url);
+    expect(res.ok).toBe(true);
+});
+
+test('Retries once, and succeeds', async () => {
+    const url = 'https://test.some.com';
+    const addon = new SlackAddon({
+        getLogger: noLogger,
+        unleashUrl: url,
     });
-    await addon.fetchRetry(url);
-    jest.advanceTimersByTime(1000);
-    jest.useRealTimers();
+    nock(url).get('/').replyWithError('testing retry');
+    nock(url).get('/').reply(200);
+    const res = await addon.fetchRetry(url);
+    expect(res.ok).toBe(true);
+    expect(nock.isDone()).toBe(true);
 });
 
-test('does not crash even if fetch throws', async () => {
+test('Does not throw if response is error', async () => {
     const url = 'https://test.some.com';
-    jest.useFakeTimers('modern');
     const addon = new SlackAddon({
         getLogger: noLogger,
         unleashUrl: url,
     });
-    fetchMock.mockResponse(() => {
-        throw new Error('Network error');
+    nock(url).get('/').twice().replyWithError('testing retry');
+    const res = await addon.fetchRetry(url);
+    expect(res.ok).toBe(false);
+});
+
+test('Supports custom number of retries', async () => {
+    const url = 'https://test.some.com';
+    const addon = new SlackAddon({
+        getLogger: noLogger,
+        unleashUrl: url,
     });
-    await addon.fetchRetry(url);
-    jest.advanceTimersByTime(1000);
-    expect(fetchMock.mock.calls).toHaveLength(2);
-    jest.useRealTimers();
+    let retries = 0;
+    nock(url).get('/').twice().replyWithError('testing retry');
+    nock(url).get('/').reply(201);
+    const res = await addon.fetchRetry(
+        url,
+        {
+            onRetry: () => {
+                retries = retries + 1;
+            },
+        },
+        2,
+    );
+
+    expect(retries).toBe(2);
+    expect(res.ok).toBe(true);
+    expect(nock.isDone()).toBe(true);
+});
+
+afterEach(() => {
+    nock.enableNetConnect();
 });

src/lib/addons/addon.ts

@@ -1,4 +1,5 @@
-import fetch, { Response } from 'node-fetch';
+import { Response } from 'node-fetch';
+import fetch from 'make-fetch-happen';
 import { addonDefinitionSchema } from './addon-schema';
 import { IUnleashConfig } from '../types/option';
 import { Logger } from '../logger';
@@ -41,23 +42,18 @@ export default abstract class Addon {
         url: string,
         options = {},
         retries: number = 1,
-        backoff: number = 300,
     ): Promise<Response> {
-        const retryCodes = [408, 500, 502, 503, 504, 522, 524];
         let res;
         try {
-            res = await fetch(url, options);
-        } catch (error) {
-            res = { status: 500, ok: false };
-        }
-        if (res.ok) {
+            res = await fetch(url, {
+                retry: {
+                    retries,
+                },
+                ...options,
+            });
             return res;
-        }
-        if (retries > 0 && retryCodes.includes(res.status)) {
-            setTimeout(
-                () => this.fetchRetry(url, options, retries - 1, backoff * 2),
-                backoff,
-            );
+        } catch (e) {
+            res = { statusCode: e.code, ok: false };
         }
         return res;
     }

src/lib/services/version-service.test.ts

@@ -1,12 +1,16 @@
-import fetchMock from 'jest-fetch-mock';
+import nock from 'nock';
 import createStores from '../../test/fixtures/store';
 import version from '../util/version';
 import getLogger from '../../test/fixtures/no-logger';
 
 import VersionService from './version-service';
 
 beforeEach(() => {
-    fetchMock.resetMocks();
+    nock.disableNetConnect();
+});
+
+afterEach(() => {
+    nock.enableNetConnect();
 });
 
 test('yields current versions', async () => {
@@ -17,13 +21,15 @@ test('yields current versions', async () => {
         oss: '5.0.0',
         enterprise: '5.0.0',
     };
-    fetchMock.mockResponse(
-        JSON.stringify({
-            latest: false,
-            versions: latest,
-        }),
-        { status: 200 },
-    );
+    nock(testurl)
+        .post('/')
+        .reply(() => [
+            200,
+            JSON.stringify({
+                latest: false,
+                versions: latest,
+            }),
+        ]);
     const service = new VersionService(
         { settingStore },
         {
@@ -50,13 +56,15 @@ test('supports setting enterprise version as well', async () => {
         oss: '4.0.0',
         enterprise: '4.0.0',
     };
-    fetchMock.mockResponse(
-        JSON.stringify({
-            latest: false,
-            versions: latest,
-        }),
-        { status: 200 },
-    );
+    nock(testurl)
+        .post('/')
+        .reply(() => [
+            200,
+            JSON.stringify({
+                latest: false,
+                versions: latest,
+            }),
+        ]);
 
     const service = new VersionService(
         { settingStore },
@@ -85,13 +93,15 @@ test('if version check is not enabled should not make any calls', async () => {
         oss: '4.0.0',
         enterprise: '4.0.0',
     };
-    fetchMock.mockResponse(
-        JSON.stringify({
-            latest: false,
-            versions: latest,
-        }),
-        { status: 200 },
-    );
+    const scope = nock(testurl)
+        .get('/')
+        .reply(() => [
+            200,
+            JSON.stringify({
+                latest: false,
+                versions: latest,
+            }),
+        ]);
 
     const service = new VersionService(
         { settingStore },
@@ -103,11 +113,12 @@ test('if version check is not enabled should not make any calls', async () => {
     );
     await service.checkLatestVersion();
     const versionInfo = service.getVersionInfo();
-    expect(fetchMock.mock.calls).toHaveLength(0);
+    expect(scope.isDone()).toBeFalsy();
     expect(versionInfo.current.oss).toBe(version);
     expect(versionInfo.current.enterprise).toBe(enterpriseVersion);
     // @ts-ignore
     expect(versionInfo.latest.oss).toBeFalsy();
     // @ts-ignore
     expect(versionInfo.latest.enterprise).toBeFalsy();
+    nock.cleanAll();
 });

src/lib/services/version-service.ts

@@ -1,4 +1,4 @@
-import fetch from 'node-fetch';
+import fetch from 'make-fetch-happen';
 import { IUnleashStores } from '../types/stores';
 import { IUnleashConfig } from '../types/option';
 import version from '../util/version';

src/lib/util/load-index-html.ts

@@ -2,7 +2,7 @@ import fs from 'fs';
 import { IUnleashConfig } from '../server-impl';
 import { rewriteHTML } from './rewriteHTML';
 import path from 'path';
-import fetch from 'node-fetch';
+import fetch from 'make-fetch-happen';
 
 export async function loadIndexHTML(
     config: IUnleashConfig,

tsconfig.json

@@ -78,7 +78,6 @@
     "snapshots",
     "coverage",
     "website",
-    "setupJest.js",
     "scripts"
   ]
 }