Repository: auth0/auth0-spa-js
Pull request #336 (2020-01-17 17:04:42)

[SDK-1279] getTokenSilently retry logic
### Description

Implements retry logic when using `fetch` to get a new access token. The implementation relies on `fetch` only failing in the event of a network failure. Failure responses, such as 4xx or 5xx errors still return a resolved promise but with the error detail in the body.

### References

[Fetch API on MDN](https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API)

### Testing

- [x] This change adds test coverage for new/changed/fixed functionality

### Checklist

- [x] I have added documentation for new/changed functionality in this PR or in auth0.com/docs
- [x] All active GitHub checks for tests, formatting, and security are passing
- [x] The correct base branch is being used, if not `master`


joshcanhelp: @stevehobbsdev - Feel free to ping me when this is back in shape!
stevehobbsdev: @joshcanhelp This is ready for you to look at again please.
stevehobbsdev: > Can you summarize what is the difference between the existing `getJSON` method and the newly introduced `fetchWithTimeout`? Both receive a timeout param and that confuses me.

@lbalmaceda `getJSON` _uses_ `fetchWithTimeout`. Right now on master, `getJSON` uses `fetch` directly but now uses this wrapped version that takes care of a timeout.
stevehobbsdev: @lbalmaceda thank you for being thorough!

