Add conditional delays to auth-next (#2934)

* Add conditional delays to auth-next

* [AUTOMATED]: Prettier Code Styling

* Use typescript asserts keyword for typesafe assertions

* [AUTOMATED]: Prettier Code Styling

* Rebase conflicts & PR feedback

* More PR Feedback

* Strip debug asserts from prod builds

* [AUTOMATED]: Prettier Code Styling

* [AUTOMATED]: API Reports

* Fix logic in assertion

* Revert merge artifacts

* PR Feedback

Author: avolkovi

packages-exp/auth-exp/package.json

@@ -29,11 +29,13 @@
     "@firebase/app-types-exp": "0.x"
   },
   "dependencies": {
+    "@firebase/logger": "^0.2.2",
     "@firebase/util": "^0.2.44",
     "tslib": "1.11.1"
   },
   "license": "Apache-2.0",
   "devDependencies": {
+    "@rollup/plugin-strip": "^1.3.2",
     "rollup": "1.32.1",
     "rollup-plugin-json": "4.0.0",
     "rollup-plugin-replace": "2.2.0",

packages-exp/auth-exp/rollup.config.js

@@ -15,6 +15,7 @@
  * limitations under the License.
  */
 
+import strip from '@rollup/plugin-strip';
 import typescriptPlugin from 'rollup-plugin-typescript2';
 import typescript from 'typescript';
 import pkg from './package.json';
@@ -23,10 +24,20 @@ const deps = Object.keys(
   Object.assign({}, pkg.peerDependencies, pkg.dependencies)
 );
 
+/**
+ * Common plugins for all builds
+ */
+const commonPlugins = [
+  strip({
+    functions: ['debugAssert.*']
+  })
+];
+
 /**
  * ES5 Builds
  */
 const es5BuildPlugins = [
+  ...commonPlugins,
   typescriptPlugin({
     typescript
   })
@@ -57,6 +68,7 @@ const es5Builds = [
  * ES2017 Builds
  */
 const es2017BuildPlugins = [
+  ...commonPlugins,
   typescriptPlugin({
     typescript,
     tsconfigOverride: {

packages-exp/auth-exp/src/api/index.test.ts

@@ -175,7 +175,7 @@ describe('performApiRequest', () => {
         Endpoint.SIGN_UP,
         request
       );
-      clock.tick(DEFAULT_API_TIMEOUT_MS + 1);
+      clock.tick(DEFAULT_API_TIMEOUT_MS.get() + 1);
       await expect(promise).to.be.rejectedWith(
         FirebaseError,
         'Firebase: The operation has timed out. (auth/timeout).'

packages-exp/auth-exp/src/api/index.ts

@@ -25,6 +25,7 @@ import {
   ServerErrorMap,
   SERVER_ERROR_MAP
 } from './errors';
+import { Delay } from '../core/util/delay';
 
 export enum HttpMethod {
   POST = 'POST',
@@ -53,7 +54,7 @@ export enum Endpoint {
   WITHDRAW_MFA = '/v2/accounts/mfaEnrollment:withdraw'
 }
 
-export const DEFAULT_API_TIMEOUT_MS = 30_000;
+export const DEFAULT_API_TIMEOUT_MS = new Delay(30_000, 60_000);
 
 export async function performApiRequest<T, V>(
   auth: Auth,
@@ -101,7 +102,7 @@ export async function performApiRequest<T, V>(
               appName: auth.name
             })
           );
-        }, DEFAULT_API_TIMEOUT_MS)
+        }, DEFAULT_API_TIMEOUT_MS.get())
       )
     ]);
     if (response.ok) {

packages-exp/auth-exp/src/core/auth/auth_impl.ts

@@ -97,7 +97,8 @@ class AuthImpl implements Auth {
   }
 
   private get assertedPersistence(): PersistenceUserManager {
-    return assert(this.persistenceManager, this.name);
+    assert(this.persistenceManager, this.name);
+    return this.persistenceManager;
   }
 }
 
@@ -110,8 +111,9 @@ export function initializeAuth(
   const { apiKey, authDomain } = app.options;
 
   // TODO: platform needs to be determined using heuristics
+  assert(apiKey, app.name, AuthErrorCode.INVALID_API_KEY);
   const config: Config = {
-    apiKey: assert(apiKey, app.name, AuthErrorCode.INVALID_API_KEY),
+    apiKey,
     authDomain,
     apiHost: DEFAULT_API_HOST,
     apiScheme: DEFAULT_API_SCHEME,

packages-exp/auth-exp/src/core/user/token_manager.ts

@@ -17,7 +17,7 @@
 
 import { IdTokenResponse } from '../../model/id_token';
 import { PersistedBlob } from '../persistence';
-import { assertType } from '../util/assert';
+import { assert } from '../util/assert';
 
 /**
  * The number of milliseconds before the official expiration time of a token
@@ -79,13 +79,16 @@ export class StsTokenManager {
 
     const manager = new StsTokenManager();
     if (refreshToken) {
-      manager.refreshToken = assertType(refreshToken, 'string', appName);
+      assert(typeof refreshToken === 'string', appName);
+      manager.refreshToken = refreshToken;
     }
     if (accessToken) {
-      manager.accessToken = assertType(accessToken, 'string', appName);
+      assert(typeof accessToken === 'string', appName);
+      manager.accessToken = accessToken;
     }
     if (expirationTime) {
-      manager.expirationTime = assertType(expirationTime, 'number', appName);
+      assert(typeof expirationTime === 'number', appName);
+      manager.expirationTime = expirationTime;
     }
     return manager;
   }

packages-exp/auth-exp/src/core/user/user_impl.ts

@@ -20,7 +20,7 @@ import { IdTokenResult } from '../../model/id_token';
 import { User } from '../../model/user';
 import { PersistedBlob } from '../persistence';
 import { ProviderId } from '../providers';
-import { assert, assertType } from '../util/assert';
+import { assert } from '../util/assert';
 import { StsTokenManager } from './token_manager';
 
 export interface UserParameters {
@@ -34,6 +34,16 @@ export interface UserParameters {
   photoURL?: string;
 }
 
+function assertStringOrUndefined(
+  assertion: unknown,
+  appName: string
+): asserts assertion is string | undefined {
+  assert(
+    typeof assertion === 'string' || typeof assertion === 'undefined',
+    appName
+  );
+}
+
 export class UserImpl implements User {
   // For the user object, provider is always Firebase.
   readonly providerId = ProviderId.FIREBASE;
@@ -111,16 +121,19 @@ export class UserImpl implements User {
       plainObjectTokenManager as PersistedBlob
     );
 
-    const stringOrUndef = ['string', 'undefined'];
-
+    assert(typeof uid === 'string', auth.name);
+    assertStringOrUndefined(displayName, auth.name);
+    assertStringOrUndefined(email, auth.name);
+    assertStringOrUndefined(phoneNumber, auth.name);
+    assertStringOrUndefined(photoURL, auth.name);
     return new UserImpl({
-      uid: assertType(uid, 'string', auth.name),
+      uid,
       auth,
       stsTokenManager,
-      displayName: assertType(displayName, stringOrUndef, auth.name),
-      email: assertType(email, stringOrUndef, auth.name),
-      phoneNumber: assertType(phoneNumber, stringOrUndef, auth.name),
-      photoURL: assertType(photoURL, stringOrUndef, auth.name)
+      displayName,
+      email,
+      phoneNumber,
+      photoURL
     });
   }
 }

packages-exp/auth-exp/src/core/util/assert.ts

@@ -15,29 +15,65 @@
  * limitations under the License.
  */
 
-import { AUTH_ERROR_FACTORY, AuthErrorCode } from '../errors';
+import { AuthErrorCode, AUTH_ERROR_FACTORY } from '../errors';
+import { logError } from './log';
 
-export function assert<T>(
-  expression: T | null | undefined,
+/**
+ * Unconditionally fails, throwing a developer facing INTERNAL_ERROR
+ *
+ * @param appName App name for tagging the error
+ * @throws FirebaseError
+ */
+export function fail(appName: string, errorCode: AuthErrorCode): never {
+  throw AUTH_ERROR_FACTORY.create(errorCode, { appName });
+}
+
+/**
+ * Verifies the given condition and fails if false, throwing a developer facing error
+ *
+ * @param assertion
+ * @param appName
+ */
+export function assert(
+  assertion: unknown,
   appName: string,
-  code = AuthErrorCode.INTERNAL_ERROR
-): T {
-  if (!expression) {
-    throw AUTH_ERROR_FACTORY.create(code, { appName });
+  errorCode = AuthErrorCode.INTERNAL_ERROR
+): asserts assertion {
+  if (!assertion) {
+    fail(appName, errorCode);
   }
+}
 
-  return expression;
+/**
+ * Unconditionally fails, throwing an internal error with the given message.
+ *
+ * @param failure type of failure encountered
+ * @throws Error
+ */
+export function debugFail(failure: string): never {
+  // Log the failure in addition to throw an exception, just in case the
+  // exception is swallowed.
+  const message = `INTERNAL ASSERTION FAILED: ` + failure;
+  logError(message);
+
+  // NOTE: We don't use FirebaseError here because these are internal failures
+  // that cannot be handled by the user. (Also it would create a circular
+  // dependency between the error and assert modules which doesn't work.)
+  throw new Error(message);
 }
 
-export function assertType<T>(
-  expression: unknown,
-  expected: string | string[],
-  appName: string
-): T {
-  if (typeof expected === 'string') {
-    expected = [expected];
+/**
+ * Fails if the given assertion condition is false, throwing an Error with the
+ * given message if it did.
+ *
+ * @param assertion
+ * @param message
+ */
+export function debugAssert(
+  assertion: unknown,
+  message: string
+): asserts assertion {
+  if (!assertion) {
+    debugFail(message);
   }
-
-  assert(expected.includes(typeof expression), appName);
-  return expression as T;
 }

packages-exp/auth-exp/src/core/util/delay.test.ts

@@ -0,0 +1,55 @@
+/**
+ * @license
+ * Copyright 2020 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import * as util from '@firebase/util';
+import { expect } from 'chai';
+import { restore, stub } from 'sinon';
+import { Delay, _OFFLINE_DELAY_MS } from './delay';
+import * as navigator from './navigator';
+
+describe('Delay.get()', () => {
+  const SHORT_DELAY = 30_000;
+  const LONG_DELAY = 60_000;
+
+  afterEach(restore);
+
+  it('should return the short delay in browser environments', () => {
+    const delay = new Delay(SHORT_DELAY, LONG_DELAY);
+    expect(delay.get()).to.eq(SHORT_DELAY);
+  });
+
+  it('should return the long delay in Cordova environments', () => {
+    const mock = stub(util, 'isMobileCordova');
+    mock.callsFake(() => true);
+    const delay = new Delay(SHORT_DELAY, LONG_DELAY);
+    expect(delay.get()).to.eq(LONG_DELAY);
+  });
+
+  it('should return the long delay in React Native environments', () => {
+    const mock = stub(util, 'isReactNative');
+    mock.callsFake(() => true);
+    const delay = new Delay(SHORT_DELAY, LONG_DELAY);
+    expect(delay.get()).to.eq(LONG_DELAY);
+  });
+
+  it('should return quicker when offline', () => {
+    const mock = stub(navigator, 'isOnline');
+    mock.callsFake(() => false);
+    const delay = new Delay(SHORT_DELAY, LONG_DELAY);
+    expect(delay.get()).to.eq(_OFFLINE_DELAY_MS);
+  });
+});

packages-exp/auth-exp/src/core/util/delay.ts

@@ -0,0 +1,56 @@
+/**
+ * @license
+ * Copyright 2020 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { isMobileCordova, isReactNative } from '@firebase/util';
+import { isOnline } from './navigator';
+import { debugAssert } from './assert';
+
+export const _OFFLINE_DELAY_MS = 5000;
+
+/**
+ * A structure to help pick between a range of long and short delay durations
+ * depending on the current environment. In general, the long delay is used for
+ * mobile environments whereas short delays are used for desktop environments.
+ */
+export class Delay {
+  // The default value for the offline delay timeout in ms.
+
+  private readonly isMobile: boolean;
+  constructor(
+    private readonly shortDelay: number,
+    private readonly longDelay: number
+  ) {
+    // Internal error when improperly initialized.
+    debugAssert(
+      longDelay > shortDelay,
+      'Short delay should be less than long delay!'
+    );
+    this.isMobile = isMobileCordova() || isReactNative();
+  }
+
+  get(): number {
+    if (!isOnline()) {
+      // Pick the shorter timeout.
+      return Math.min(_OFFLINE_DELAY_MS, this.shortDelay);
+    }
+    // If running in a mobile environment, return the long delay, otherwise
+    // return the short delay.
+    // This could be improved in the future to dynamically change based on other
+    // variables instead of just reading the current environment.
+    return this.isMobile ? this.longDelay : this.shortDelay;
+  }
+}