From e668a1d1e1208ca231b1fe37c6b1e1ec6fa1ae19 Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Fri, 5 Feb 2021 15:45:41 -0800 Subject: [PATCH 1/2] Make the prod error map be verbose for this one error --- .../src/core/auth/firebase_internal.test.ts | 26 ++++++++++++++++++- .../src/core/auth/firebase_internal.ts | 10 +++++++ packages-exp/auth-exp/src/core/errors.ts | 17 +++++++++++- 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/packages-exp/auth-exp/src/core/auth/firebase_internal.test.ts b/packages-exp/auth-exp/src/core/auth/firebase_internal.test.ts index 23545fcd5c3..e44e092a561 100644 --- a/packages-exp/auth-exp/src/core/auth/firebase_internal.test.ts +++ b/packages-exp/auth-exp/src/core/auth/firebase_internal.test.ts @@ -15,14 +15,18 @@ * limitations under the License. */ -import { expect } from 'chai'; +import { FirebaseError } from '@firebase/util'; +import { expect, use } from 'chai'; import * as sinon from 'sinon'; +import * as chaiAsPromised from 'chai-as-promised'; import { testAuth, testUser } from '../../../test/helpers/mock_auth'; import { Auth } from '../../model/auth'; import { User } from '../../model/user'; import { AuthInternal } from './firebase_internal'; +use(chaiAsPromised); + describe('core/auth/firebase_internal', () => { let auth: Auth; let authInternal: AuthInternal; @@ -45,6 +49,11 @@ describe('core/auth/firebase_internal', () => { await auth._updateCurrentUser(user); expect(authInternal.getUid()).to.eq('uid'); }); + + it('errors if Auth is not initialized', () => { + delete (auth as unknown as Record)['_initializationPromise']; + expect(() => authInternal.getUid()).to.throw(FirebaseError, 'auth/dependent-sdk-initialized-before-auth'); + }); }); context('getToken', () => { @@ -62,6 +71,11 @@ describe('core/auth/firebase_internal', () => { accessToken: 'access-token' }); }); + + it('errors if Auth is not initialized', async () => { + delete (auth as unknown as Record)['_initializationPromise']; + await expect(authInternal.getToken()).to.be.rejectedWith(FirebaseError, 'auth/dependent-sdk-initialized-before-auth'); + }); }); context('token listeners', () => { @@ -115,6 +129,11 @@ describe('core/auth/firebase_internal', () => { expect(tokenCount).to.eq(5); }); + + it('errors if Auth is not initialized', () => { + delete (auth as unknown as Record)['_initializationPromise']; + expect(() => authInternal.addAuthTokenListener(() => {})).to.throw(FirebaseError, 'auth/dependent-sdk-initialized-before-auth'); + }); }); context('removeAuthTokenListener', () => { @@ -168,6 +187,11 @@ describe('core/auth/firebase_internal', () => { authInternal.addAuthTokenListener(listenerB); expect(isProactiveRefresh).to.be.true; }); + + it('errors if Auth is not initialized', () => { + delete (auth as unknown as Record)['_initializationPromise']; + expect(() => authInternal.removeAuthTokenListener(() => {})).to.throw(FirebaseError, 'auth/dependent-sdk-initialized-before-auth'); + }); }); }); }); diff --git a/packages-exp/auth-exp/src/core/auth/firebase_internal.ts b/packages-exp/auth-exp/src/core/auth/firebase_internal.ts index ee1e2473536..8166795fc18 100644 --- a/packages-exp/auth-exp/src/core/auth/firebase_internal.ts +++ b/packages-exp/auth-exp/src/core/auth/firebase_internal.ts @@ -20,6 +20,8 @@ import { FirebaseAuthInternal } from '@firebase/auth-interop-types'; import { Auth } from '../../model/auth'; import { User } from '../../model/user'; +import { _assert } from '../util/assert'; +import { AuthErrorCode } from '../errors'; interface TokenListener { (tok: string | null): unknown; @@ -34,12 +36,14 @@ export class AuthInternal implements FirebaseAuthInternal { constructor(private readonly auth: Auth) {} getUid(): string | null { + this.assertAuthConfigured(); return this.auth.currentUser?.uid || null; } async getToken( forceRefresh?: boolean ): Promise<{ accessToken: string } | null> { + this.assertAuthConfigured(); await this.auth._initializationPromise; if (!this.auth.currentUser) { return null; @@ -50,6 +54,7 @@ export class AuthInternal implements FirebaseAuthInternal { } addAuthTokenListener(listener: TokenListener): void { + this.assertAuthConfigured(); if (this.internalListeners.has(listener)) { return; } @@ -62,6 +67,7 @@ export class AuthInternal implements FirebaseAuthInternal { } removeAuthTokenListener(listener: TokenListener): void { + this.assertAuthConfigured(); const unsubscribe = this.internalListeners.get(listener); if (!unsubscribe) { return; @@ -72,6 +78,10 @@ export class AuthInternal implements FirebaseAuthInternal { this.updateProactiveRefresh(); } + private assertAuthConfigured(): void { + _assert(this.auth._initializationPromise, AuthErrorCode.DEPENDENT_SDK_INIT_BEFORE_AUTH); + } + private updateProactiveRefresh(): void { if (this.internalListeners.size > 0) { this.auth._startProactiveRefresh(); diff --git a/packages-exp/auth-exp/src/core/errors.ts b/packages-exp/auth-exp/src/core/errors.ts index 50889f048e6..5ad731e1181 100644 --- a/packages-exp/auth-exp/src/core/errors.ts +++ b/packages-exp/auth-exp/src/core/errors.ts @@ -39,6 +39,7 @@ export const enum AuthErrorCode { CREDENTIAL_ALREADY_IN_USE = 'credential-already-in-use', CREDENTIAL_MISMATCH = 'custom-token-mismatch', CREDENTIAL_TOO_OLD_LOGIN_AGAIN = 'requires-recent-login', + DEPENDENT_SDK_INIT_BEFORE_AUTH = 'dependent-sdk-initialized-before-auth', DYNAMIC_LINK_NOT_ACTIVATED = 'dynamic-link-not-activated', EMAIL_CHANGE_NEEDS_VERIFICATION = 'email-change-needs-verification', EMAIL_EXISTS = 'email-already-in-use', @@ -152,6 +153,10 @@ function _debugErrorMap(): ErrorMap { [AuthErrorCode.CREDENTIAL_TOO_OLD_LOGIN_AGAIN]: 'This operation is sensitive and requires recent authentication. Log in ' + 'again before retrying this request.', + [AuthErrorCode.DEPENDENT_SDK_INIT_BEFORE_AUTH]: + 'Another Firebase SDK was initialized and is trying to use Auth before Auth is ' + + 'initialized. Please be sure to call `initializeAuth` or `getAuth` before ' + + 'starting any other Firebase SDK.', [AuthErrorCode.DYNAMIC_LINK_NOT_ACTIVATED]: 'Please activate Dynamic Links in the Firebase Console and agree to the terms and ' + 'conditions.', @@ -351,7 +356,15 @@ export interface ErrorMapRetriever extends externs.AuthErrorMap { } function _prodErrorMap(): ErrorMap { - return {} as ErrorMap; + // We will include this one message in the prod error map since by the very + // nature of this error, developers will never be able to see the message + // using the debugErrorMap (which is installed during auth initialization). + return { + [AuthErrorCode.DEPENDENT_SDK_INIT_BEFORE_AUTH]: + 'Another Firebase SDK was initialized and is trying to use Auth before Auth is ' + + 'initialized. Please be sure to call `initializeAuth` or `getAuth` before ' + + 'starting any other Firebase SDK.', + } as ErrorMap; } /** @@ -386,6 +399,7 @@ type GenericAuthErrorParams = { [key in Exclude< AuthErrorCode, | AuthErrorCode.ARGUMENT_ERROR + | AuthErrorCode.DEPENDENT_SDK_INIT_BEFORE_AUTH | AuthErrorCode.INTERNAL_ERROR | AuthErrorCode.MFA_REQUIRED >]: { @@ -397,6 +411,7 @@ type GenericAuthErrorParams = { export interface AuthErrorParams extends GenericAuthErrorParams { [AuthErrorCode.ARGUMENT_ERROR]: { appName?: AppName }; + [AuthErrorCode.DEPENDENT_SDK_INIT_BEFORE_AUTH]: { appName?: AppName }; [AuthErrorCode.INTERNAL_ERROR]: { appName?: AppName }; [AuthErrorCode.MFA_REQUIRED]: { appName: AppName; From 9ee881c7b3a0f57a6dcd376dde5e27d77cfb73ce Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Fri, 5 Feb 2021 15:45:59 -0800 Subject: [PATCH 2/2] Formatting --- .../src/core/auth/firebase_internal.test.ts | 36 ++++++++++++++----- .../src/core/auth/firebase_internal.ts | 5 ++- packages-exp/auth-exp/src/core/errors.ts | 2 +- 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/packages-exp/auth-exp/src/core/auth/firebase_internal.test.ts b/packages-exp/auth-exp/src/core/auth/firebase_internal.test.ts index e44e092a561..80e1b0fab65 100644 --- a/packages-exp/auth-exp/src/core/auth/firebase_internal.test.ts +++ b/packages-exp/auth-exp/src/core/auth/firebase_internal.test.ts @@ -51,8 +51,13 @@ describe('core/auth/firebase_internal', () => { }); it('errors if Auth is not initialized', () => { - delete (auth as unknown as Record)['_initializationPromise']; - expect(() => authInternal.getUid()).to.throw(FirebaseError, 'auth/dependent-sdk-initialized-before-auth'); + delete ((auth as unknown) as Record)[ + '_initializationPromise' + ]; + expect(() => authInternal.getUid()).to.throw( + FirebaseError, + 'auth/dependent-sdk-initialized-before-auth' + ); }); }); @@ -73,8 +78,13 @@ describe('core/auth/firebase_internal', () => { }); it('errors if Auth is not initialized', async () => { - delete (auth as unknown as Record)['_initializationPromise']; - await expect(authInternal.getToken()).to.be.rejectedWith(FirebaseError, 'auth/dependent-sdk-initialized-before-auth'); + delete ((auth as unknown) as Record)[ + '_initializationPromise' + ]; + await expect(authInternal.getToken()).to.be.rejectedWith( + FirebaseError, + 'auth/dependent-sdk-initialized-before-auth' + ); }); }); @@ -131,8 +141,13 @@ describe('core/auth/firebase_internal', () => { }); it('errors if Auth is not initialized', () => { - delete (auth as unknown as Record)['_initializationPromise']; - expect(() => authInternal.addAuthTokenListener(() => {})).to.throw(FirebaseError, 'auth/dependent-sdk-initialized-before-auth'); + delete ((auth as unknown) as Record)[ + '_initializationPromise' + ]; + expect(() => authInternal.addAuthTokenListener(() => {})).to.throw( + FirebaseError, + 'auth/dependent-sdk-initialized-before-auth' + ); }); }); @@ -189,8 +204,13 @@ describe('core/auth/firebase_internal', () => { }); it('errors if Auth is not initialized', () => { - delete (auth as unknown as Record)['_initializationPromise']; - expect(() => authInternal.removeAuthTokenListener(() => {})).to.throw(FirebaseError, 'auth/dependent-sdk-initialized-before-auth'); + delete ((auth as unknown) as Record)[ + '_initializationPromise' + ]; + expect(() => authInternal.removeAuthTokenListener(() => {})).to.throw( + FirebaseError, + 'auth/dependent-sdk-initialized-before-auth' + ); }); }); }); diff --git a/packages-exp/auth-exp/src/core/auth/firebase_internal.ts b/packages-exp/auth-exp/src/core/auth/firebase_internal.ts index 8166795fc18..f2096d0d0e6 100644 --- a/packages-exp/auth-exp/src/core/auth/firebase_internal.ts +++ b/packages-exp/auth-exp/src/core/auth/firebase_internal.ts @@ -79,7 +79,10 @@ export class AuthInternal implements FirebaseAuthInternal { } private assertAuthConfigured(): void { - _assert(this.auth._initializationPromise, AuthErrorCode.DEPENDENT_SDK_INIT_BEFORE_AUTH); + _assert( + this.auth._initializationPromise, + AuthErrorCode.DEPENDENT_SDK_INIT_BEFORE_AUTH + ); } private updateProactiveRefresh(): void { diff --git a/packages-exp/auth-exp/src/core/errors.ts b/packages-exp/auth-exp/src/core/errors.ts index 5ad731e1181..992e69318d0 100644 --- a/packages-exp/auth-exp/src/core/errors.ts +++ b/packages-exp/auth-exp/src/core/errors.ts @@ -363,7 +363,7 @@ function _prodErrorMap(): ErrorMap { [AuthErrorCode.DEPENDENT_SDK_INIT_BEFORE_AUTH]: 'Another Firebase SDK was initialized and is trying to use Auth before Auth is ' + 'initialized. Please be sure to call `initializeAuth` or `getAuth` before ' + - 'starting any other Firebase SDK.', + 'starting any other Firebase SDK.' } as ErrorMap; }