Skip to content

fix: Decoupled proactive token refresh from FirebaseApp #1194

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.

Sign up for GitHub

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 4 commits into from
Mar 18, 2021
Merged

fix: Decoupled proactive token refresh from FirebaseApp #1194

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5b7a89a
fix: Decoupled proactive token refresh from FirebaseApp
hiranya911 Mar 11, 2021
8674df3
fix: Defined constants for duration values
hiranya911 Mar 12, 2021
e7c3588
fix: Logging errors encountered while scheduling a refresh
hiranya911 Mar 12, 2021
705801a
fix: Renamed some variables for clarity
hiranya911 Mar 18, 2021
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
Prev Previous commit
Next Next commit
fix: Logging errors encountered while scheduling a refresh
  • Loading branch information
@hiranya911
hiranya911 committed Mar 12, 2021
commit e7c3588b2030b1277ae76a9dbf0a4475e9ab2409
8 changes: 3 additions & 5 deletions src/database/database-internal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,17 +126,15 @@ export class DatabaseService {
this.scheduleTokenRefresh(delayMillis);
}
})
// eslint-disable-next-line @typescript-eslint/no-unused-vars
.catch((_) => {
// Unlikely error, since a new token was just fetched before the callback fired.
// Just here to prevent the promise chain from crashing.
.catch((err) => {
console.error('Unexpected error while attempting to schedule a token refresh:', err);
Copy link
Contributor

@schmidt-sebastian schmidt-sebastian Mar 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It think the error most likely occurred during getToken() and not during the scheduling step.

Copy link
Contributor Author

@hiranya911 hiranya911 Mar 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True. But overall this function is attempting to schedule a new token refresh event. It calls getToken() just to figure out when to schedule that event.

});
}

private scheduleTokenRefresh(delayMillis: number): void {
clearTimeout(this.tokenRefreshTimeout);
this.tokenRefreshTimeout = setTimeout(() => {
this.appInternal.INTERNAL.getToken(true)
this.appInternal.INTERNAL.getToken(/*forceRefresh=*/ true)
.catch(() => {
// Ignore the error since this might just be an intermittent failure. If we really cannot
// refresh the token, an error will be logged once the existing token expires and we try
Expand Down