Skip to content

Commit 882c99f

Browse files
hsubox76hsubox76
authored
Merge 7ae3d1d into 580b354 
2 parents 580b354 + 7ae3d1d commit 882c99f

File tree

7 files changed

+132
-30
lines changed

7 files changed

+132
-30
lines changed

‎.changeset/gold-turtles-tell.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@firebase/app-check': patch
3+
'@firebase/app-check-types': patch
4+
---
5+
6+
Fixed so token listeners added through public API call the error handler while internal token listeners return the error as a token field.

‎packages/app-check-types/index.d.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,12 @@
1616
*/
1717

1818
import { PartialObserver, Unsubscribe } from '@firebase/util';
19+
import { FirebaseApp } from '@firebase/app-types';
1920

2021
export interface FirebaseAppCheck {
22+
/** The `FirebaseApp` associated with this instance. */
23+
app: FirebaseApp;
24+
2125
/**
2226
* Activate AppCheck
2327
* @param siteKeyOrProvider - reCAPTCHA sitekey or custom token provider

‎packages/app-check/src/api.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import {
2222
import { FirebaseApp } from '@firebase/app-types';
2323
import { ERROR_FACTORY, AppCheckError } from './errors';
2424
import { initialize as initializeRecaptcha } from './recaptcha';
25-
import { getState, setState, AppCheckState } from './state';
25+
import { getState, setState, AppCheckState, ListenerType } from './state';
2626
import {
2727
getToken as getTokenInternal,
2828
addTokenListener,
@@ -162,6 +162,12 @@ export function onTokenChanged(
162162
} else if (onError) {
163163
errorFn = onError;
164164
}
165-
addTokenListener(app, platformLoggerProvider, nextFn, errorFn);
165+
addTokenListener(
166+
app,
167+
platformLoggerProvider,
168+
ListenerType.EXTERNAL,
169+
nextFn,
170+
errorFn
171+
);
166172
return () => removeTokenListener(app, nextFn);
167173
}

‎packages/app-check/src/factory.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ import { Provider } from '@firebase/component';
3737
import { PartialObserver } from '@firebase/util';
3838

3939
import { FirebaseService } from '@firebase/app-types/private';
40-
import { getState } from './state';
40+
import { getState, ListenerType } from './state';
4141

4242
export function factory(
4343
app: FirebaseApp,
@@ -92,7 +92,12 @@ export function internalFactory(
9292
getToken: forceRefresh =>
9393
getTokenInternal(app, platformLoggerProvider, forceRefresh),
9494
addTokenListener: listener =>
95-
addTokenListener(app, platformLoggerProvider, listener),
95+
addTokenListener(
96+
app,
97+
platformLoggerProvider,
98+
ListenerType.INTERNAL,
99+
listener
100+
),
96101
removeTokenListener: listener => removeTokenListener(app, listener)
97102
};
98103
}

‎packages/app-check/src/internal-api.test.ts

Lines changed: 88 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,13 @@ import * as logger from './logger';
4040
import * as client from './client';
4141
import * as storage from './storage';
4242
import * as util from './util';
43-
import { getState, clearState, setState, getDebugState } from './state';
43+
import {
44+
getState,
45+
clearState,
46+
setState,
47+
getDebugState,
48+
ListenerType
49+
} from './state';
4450
import { Deferred } from '@firebase/util';
4551
import { AppCheckTokenResult } from '../../app-check-interop-types';
4652

@@ -143,8 +149,18 @@ describe('internal api', () => {
143149

144150
const listener1 = spy();
145151
const listener2 = spy();
146-
addTokenListener(app, fakePlatformLoggingProvider, listener1);
147-
addTokenListener(app, fakePlatformLoggingProvider, listener2);
152+
addTokenListener(
153+
app,
154+
fakePlatformLoggingProvider,
155+
ListenerType.INTERNAL,
156+
listener1
157+
);
158+
addTokenListener(
159+
app,
160+
fakePlatformLoggingProvider,
161+
ListenerType.INTERNAL,
162+
listener2
163+
);
148164

149165
await getToken(app, fakePlatformLoggingProvider);
150166

@@ -164,8 +180,18 @@ describe('internal api', () => {
164180

165181
const listener1 = spy();
166182
const listener2 = spy();
167-
addTokenListener(app, fakePlatformLoggingProvider, listener1);
168-
addTokenListener(app, fakePlatformLoggingProvider, listener2);
183+
addTokenListener(
184+
app,
185+
fakePlatformLoggingProvider,
186+
ListenerType.INTERNAL,
187+
listener1
188+
);
189+
addTokenListener(
190+
app,
191+
fakePlatformLoggingProvider,
192+
ListenerType.INTERNAL,
193+
listener2
194+
);
169195

170196
await getToken(app, fakePlatformLoggingProvider);
171197

@@ -177,7 +203,7 @@ describe('internal api', () => {
177203
});
178204
});
179205

180-
it('calls optional error handler if there is an error getting a token', async () => {
206+
it('calls 3P error handler if there is an error getting a token', async () => {
181207
stub(logger.logger, 'error');
182208
activate(app, FAKE_SITE_KEY, false);
183209
stub(reCAPTCHA, 'getToken').resolves(fakeRecaptchaToken);
@@ -186,7 +212,13 @@ describe('internal api', () => {
186212

187213
const errorFn1 = spy();
188214

189-
addTokenListener(app, fakePlatformLoggingProvider, listener1, errorFn1);
215+
addTokenListener(
216+
app,
217+
fakePlatformLoggingProvider,
218+
ListenerType.EXTERNAL,
219+
listener1,
220+
errorFn1
221+
);
190222

191223
await getToken(app, fakePlatformLoggingProvider);
192224

@@ -203,8 +235,19 @@ describe('internal api', () => {
203235

204236
const errorFn1 = spy();
205237

206-
addTokenListener(app, fakePlatformLoggingProvider, listener1, errorFn1);
207-
addTokenListener(app, fakePlatformLoggingProvider, listener2);
238+
addTokenListener(
239+
app,
240+
fakePlatformLoggingProvider,
241+
ListenerType.INTERNAL,
242+
listener1,
243+
errorFn1
244+
);
245+
addTokenListener(
246+
app,
247+
fakePlatformLoggingProvider,
248+
ListenerType.INTERNAL,
249+
listener2
250+
);
208251

209252
await getToken(app, fakePlatformLoggingProvider);
210253

@@ -298,7 +341,12 @@ describe('internal api', () => {
298341
const listener = (): void => {};
299342
stub(client, 'exchangeToken').resolves(fakeRecaptchaAppCheckToken);
300343

301-
addTokenListener(app, fakePlatformLoggingProvider, listener);
344+
addTokenListener(
345+
app,
346+
fakePlatformLoggingProvider,
347+
ListenerType.INTERNAL,
348+
listener
349+
);
302350

303351
expect(getState(app).tokenObservers[0].next).to.equal(listener);
304352
});
@@ -310,7 +358,12 @@ describe('internal api', () => {
310358
expect(getState(app).tokenObservers.length).to.equal(0);
311359
expect(getState(app).tokenRefresher).to.equal(undefined);
312360

313-
addTokenListener(app, fakePlatformLoggingProvider, listener);
361+
addTokenListener(
362+
app,
363+
fakePlatformLoggingProvider,
364+
ListenerType.INTERNAL,
365+
listener
366+
);
314367

315368
expect(getState(app).tokenRefresher?.isRunning()).to.be.true;
316369

@@ -330,7 +383,12 @@ describe('internal api', () => {
330383
}
331384
});
332385

333-
addTokenListener(app, fakePlatformLoggingProvider, listener);
386+
addTokenListener(
387+
app,
388+
fakePlatformLoggingProvider,
389+
ListenerType.INTERNAL,
390+
listener
391+
);
334392
await clock.runAllAsync();
335393
expect(listener).to.be.calledWith({
336394
token: 'fake-memory-app-check-token'
@@ -355,7 +413,12 @@ describe('internal api', () => {
355413
done();
356414
};
357415

358-
addTokenListener(app, fakePlatformLoggingProvider, fakeListener);
416+
addTokenListener(
417+
app,
418+
fakePlatformLoggingProvider,
419+
ListenerType.INTERNAL,
420+
fakeListener
421+
);
359422
});
360423
});
361424

@@ -369,7 +432,12 @@ describe('internal api', () => {
369432
it('should remove token listeners', () => {
370433
stub(client, 'exchangeToken').resolves(fakeRecaptchaAppCheckToken);
371434
const listener = (): void => {};
372-
addTokenListener(app, fakePlatformLoggingProvider, listener);
435+
addTokenListener(
436+
app,
437+
fakePlatformLoggingProvider,
438+
ListenerType.INTERNAL,
439+
listener
440+
);
373441
expect(getState(app).tokenObservers.length).to.equal(1);
374442

375443
removeTokenListener(app, listener);
@@ -381,7 +449,12 @@ describe('internal api', () => {
381449
const listener = (): void => {};
382450
setState(app, { ...getState(app), isTokenAutoRefreshEnabled: true });
383451

384-
addTokenListener(app, fakePlatformLoggingProvider, listener);
452+
addTokenListener(
453+
app,
454+
fakePlatformLoggingProvider,
455+
ListenerType.INTERNAL,
456+
listener
457+
);
385458
expect(getState(app).tokenObservers.length).to.equal(1);
386459
expect(getState(app).tokenRefresher?.isRunning()).to.be.true;
387460

‎packages/app-check/src/internal-api.ts

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
AppCheckTokenInternal,
2626
AppCheckTokenObserver,
2727
getState,
28+
ListenerType,
2829
setState
2930
} from './state';
3031
import { TOKEN_REFRESH_TIME } from './constants';
@@ -176,13 +177,15 @@ export async function getToken(
176177
export function addTokenListener(
177178
app: FirebaseApp,
178179
platformLoggerProvider: Provider<'platform-logger'>,
180+
type: ListenerType,
179181
listener: AppCheckTokenListener,
180182
onError?: (error: Error) => void
181183
): void {
182184
const state = getState(app);
183185
const tokenListener: AppCheckTokenObserver = {
184186
next: listener,
185-
error: onError
187+
error: onError,
188+
type
186189
};
187190
const newState = {
188191
...state,
@@ -304,20 +307,19 @@ function notifyTokenListeners(
304307

305308
for (const observer of observers) {
306309
try {
307-
if (observer.error) {
308-
// If this listener has an error handler, handle errors differently
309-
// from successes.
310-
if (token.error) {
311-
observer.error(token.error);
312-
} else {
313-
observer.next(token);
314-
}
310+
if (observer.type === ListenerType.EXTERNAL && token.error != null) {
311+
// If this listener was added by a 3P call, send any token error to
312+
// the supplied error handler. A 3P observer always has an error
313+
// handler.
314+
observer.error!(token.error);
315315
} else {
316-
// Otherwise return the token, whether or not it has an error field.
316+
// If the token has no error field, always return the token.
317+
// If this is a 2P listener, return the token, whether or not it
318+
// has an error field.
317319
observer.next(token);
318320
}
319321
} catch (ignored) {
320-
// If any handler fails, ignore and run next handler.
322+
// Errors in the listener function itself are always ignored.
321323
}
322324
}
323325
}

‎packages/app-check/src/state.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,12 @@ export interface AppCheckTokenObserver
3434
extends PartialObserver<AppCheckTokenResult> {
3535
// required
3636
next: AppCheckTokenListener;
37+
type: ListenerType;
38+
}
39+
40+
export const enum ListenerType {
41+
'INTERNAL' = 'INTERNAL',
42+
'EXTERNAL' = 'EXTERNAL'
3743
}
3844

3945
export interface AppCheckState {

0 commit comments

Comments
 (0)