Skip to content

Commit eff049b

Browse files
authored
Refactor auth error maps to make the verbose error map an optional dependency (#4008)
* Wow * Fix some broken bits * Tests * Fixing tests * Fix compat * Formatting * Remove auth error code from export * Fix bad rebase * Formatting * PR feedback * Fomratting * Fix broken bit * Fix broken build * Fix broken tests
1 parent c2b215c commit eff049b

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

65 files changed

+798
-690
lines changed

common/api-review/auth-exp.api.md

+7
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import { Auth } from '@firebase/auth-types-exp';
88
import { CompleteFn } from '@firebase/util';
9+
import { ErrorFactory } from '@firebase/util';
910
import { ErrorFn } from '@firebase/util';
1011
import * as externs from '@firebase/auth-types-exp';
1112
import { FirebaseApp } from '@firebase/app-types-exp';
@@ -82,6 +83,9 @@ export function createUserWithEmailAndPassword(auth: externs.Auth, email: string
8283
// @public
8384
export type CustomParameters = Record<string, string>;
8485

86+
// @public
87+
export const debugErrorMap: externs.AuthErrorMap;
88+
8589
// @public
8690
export function deleteUser(user: externs.User): Promise<void>;
8791

@@ -309,6 +313,9 @@ export class PhoneMultiFactorGenerator implements externs.PhoneMultiFactorGenera
309313
static assertion(credential: externs.PhoneAuthCredential): externs.PhoneMultiFactorAssertion;
310314
}
311315

316+
// @public
317+
export const prodErrorMap: externs.AuthErrorMap;
318+
312319
// @public
313320
export function reauthenticateWithCredential(user: externs.User, credential: externs.AuthCredential): Promise<externs.UserCredential>;
314321

packages-exp/auth-compat-exp/src/auth.ts

+14-11
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import {
3737
import { unwrap, Wrapper } from './wrap';
3838

3939
const PERSISTENCE_KEY = 'persistence';
40+
const _assert: typeof impl._assert = impl._assert;
4041

4142
export class Auth
4243
implements compat.FirebaseAuth, Wrapper<externs.Auth>, _FirebaseService {
@@ -59,10 +60,12 @@ export class Auth
5960
const hierarchy = persistences.map<impl.Persistence>(impl._getInstance);
6061

6162
// TODO: platform needs to be determined using heuristics
62-
impl.assertFn(apiKey, impl.AuthErrorCode.INVALID_API_KEY, {
63+
_assert(apiKey, impl.AuthErrorCode.INVALID_API_KEY, {
6364
appName: app.name
6465
});
6566

67+
this.auth._updateErrorMap(impl.debugErrorMap);
68+
6669
// This promise is intended to float; auth initialization happens in the
6770
// background, meanwhile the auth object may be used by the app.
6871
// eslint-disable-next-line @typescript-eslint/no-floating-promises
@@ -128,10 +131,10 @@ export class Auth
128131
return impl.isSignInWithEmailLink(this.auth, emailLink);
129132
}
130133
async getRedirectResult(): Promise<compat.UserCredential> {
131-
impl.assertFn(
134+
_assert(
132135
_isPopupRedirectSupported(),
133-
impl.AuthErrorCode.OPERATION_NOT_SUPPORTED,
134-
{ appName: this.app.name }
136+
this.auth,
137+
impl.AuthErrorCode.OPERATION_NOT_SUPPORTED
135138
);
136139
const credential = await impl.getRedirectResult(
137140
this.auth,
@@ -201,7 +204,7 @@ export class Auth
201204
case Persistence.NONE:
202205
return impl.inMemoryPersistence;
203206
default:
204-
return impl.fail(impl.AuthErrorCode.ARGUMENT_ERROR, {
207+
return impl._fail(impl.AuthErrorCode.ARGUMENT_ERROR, {
205208
appName: auth.name
206209
});
207210
}
@@ -266,10 +269,10 @@ export class Auth
266269
async signInWithPopup(
267270
provider: compat.AuthProvider
268271
): Promise<compat.UserCredential> {
269-
impl.assertFn(
272+
_assert(
270273
_isPopupRedirectSupported(),
271-
impl.AuthErrorCode.OPERATION_NOT_SUPPORTED,
272-
{ appName: this.app.name }
274+
this.auth,
275+
impl.AuthErrorCode.OPERATION_NOT_SUPPORTED
273276
);
274277
return convertCredential(
275278
this.auth,
@@ -281,10 +284,10 @@ export class Auth
281284
);
282285
}
283286
async signInWithRedirect(provider: compat.AuthProvider): Promise<void> {
284-
impl.assertFn(
287+
_assert(
285288
_isPopupRedirectSupported(),
286-
impl.AuthErrorCode.OPERATION_NOT_SUPPORTED,
287-
{ appName: this.app.name }
289+
this.auth,
290+
impl.AuthErrorCode.OPERATION_NOT_SUPPORTED
288291
);
289292
this.savePersistenceForRedirect();
290293
return impl.signInWithRedirect(

packages-exp/auth-compat-exp/src/persistence.ts

+16-16
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
* limitations under the License.
1616
*/
1717

18-
import { assertFn, AuthErrorCode } from '@firebase/auth-exp/internal';
18+
import { _assert, AuthErrorCode } from '@firebase/auth-exp/internal';
1919
import * as externs from '@firebase/auth-types-exp';
2020
import { isIndexedDBAvailable, isNode, isReactNative } from '@firebase/util';
2121
import { _isWebStorageSupported, _isWorker } from './platform';
@@ -34,45 +34,45 @@ export function _validatePersistenceArgument(
3434
auth: externs.Auth,
3535
persistence: string
3636
): void {
37-
assertFn(
37+
_assert(
3838
Object.values(Persistence).includes(persistence),
39-
AuthErrorCode.INVALID_PERSISTENCE,
40-
{ appName: auth.name }
39+
auth,
40+
AuthErrorCode.INVALID_PERSISTENCE
4141
);
4242
// Validate if the specified type is supported in the current environment.
4343
if (isReactNative()) {
4444
// This is only supported in a browser.
45-
assertFn(
45+
_assert(
4646
persistence !== Persistence.SESSION,
47-
AuthErrorCode.UNSUPPORTED_PERSISTENCE,
48-
{ appName: auth.name }
47+
auth,
48+
AuthErrorCode.UNSUPPORTED_PERSISTENCE
4949
);
5050
return;
5151
}
5252
if (isNode()) {
5353
// Only none is supported in Node.js.
54-
assertFn(
54+
_assert(
5555
persistence === Persistence.NONE,
56-
AuthErrorCode.UNSUPPORTED_PERSISTENCE,
57-
{ appName: auth.name }
56+
auth,
57+
AuthErrorCode.UNSUPPORTED_PERSISTENCE
5858
);
5959
return;
6060
}
6161
if (_isWorker()) {
6262
// In a worker environment, either LOCAL or NONE are supported.
6363
// If indexedDB not supported and LOCAL provided, throw an error
64-
assertFn(
64+
_assert(
6565
persistence === Persistence.NONE ||
6666
(persistence === Persistence.LOCAL && isIndexedDBAvailable()),
67-
AuthErrorCode.UNSUPPORTED_PERSISTENCE,
68-
{ appName: auth.name }
67+
auth,
68+
AuthErrorCode.UNSUPPORTED_PERSISTENCE
6969
);
7070
return;
7171
}
7272
// This is restricted by what the browser supports.
73-
assertFn(
73+
_assert(
7474
persistence === Persistence.NONE || _isWebStorageSupported(),
75-
AuthErrorCode.UNSUPPORTED_PERSISTENCE,
76-
{ appName: auth.name }
75+
auth,
76+
AuthErrorCode.UNSUPPORTED_PERSISTENCE
7777
);
7878
}

packages-exp/auth-compat-exp/src/recaptcha_verifier.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ import * as compat from '@firebase/auth-types';
2222
import * as externs from '@firebase/auth-types-exp';
2323
import { unwrap, Wrapper } from './wrap';
2424

25+
const _assert: typeof impl._assert = impl._assert;
26+
2527
export class RecaptchaVerifier
2628
implements compat.RecaptchaVerifier, Wrapper<externs.ApplicationVerifier> {
2729
readonly verifier: externs.RecaptchaVerifier;
@@ -32,7 +34,7 @@ export class RecaptchaVerifier
3234
app: FirebaseApp = firebase.app()
3335
) {
3436
// API key is required for web client RPC calls.
35-
impl.assertFn(app.options?.apiKey, impl.AuthErrorCode.INVALID_API_KEY, {
37+
_assert(app.options?.apiKey, impl.AuthErrorCode.INVALID_API_KEY, {
3638
appName: app.name
3739
});
3840
this.verifier = new impl.RecaptchaVerifier(

packages-exp/auth-exp/internal/index.ts

+1-4
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@
2020
*/
2121
export * from '../index';
2222

23-
import { assert } from '../src/core/util/assert';
24-
2523
export { SignInWithIdpResponse } from '../src/api/authentication/idp';
2624
export { AuthErrorCode } from '../src/core/errors';
2725
export { Persistence } from '../src/core/persistence';
@@ -36,5 +34,4 @@ export { ClientPlatform, _getClientVersion } from '../src/core/util/version';
3634

3735
export { _generateEventId } from '../src/core/util/event_id';
3836

39-
export { fail } from '../src/core/util/assert';
40-
export const assertFn: typeof assert = assert;
37+
export { _fail, _assert } from '../src/core/util/assert';

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ describe('api/_performApiRequest', () => {
173173
);
174174
await expect(promise).to.be.rejectedWith(
175175
FirebaseError,
176-
'auth/internal-error'
176+
'auth/awesome-error'
177177
);
178178
expect(mock.calls[0].request).to.eql(request);
179179
});

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

+12-29
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,8 @@
1717

1818
import { FirebaseError, querystring } from '@firebase/util';
1919

20-
import {
21-
AUTH_ERROR_FACTORY,
22-
AuthErrorCode,
23-
NamedErrorParams,
24-
ERRORS
25-
} from '../core/errors';
26-
import { fail } from '../core/util/assert';
20+
import { AuthErrorCode, NamedErrorParams } from '../core/errors';
21+
import { _createError, _fail } from '../core/util/assert';
2722
import { Delay } from '../core/util/delay';
2823
import { _emulatorUrl } from '../core/util/emulator';
2924
import { FetchProvider } from '../core/util/fetch_provider';
@@ -122,7 +117,7 @@ export async function _performFetchWithErrorHandling<V>(
122117
(auth as AuthInternal)._canInitEmulator = false;
123118
const errorMap = { ...SERVER_ERROR_MAP, ...customErrorMap };
124119
try {
125-
const networkTimeout = new NetworkTimeout<Response>(auth.name);
120+
const networkTimeout = new NetworkTimeout<Response>(auth);
126121
const response: Response = await Promise.race<Promise<Response>>([
127122
fetchFn(),
128123
networkTimeout.promise
@@ -154,21 +149,14 @@ export async function _performFetchWithErrorHandling<V>(
154149
errorMap[serverErrorCode] ||
155150
((serverErrorCode
156151
.toLowerCase()
157-
.replace(/_/g, '-') as unknown) as AuthErrorCode);
158-
if (authError && Object.keys(ERRORS).includes(authError)) {
159-
fail(authError, { appName: auth.name });
160-
} else {
161-
// TODO probably should handle improperly formatted errors as well
162-
// If you see this, add an entry to SERVER_ERROR_MAP for the corresponding error
163-
console.error(`Unexpected API error: ${json.error.message}`);
164-
fail(AuthErrorCode.INTERNAL_ERROR, { appName: auth.name });
165-
}
152+
.replace(/[_\s]+/g, '-') as unknown) as AuthErrorCode);
153+
_fail(auth, authError);
166154
}
167155
} catch (e) {
168156
if (e instanceof FirebaseError) {
169157
throw e;
170158
}
171-
fail(AuthErrorCode.NETWORK_REQUEST_FAILED, { appName: auth.name });
159+
_fail(auth, AuthErrorCode.NETWORK_REQUEST_FAILED);
172160
}
173161
}
174162

@@ -187,8 +175,7 @@ export async function _performSignInRequest<T, V extends IdTokenResponse>(
187175
customErrorMap
188176
)) as V;
189177
if ('mfaPendingCredential' in serverResponse) {
190-
throw AUTH_ERROR_FACTORY.create(AuthErrorCode.MFA_REQUIRED, {
191-
appName: auth.name,
178+
_fail(auth, AuthErrorCode.MFA_REQUIRED, {
192179
serverResponse
193180
});
194181
}
@@ -218,19 +205,15 @@ class NetworkTimeout<T> {
218205
private timer: any | null = null;
219206
readonly promise = new Promise<T>((_, reject) => {
220207
this.timer = setTimeout(() => {
221-
return reject(
222-
AUTH_ERROR_FACTORY.create(AuthErrorCode.TIMEOUT, {
223-
appName: this.appName
224-
})
225-
);
208+
return reject(_createError(this.auth, AuthErrorCode.TIMEOUT));
226209
}, DEFAULT_API_TIMEOUT_MS.get());
227210
});
228211

229212
clearNetworkTimeout(): void {
230213
clearTimeout(this.timer);
231214
}
232215

233-
constructor(private readonly appName: string) {}
216+
constructor(private readonly auth: Auth) {}
234217
}
235218

236219
interface PotentialResponse extends IdTokenResponse {
@@ -239,12 +222,12 @@ interface PotentialResponse extends IdTokenResponse {
239222
}
240223

241224
function makeTaggedError(
242-
{ name }: Auth,
225+
auth: Auth,
243226
code: AuthErrorCode,
244227
response: PotentialResponse
245228
): FirebaseError {
246229
const errorParams: NamedErrorParams = {
247-
appName: name
230+
appName: auth.name
248231
};
249232

250233
if (response.email) {
@@ -254,7 +237,7 @@ function makeTaggedError(
254237
errorParams.phoneNumber = response.phoneNumber;
255238
}
256239

257-
const error = AUTH_ERROR_FACTORY.create(code, errorParams);
240+
const error = _createError(auth, code, errorParams);
258241

259242
// We know customData is defined on error because errorParams is defined
260243
(error.customData! as TaggedWithTokenResponse)._tokenResponse = response;

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

+3-4
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616
*/
1717

1818
import * as externs from '@firebase/auth-types-exp';
19-
import { AuthErrorCode, AUTH_ERROR_FACTORY } from './errors';
19+
import { AuthErrorCode } from './errors';
20+
import { _assert } from './util/assert';
2021

2122
/**
2223
* Enums for fields in URL query string.
@@ -108,9 +109,7 @@ export class ActionCodeURL implements externs.ActionCodeURL {
108109
const code = uri.searchParams.get(QueryField.CODE);
109110
const operation = parseMode(uri.searchParams.get(QueryField.MODE));
110111
// Validate API key, code and mode.
111-
if (!apiKey || !code || !operation) {
112-
throw AUTH_ERROR_FACTORY.create(AuthErrorCode.ARGUMENT_ERROR, {});
113-
}
112+
_assert(apiKey && code && operation, AuthErrorCode.ARGUMENT_ERROR);
114113
this.apiKey = apiKey;
115114
this.operation = operation;
116115
this.code = code;

packages-exp/auth-exp/src/core/auth/auth_event_manager.test.ts

+3-2
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import { expect, use } from 'chai';
1919
import * as sinon from 'sinon';
2020
import * as sinonChai from 'sinon-chai';
21+
import { testAuth } from '../../../test/helpers/mock_auth';
2122

2223
import {
2324
AuthEvent,
@@ -53,8 +54,8 @@ describe('core/auth/auth_event_manager', () => {
5354
} as AuthEvent;
5455
}
5556

56-
beforeEach(() => {
57-
manager = new AuthEventManager('app-name');
57+
beforeEach(async () => {
58+
manager = new AuthEventManager(await testAuth());
5859
});
5960

6061
it('multiple consumers may be registered for one event type', () => {

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

+5-7
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ import {
2121
AuthEventType,
2222
EventManager
2323
} from '../../model/popup_redirect';
24-
import { AUTH_ERROR_FACTORY, AuthErrorCode } from '../errors';
24+
import { AuthErrorCode } from '../errors';
25+
import { Auth } from '../../model/auth';
26+
import { _createError } from '../util/assert';
2527

2628
// The amount of time to store the UIDs of seen events; this is
2729
// set to 10 min by default
@@ -34,7 +36,7 @@ export class AuthEventManager implements EventManager {
3436
private hasHandledPotentialRedirect = false;
3537
private lastProcessedEventTime = Date.now();
3638

37-
constructor(private readonly appName: string) {}
39+
constructor(private readonly auth: Auth) {}
3840

3941
registerConsumer(authEventConsumer: AuthEventConsumer): void {
4042
this.consumers.add(authEventConsumer);
@@ -90,11 +92,7 @@ export class AuthEventManager implements EventManager {
9092
const code =
9193
(event.error.code?.split('auth/')[1] as AuthErrorCode) ||
9294
AuthErrorCode.INTERNAL_ERROR;
93-
consumer.onError(
94-
AUTH_ERROR_FACTORY.create(code, {
95-
appName: this.appName
96-
})
97-
);
95+
consumer.onError(_createError(this.auth, code));
9896
} else {
9997
consumer.onAuthEvent(event);
10098
}

0 commit comments

Comments
 (0)