Skip to content

[Auth] Add custom error factory for custom messages; add special handling for instanceof checks #5427

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions packages/auth/src/api/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,13 +220,8 @@ describe('api/_performApiRequest', () => {
Endpoint.SIGN_UP,
request
);
let error: FirebaseError;
try {
await promise;
} catch (e) {
error = e;
}
expect(error!.customData!.message).to.eql('Text text text');
await expect(promise)
.to.be.rejectedWith(FirebaseError, 'Text text text');
});

it('should handle unknown server errors', async () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/auth/src/api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import { FirebaseError, querystring } from '@firebase/util';

import { AuthErrorCode, NamedErrorParams } from '../core/errors';
import { _createError, _fail } from '../core/util/assert';
import { _createError, _errorWithCustomMessage, _fail } from '../core/util/assert';
import { Delay } from '../core/util/delay';
import { _emulatorUrl } from '../core/util/emulator';
import { FetchProvider } from '../core/util/fetch_provider';
Expand Down Expand Up @@ -168,7 +168,7 @@ export async function _performFetchWithErrorHandling<V>(
.toLowerCase()
.replace(/[_\s]+/g, '-') as unknown) as AuthErrorCode);
if (serverErrorMessage) {
_fail(auth, authError, {message: serverErrorMessage});
throw _errorWithCustomMessage(auth, authError, serverErrorMessage);
} else {
_fail(auth, authError);
}
Expand Down
44 changes: 43 additions & 1 deletion packages/auth/src/core/util/assert.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ import { expect } from 'chai';

import { FirebaseError } from '@firebase/util';

import { assertTypes, opt } from './assert';
import { assertTypes, opt, _assertInstanceOf } from './assert';
import { Auth } from '../../model/public_types';
import { testAuth } from '../../../test/helpers/mock_auth';

class Parent {}
class Child extends Parent {}
Expand Down Expand Up @@ -147,3 +149,43 @@ describe('assertTypes', () => {
});
});
});

describe('_assertInstanceOf', () => {
interface Constructable {
new (): object;
}

let auth: Auth;
beforeEach(async () => {
auth = await testAuth();
});

function makeClass(): Constructable {
class Test {};
return Test;
};

class WrongClass {}

it('fails with the wrong class', () => {
expect(() => {
const foo = new WrongClass();
_assertInstanceOf(auth, foo, makeClass());
}).to.throw(FirebaseError, 'auth/argument-error');
});

it('fails with the right class wrong instance with custom message', () => {
expect(() => {
const a = makeClass();
const b = makeClass();
_assertInstanceOf(auth, new a(), b);
}).to.throw(FirebaseError, 'Type of Test does not match');
});

it('passes if all is well', () => {
expect(() => {
const a = makeClass();
_assertInstanceOf(auth, new a(), a);
}).not.to.throw();
});
});
32 changes: 30 additions & 2 deletions packages/auth/src/core/util/assert.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@
*/

import { Auth } from '../../model/public_types';
import { FirebaseError } from '@firebase/util';
import { ErrorFactory, FirebaseError } from '@firebase/util';
import { AuthInternal } from '../../model/auth';
import {
_DEFAULT_AUTH_ERROR_FACTORY,
AuthErrorCode,
AuthErrorParams
AuthErrorParams,
prodErrorMap,
ErrorMapRetriever
} from '../errors';
import { _logError } from './log';

Expand Down Expand Up @@ -81,6 +83,31 @@ export function _createError<K extends AuthErrorCode>(
return createErrorInternal(authOrCode, ...rest);
}

export function _errorWithCustomMessage(auth: Auth, code: AuthErrorCode, message: string): FirebaseError {
const errorMap = {...(prodErrorMap as ErrorMapRetriever)(), [code]: message};
const factory = new ErrorFactory<AuthErrorCode, AuthErrorParams>(
'auth',
'Firebase',
errorMap
);
return factory.create(code, {
appName: auth.name,
});
}

export function _assertInstanceOf(auth: Auth, object: object, instance: unknown): void {
const constructorInstance = (instance as { new (...args: unknown[]): unknown });
if (!(object instanceof constructorInstance)) {
if (constructorInstance.name !== object.constructor.name) {
_fail(auth, AuthErrorCode.ARGUMENT_ERROR);
}

throw _errorWithCustomMessage(auth, AuthErrorCode.ARGUMENT_ERROR,
`Type of ${object.constructor.name} does not match expected instance.` +
`Did you pass a reference from a different Auth SDK?`);
}
}

function createErrorInternal<K extends AuthErrorCode>(
authOrCode: Auth | K,
...rest: unknown[]
Expand Down Expand Up @@ -244,3 +271,4 @@ export function debugAssert(
debugFail(message);
}
}