Patch:
diff --git a/package-lock.json b/package-lock.json
index 17c3fa5..778d5cf 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1,6 +1,6 @@
 {
   "name": "@auth0/auth0-spa-js",
-  "version": "1.6.0",
+  "version": "1.7.0-beta.2",
   "lockfileVersion": 1,
   "requires": true,
   "dependencies": {
@@ -936,6 +936,11 @@
       "integrity": "sha512-sY5AXXVZv4Y1VACTtR11UJCPHHudgY5i26Qj5TypE6DKlIApbwb5uqhXcJ5UUGbvZNRh7EeIoW+LrJumBsKp7w==",
       "dev": true
     },
+    "abortcontroller-polyfill": {
+      "version": "1.4.0",
+      "resolved": "https://registry.npmjs.org/abortcontroller-polyfill/-/abortcontroller-polyfill-1.4.0.tgz",
+      "integrity": "sha512-3ZFfCRfDzx3GFjO6RAkYx81lPGpUS20ISxux9gLxuKnqafNcFQo59+IoZqpO2WvQlyc287B62HDnDdNYRmlvWA=="
+    },
     "accepts": {
       "version": "1.3.7",
       "resolved": "https://registry.npmjs.org/accepts/-/accepts-1.3.7.tgz",
diff --git a/package.json b/package.json
index 704eddb..f14f7f1 100644
--- a/package.json
+++ b/package.json
@@ -71,6 +71,7 @@
     "wait-on": "^3.3.0"
   },
   "dependencies": {
+    "abortcontroller-polyfill": "^1.4.0",
     "browser-tabs-lock": "^1.2.1",
     "core-js": "^3.2.1",
     "es-cookie": "^1.2.0",
diff --git a/src/Auth0Client.ts b/src/Auth0Client.ts
index d4318ce..acf9778 100644
--- a/src/Auth0Client.ts
+++ b/src/Auth0Client.ts
@@ -19,7 +19,10 @@ import TransactionManager from './transaction-manager';
 import { verify as verifyIdToken } from './jwt';
 import { AuthenticationError, GenericError } from './errors';
 import * as ClientStorage from './storage';
-import { DEFAULT_POPUP_CONFIG_OPTIONS } from './constants';
+import {
+  DEFAULT_POPUP_CONFIG_OPTIONS,
+  DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS
+} from './constants';
 import version from './version';
 import {
   Auth0ClientOptions,
@@ -221,7 +224,7 @@ export default class Auth0Client {
    */
   public async loginWithPopup(
     options: PopupLoginOptions = {},
-    config: PopupConfigOptions = DEFAULT_POPUP_CONFIG_OPTIONS
+    config: PopupConfigOptions = {}
   ) {
     const popup = await openPopup();
     const { ...authorizeOptions } = options;
@@ -247,7 +250,9 @@ export default class Auth0Client {
     const codeResult = await runPopup(popup, url, {
       ...config,
       timeoutInSeconds:
-        config.timeoutInSeconds || this.options.authorizeTimeoutInSeconds
+        config.timeoutInSeconds ||
+        this.options.authorizeTimeoutInSeconds ||
+        DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS
     });
 
     if (stateIn !== codeResult.state) {
diff --git a/src/constants.ts b/src/constants.ts
index 1c3d9e9..b9fdd72 100644
--- a/src/constants.ts
+++ b/src/constants.ts
@@ -8,4 +8,17 @@ export const DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS = 60;
 /**
  * @ignore
  */
-export const DEFAULT_POPUP_CONFIG_OPTIONS: PopupConfigOptions = {};
+
+export const DEFAULT_POPUP_CONFIG_OPTIONS: PopupConfigOptions = {
+  timeoutInSeconds: DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS
+};
+
+/**
+ * @ignore
+ */
+export const DEFAULT_SILENT_TOKEN_RETRY_COUNT = 3;
+
+/**
+ * @ignore
+ */
+export const DEFAULT_FETCH_TIMEOUT_MS = 10000;
diff --git a/src/global.ts b/src/global.ts
index cfae8f3..04eff89 100644
--- a/src/global.ts
+++ b/src/global.ts
@@ -273,6 +273,7 @@ export interface TokenEndpointOptions {
   baseUrl: string;
   client_id: string;
   grant_type: string;
+  timeout?: number;
 }
 
 /**
diff --git a/src/index.ts b/src/index.ts
index c424b7e..0d82cf6 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -5,6 +5,7 @@ import 'core-js/es/array/includes';
 import 'core-js/es/string/includes';
 import 'promise-polyfill/src/polyfill';
 import 'fast-text-encoding';
+import 'abortcontroller-polyfill/dist/abortcontroller-polyfill-only';
 
 // @ts-ignore
 import Auth0Client from './Auth0Client';
diff --git a/src/utils.ts b/src/utils.ts
index 7064a4f..d44094c 100644
--- a/src/utils.ts
+++ b/src/utils.ts
@@ -1,15 +1,23 @@
 import fetch from 'unfetch';
 
-import { DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS } from './constants';
 import {
   AuthenticationResult,
   PopupConfigOptions,
   TokenEndpointOptions
 } from './global';
 
+import {
+  DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS,
+  DEFAULT_SILENT_TOKEN_RETRY_COUNT,
+  DEFAULT_FETCH_TIMEOUT_MS
+} from './constants';
+
 const dedupe = arr => arr.filter((x, i) => arr.indexOf(x) === i);
 
 const TIMEOUT_ERROR = { error: 'timeout', error_description: 'Timeout' };
+
+export const createAbortController = () => new AbortController();
+
 export const getUniqueScopes = (...scopes: string[]) => {
   const scopeString = scopes.filter(Boolean).join();
   return dedupe(scopeString.replace(/\s/g, ',').split(','))
@@ -182,25 +190,70 @@ export const bufferToBase64UrlEncoded = input => {
   );
 };
 
-const getJSON = async (url, options) => {
-  const response = await fetch(url, options);
+const fetchWithTimeout = (url, options, timeout = DEFAULT_FETCH_TIMEOUT_MS) => {
+  const controller = createAbortController();
+  const signal = controller.signal;
+
+  const fetchOptions = {
+    ...options,
+    signal
+  };
+
+  // The promise will resolve with one of these two promises (the fetch and the timeout), whichever completes first.
+  return Promise.race([
+    fetch(url, fetchOptions),
+    new Promise((_, reject) => {
+      setTimeout(() => {
+        controller.abort();
+        reject(new Error("Timeout when executing 'fetch'"));
+      }, timeout);
+    })
+  ]);
+};
+
+const getJSON = async (url, timeout, options) => {
+  let fetchError, response;
+
+  for (let i = 0; i < DEFAULT_SILENT_TOKEN_RETRY_COUNT; i++) {
+    try {
+      response = await fetchWithTimeout(url, options, timeout);
+      fetchError = null;
+      break;
+    } catch (e) {
+      // Fetch only fails in the case of a network issue, so should be
+      // retried here. Failure status (4xx, 5xx, etc) return a resolved Promise
+      // with the failure in the body.
+      // https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API
+      fetchError = e;
+    }
+  }
+
+  if (fetchError) {
+    throw fetchError;
+  }
+
   const { error, error_description, ...success } = await response.json();
+
   if (!response.ok) {
     const errorMessage =
       error_description || `HTTP error. Unable to fetch ${url}`;
     const e = <any>new Error(errorMessage);
+
     e.error = error || 'request_error';
     e.error_description = errorMessage;
+
     throw e;
   }
+
   return success;
 };
 
 export const oauthToken = async ({
   baseUrl,
+  timeout,
   ...options
 }: TokenEndpointOptions) =>
-  await getJSON(`${baseUrl}/oauth/token`, {
+  await getJSON(`${baseUrl}/oauth/token`, timeout, {
     method: 'POST',
     body: JSON.stringify({
       redirect_uri: window.location.origin,


Test patch:
diff --git a/__tests__/index.test.ts b/__tests__/index.test.ts
index f9bed24..6ab19da 100644
--- a/__tests__/index.test.ts
+++ b/__tests__/index.test.ts
@@ -9,8 +9,15 @@ import createAuth0Client, {
   PopupConfigOptions,
   GetTokenSilentlyOptions
 } from '../src/index';
+
 import { AuthenticationError } from '../src/errors';
 import version from '../src/version';
+
+import {
+  DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS,
+  DEFAULT_FETCH_TIMEOUT_MS
+} from '../src/constants';
+
 const GET_TOKEN_SILENTLY_LOCK_KEY = 'auth0.lock.getTokenSilently';
 
 const TEST_DOMAIN = 'test.auth0.com';
@@ -37,7 +44,7 @@ const TEST_TELEMETRY_QUERY_STRING = `&auth0Client=${encodeURIComponent(
   )
 )}`;
 
-const DEFAULT_POPUP_CONFIG_OPTIONS: PopupConfigOptions = {};
+import { DEFAULT_POPUP_CONFIG_OPTIONS } from '../src/constants';
 
 const mockEnclosedCache = {
   get: jest.fn(),
@@ -280,7 +287,7 @@ describe('Auth0', () => {
       const { auth0, utils } = await setup({ authorizeTimeoutInSeconds: 1 });
       const popup = {};
       utils.openPopup.mockReturnValue(popup);
-      await auth0.loginWithPopup({}, DEFAULT_POPUP_CONFIG_OPTIONS);
+      await auth0.loginWithPopup({});
       expect(utils.runPopup).toHaveBeenCalledWith(
         popup,
         `https://test.auth0.com/authorize?${TEST_QUERY_PARAMS}${TEST_TELEMETRY_QUERY_STRING}`,
diff --git a/__tests__/utils.test.ts b/__tests__/utils.test.ts
index 8d4db4e..e1a0ea0 100644
--- a/__tests__/utils.test.ts
+++ b/__tests__/utils.test.ts
@@ -16,7 +16,11 @@ import {
   getCryptoSubtle,
   validateCrypto
 } from '../src/utils';
-import { DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS } from '../src/constants';
+
+import {
+  DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS,
+  DEFAULT_SILENT_TOKEN_RETRY_COUNT
+} from '../src/constants';
 
 (<any>global).TextEncoder = TextEncoder;
 
@@ -254,21 +258,33 @@ describe('utils', () => {
       expect(openPopup).toThrowError('Could not open popup');
     });
   });
+
   describe('oauthToken', () => {
     let oauthToken;
     let mockUnfetch;
+    let abortController;
+
     beforeEach(() => {
       jest.resetModules();
       jest.mock('unfetch');
       mockUnfetch = require('unfetch');
-      oauthToken = require('../src/utils').oauthToken;
+
+      const utils = require('../src/utils');
+      oauthToken = utils.oauthToken;
+
+      // Set up an AbortController that we can test has been called in the event of a timeout
+      abortController = new AbortController();
+      jest.spyOn(abortController, 'abort');
+      utils.createAbortController = jest.fn(() => abortController);
     });
+
     it('calls oauth/token with the correct url', async () => {
       mockUnfetch.mockReturnValue(
         new Promise(res =>
           res({ ok: true, json: () => new Promise(ress => ress(true)) })
         )
       );
+
       await oauthToken({
         grant_type: 'authorization_code',
         baseUrl: 'https://test.com',
@@ -277,18 +293,23 @@ describe('utils', () => {
         code_verifier: 'code_verifierIn'
       });
 
-      expect(mockUnfetch).toHaveBeenCalledWith('https://test.com/oauth/token', {
+      expect(mockUnfetch).toBeCalledWith('https://test.com/oauth/token', {
         body:
           '{"redirect_uri":"http://localhost","grant_type":"authorization_code","client_id":"client_idIn","code":"codeIn","code_verifier":"code_verifierIn"}',
         headers: { 'Content-type': 'application/json' },
-        method: 'POST'
+        method: 'POST',
+        signal: abortController.signal
       });
+
+      expect(mockUnfetch.mock.calls[0][1].signal).not.toBeUndefined();
     });
+
     it('handles error with error response', async () => {
       const theError = {
         error: 'the-error',
         error_description: 'the-error-description'
       };
+
       mockUnfetch.mockReturnValue(
         new Promise(res =>
           res({
@@ -297,6 +318,7 @@ describe('utils', () => {
           })
         )
       );
+
       try {
         await oauthToken({
           baseUrl: 'https://test.com',
@@ -310,6 +332,7 @@ describe('utils', () => {
         expect(error.error_description).toBe(theError.error_description);
       }
     });
+
     it('handles error without error response', async () => {
       mockUnfetch.mockReturnValue(
         new Promise(res =>
@@ -319,6 +342,7 @@ describe('utils', () => {
           })
         )
       );
+
       try {
         await oauthToken({
           baseUrl: 'https://test.com',
@@ -336,7 +360,115 @@ describe('utils', () => {
         );
       }
     });
+
+    it('retries the request in the event of a network failure', async () => {
+      // Fetch only fails in the case of a network issue, so should be
+      // retried here. Failure status (4xx, 5xx, etc) return a resolved Promise
+      // with the failure in the body.
+      // https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API
+      mockUnfetch.mockReturnValue(Promise.reject(new Error('Network failure')));
+
+      try {
+        await oauthToken({
+          baseUrl: 'https://test.com',
+          client_id: 'client_idIn',
+          code: 'codeIn',
+          code_verifier: 'code_verifierIn'
+        });
+      } catch (error) {
+        expect(error.message).toBe('Network failure');
+
+        expect(mockUnfetch).toHaveBeenCalledTimes(
+          DEFAULT_SILENT_TOKEN_RETRY_COUNT
+        );
+      }
+    });
+
+    it('continues the program after failing a couple of times then succeeding', async () => {
+      // Fetch only fails in the case of a network issue, so should be
+      // retried here. Failure status (4xx, 5xx, etc) return a resolved Promise
+      // with the failure in the body.
+      // https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API
+      mockUnfetch
+        .mockReturnValueOnce(Promise.reject(new Error('Network failure')))
+        .mockReturnValueOnce(Promise.reject(new Error('Network failure')))
+        .mockReturnValue(
+          Promise.resolve({
+            ok: true,
+            json: () => Promise.resolve({ access_token: 'access-token' })
+          })
+        );
+
+      const result = await oauthToken({
+        baseUrl: 'https://test.com',
+        client_id: 'client_idIn',
+        code: 'codeIn',
+        code_verifier: 'code_verifierIn'
+      });
+
+      expect(result.access_token).toBe('access-token');
+      expect(mockUnfetch).toHaveBeenCalledTimes(3);
+      expect(abortController.abort).not.toHaveBeenCalled();
+    });
+
+    it('surfaces a timeout error when the fetch continuously times out', async () => {
+      const createPromise = () =>
+        new Promise((resolve, _) => {
+          setTimeout(
+            () =>
+              resolve({
+                ok: true,
+                json: () => Promise.resolve({ access_token: 'access-token' })
+              }),
+            500
+          );
+        });
+
+      mockUnfetch.mockReturnValue(createPromise());
+
+      try {
+        await oauthToken({
+          baseUrl: 'https://test.com',
+          client_id: 'client_idIn',
+          code: 'codeIn',
+          code_verifier: 'code_verifierIn',
+          timeout: 100
+        });
+      } catch (e) {
+        expect(e.message).toBe("Timeout when executing 'fetch'");
+        expect(mockUnfetch).toHaveBeenCalledTimes(3);
+        expect(abortController.abort).toHaveBeenCalledTimes(3);
+      }
+    });
+
+    it('retries the request in the event of a timeout', async () => {
+      const fetchResult = {
+        ok: true,
+        json: () => Promise.resolve({ access_token: 'access-token' })
+      };
+
+      mockUnfetch.mockReturnValueOnce(
+        new Promise((resolve, _) => {
+          setTimeout(() => resolve(fetchResult), 1000);
+        })
+      );
+
+      mockUnfetch.mockReturnValue(Promise.resolve(fetchResult));
+
+      const result = await oauthToken({
+        baseUrl: 'https://test.com',
+        client_id: 'client_idIn',
+        code: 'codeIn',
+        code_verifier: 'code_verifierIn',
+        timeout: 500
+      });
+
+      expect(result.access_token).toBe('access-token');
+      expect(mockUnfetch).toHaveBeenCalledTimes(2);
+      expect(abortController.abort).toHaveBeenCalled();
+    });
   });
+
   describe('runPopup', () => {
     const TIMEOUT_ERROR = { error: 'timeout', error_description: 'Timeout' };
     const setup = customMessage => {