Skip to content

Commit 58f60d6

Browse files
authored
fix(rtdb): Fixing a token refresh livelock in Cloud Functions (#1234)
1 parent 011c530 commit 58f60d6

File tree

3 files changed

+16
-15
lines changed

3 files changed

+16
-15
lines changed

src/database/database-internal.ts

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -116,18 +116,15 @@ export class DatabaseService {
116116

117117
// eslint-disable-next-line @typescript-eslint/no-unused-vars
118118
private onTokenChange(_: string): void {
119-
this.appInternal.INTERNAL.getToken()
120-
.then((token) => {
121-
const delayMillis = token.expirationTime - TOKEN_REFRESH_THRESHOLD_MILLIS - Date.now();
122-
// If the new token is set to expire soon (unlikely), do nothing. Somebody will eventually
123-
// notice and refresh the token, at which point this callback will fire again.
124-
if (delayMillis > 0) {
125-
this.scheduleTokenRefresh(delayMillis);
126-
}
127-
})
128-
.catch((err) => {
129-
console.error('Unexpected error while attempting to schedule a token refresh:', err);
130-
});
119+
const token = this.appInternal.INTERNAL.getCachedToken();
120+
if (token) {
121+
const delayMillis = token.expirationTime - TOKEN_REFRESH_THRESHOLD_MILLIS - Date.now();
122+
// If the new token is set to expire soon (unlikely), do nothing. Somebody will eventually
123+
// notice and refresh the token, at which point this callback will fire again.
124+
if (delayMillis > 0) {
125+
this.scheduleTokenRefresh(delayMillis);
126+
}
127+
}
131128
}
132129

133130
private scheduleTokenRefresh(delayMillis: number): void {

src/firebase-app.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,10 @@ export class FirebaseAppInternals {
7474
return Promise.resolve(this.cachedToken_);
7575
}
7676

77+
public getCachedToken(): FirebaseAccessToken | null {
78+
return this.cachedToken_ || null;
79+
}
80+
7781
private refreshToken(): Promise<FirebaseAccessToken> {
7882
return Promise.resolve(this.credential_.getAccessToken())
7983
.then((result) => {
@@ -97,6 +101,8 @@ export class FirebaseAppInternals {
97101
if (!this.cachedToken_
98102
|| this.cachedToken_.accessToken !== token.accessToken
99103
|| this.cachedToken_.expirationTime !== token.expirationTime) {
104+
// Update the cache before firing listeners. Listeners may directly query the
105+
// cached token state.
100106
this.cachedToken_ = token;
101107
this.tokenListeners_.forEach((listener) => {
102108
listener(token.accessToken);

test/unit/database/database.spec.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -211,9 +211,7 @@ describe('Database', () => {
211211
});
212212
});
213213

214-
// Currently doesn't work as expected since onTokenChange() can force a token refresh
215-
// by calling getToken(). Skipping for now.
216-
xit('should not reschedule when the token is about to expire in 5 minutes', () => {
214+
it('should not reschedule when the token is about to expire in 5 minutes', () => {
217215
database.getDatabase();
218216
return mockApp.INTERNAL.getToken()
219217
.then((token1) => {

0 commit comments

Comments
 (0)