fix(auth): retry IndexedDB availability check on startup failure#9845
fix(auth): retry IndexedDB availability check on startup failure#9845macastelaz wants to merge 2 commits intofirebase:mainfrom
Conversation
In environments like Chrome Extension Manifest V3 Service Workers, the initial IndexedDB transaction upon a cold start can sometimes drop or timeout. Previously, `_isAvailable()` would immediately catch the error, assume IndexedDB is broken, and silently fall back to `inMemoryPersistence`. This updates `_isAvailable()` to use the `_withRetries()` helper, ensuring it benefits from the same retry logic as actual storage operations. This prevents the SDK from incorrectly abandoning existing IndexedDB auth state. Fixes firebase#9732
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements retry logic for IndexedDB availability checks by utilizing the _withRetries helper within the _isAvailable method. It also includes new test cases to verify the retry behavior during database creation failures. Feedback was provided regarding a potential off-by-one error in the _withRetries loop condition, which currently requires an unintuitive call count assertion in the tests.
| expect((indexedDB.open as sinon.SinonStub).callCount).to.eq( | ||
| _TRANSACTION_RETRY_COUNT + 2 | ||
| ); |
There was a problem hiding this comment.
The expectation of _TRANSACTION_RETRY_COUNT + 2 (which evaluates to 5 attempts when the retry count is 3) indicates an off-by-one error in the underlying _withRetries helper's loop condition (numAttempts++ > _TRANSACTION_RETRY_COUNT). While _withRetries is not modified in this PR, this magic number in the test makes the logic less intuitive. Consider addressing the discrepancy in _withRetries by changing the condition to numAttempts++ >= _TRANSACTION_RETRY_COUNT so that the number of retries matches the constant's value, and then updating this test to expect _TRANSACTION_RETRY_COUNT + 1 total attempts.
1 participant
In environments like Chrome Extension Manifest V3 Service Workers, the initial IndexedDB transaction upon a cold start can sometimes drop or timeout. Previously,
_isAvailable()would immediately catch the error, assume IndexedDB is broken, and silently fall back toinMemoryPersistence.This updates
_isAvailable()to use the_withRetries()helper, ensuring it benefits from the same retry logic as actual storage operations. This prevents the SDK from incorrectly abandoning existing IndexedDB auth state.Fixes #9732
Hey there! So you want to contribute to a Firebase SDK?
Before you file this pull request, please read these guidelines:
Discussion
If not, go file an issue about this before creating a pull request to discuss.
Testing
API Changes
PR that changes the public API, we would suggest first proposing your change in an
issue so that we can discuss it together.