Make shutdown idempotent #3575
Make shutdown idempotent #3575
Conversation
🦋 Changeset is good to goLatest commit: c383178 We got this. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Binary Size ReportAffected SDKs
Test Logs
|
| // RemoteStore as it will prevent the RemoteStore from retrieving | ||
| // auth tokens. | ||
| this._credentials.removeChangeListener(); | ||
| this._queue.initiateShutdown(); |
There was a problem hiding this comment.
This is essentially a parallel change to the one in the iOS repo (which I hadn't yet ported everywhere else). During that change discussion arose that "shutdown" didn't really capture what was happening anymore: the queue is really still running, but there's just a limited set of operations you can perform on it. This method became enterRestrictedMode and the corresponding enqueue would be enqueueAndForgetEvenWhileRestricted.
Would you be willing to make that renaming change while you're in here? Usage ends up being the same as here: https://github.com/firebase/firebase-ios-sdk/blob/master/Firestore/core/src/core/firestore_client.cc#L230, so structurally this is already a move toward convergence.
There was a problem hiding this comment.
Done. This is a good reminder that I need to port the RemoteStore modes to the other platforms.
…rebase-js-sdk into mrschmidt/muchbettershutdown
The CI doesn't seem to work right now, but I still wanted to send this out for review. This replaces #3164 and makes it possible to call terminate() again if it failed with an IndexedDB exception.
This is the last PR for IndexedDB retries (at least for the current Firestore SDK, firestore-exp has one TODO to use a retryable operation).
Fixes #2755