23 changes: 4 additions & 19 deletions packages/auth/src/platform_browser/strategies/popup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {

import { _castAuth } from '../../core/auth/auth_impl';
import { AuthErrorCode } from '../../core/errors';
import { _assert, debugAssert, _createError } from '../../core/util/assert';
import { _assert, debugAssert, _createError, _assertInstanceOf } from '../../core/util/assert';
import { Delay } from '../../core/util/delay';
import { _generateEventId } from '../../core/util/event_id';
import { AuthInternal } from '../../model/auth';
Expand Down Expand Up @@ -83,12 +83,7 @@ export async function signInWithPopup(
resolver?: PopupRedirectResolver
): Promise<UserCredential> {
const authInternal = _castAuth(auth);
_assert(
provider instanceof FederatedAuthProvider,
auth,
AuthErrorCode.ARGUMENT_ERROR
);

_assertInstanceOf(auth, provider, FederatedAuthProvider);
const resolverInternal = _withDefaultResolver(authInternal, resolver);
const action = new PopupOperation(
authInternal,
Expand Down Expand Up @@ -130,12 +125,7 @@ export async function reauthenticateWithPopup(
resolver?: PopupRedirectResolver
): Promise<UserCredential> {
const userInternal = getModularInstance(user) as UserInternal;
_assert(
provider instanceof FederatedAuthProvider,
userInternal.auth,
AuthErrorCode.ARGUMENT_ERROR
);

_assertInstanceOf(userInternal.auth, provider, FederatedAuthProvider);
const resolverInternal = _withDefaultResolver(userInternal.auth, resolver);
const action = new PopupOperation(
userInternal.auth,
Expand Down Expand Up @@ -177,12 +167,7 @@ export async function linkWithPopup(
resolver?: PopupRedirectResolver
): Promise<UserCredential> {
const userInternal = getModularInstance(user) as UserInternal;
_assert(
provider instanceof FederatedAuthProvider,
userInternal.auth,
AuthErrorCode.ARGUMENT_ERROR
);

_assertInstanceOf(userInternal.auth, provider, FederatedAuthProvider);
const resolverInternal = _withDefaultResolver(userInternal.auth, resolver);

const action = new PopupOperation(
Expand Down
24 changes: 4 additions & 20 deletions packages/auth/src/platform_browser/strategies/redirect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,8 @@ import {
} from '../../model/public_types';

import { _castAuth } from '../../core/auth/auth_impl';
import { AuthErrorCode } from '../../core/errors';
import { _assertLinkedStatus } from '../../core/user/link_unlink';
import { _assert } from '../../core/util/assert';
import { _assertInstanceOf } from '../../core/util/assert';
import { _generateEventId } from '../../core/util/event_id';
import { AuthEventType } from '../../model/popup_redirect';
import { UserInternal } from '../../model/user';
Expand Down Expand Up @@ -91,12 +90,7 @@ export async function _signInWithRedirect(
resolver?: PopupRedirectResolver
): Promise<void | never> {
const authInternal = _castAuth(auth);
_assert(
provider instanceof FederatedAuthProvider,
auth,
AuthErrorCode.ARGUMENT_ERROR
);

_assertInstanceOf(auth, provider, FederatedAuthProvider);
const resolverInternal = _withDefaultResolver(authInternal, resolver);
await _setPendingRedirectStatus(resolverInternal, authInternal);

Expand Down Expand Up @@ -152,12 +146,7 @@ export async function _reauthenticateWithRedirect(
resolver?: PopupRedirectResolver
): Promise<void | never> {
const userInternal = getModularInstance(user) as UserInternal;
_assert(
provider instanceof FederatedAuthProvider,
userInternal.auth,
AuthErrorCode.ARGUMENT_ERROR
);

_assertInstanceOf(userInternal.auth, provider, FederatedAuthProvider);
// Allow the resolver to error before persisting the redirect user
const resolverInternal = _withDefaultResolver(userInternal.auth, resolver);
await _setPendingRedirectStatus(resolverInternal, userInternal.auth);
Expand Down Expand Up @@ -209,12 +198,7 @@ export async function _linkWithRedirect(
resolver?: PopupRedirectResolver
): Promise<void | never> {
const userInternal = getModularInstance(user) as UserInternal;
_assert(
provider instanceof FederatedAuthProvider,
userInternal.auth,
AuthErrorCode.ARGUMENT_ERROR
);

_assertInstanceOf(userInternal.auth, provider, FederatedAuthProvider);
// Allow the resolver to error before persisting the redirect user
const resolverInternal = _withDefaultResolver(userInternal.auth, resolver);
await _assertLinkedStatus(false, userInternal, provider.providerId);
Expand Down