diff --git a/packages/firestore/CHANGELOG.md b/packages/firestore/CHANGELOG.md index 80f848509ba..cd8cc4c7702 100644 --- a/packages/firestore/CHANGELOG.md +++ b/packages/firestore/CHANGELOG.md @@ -1,4 +1,12 @@ # Unreleased +- [fixed] Fixed a regression introduced in 5.7.0 that caused apps using + experimentalTabSynchronization to hit an exception for "Failed to obtain + primary lease for action 'Collect garbage'". + +# 0.9.1 +- [changed] Added a custom error for schema downgrades. + +# 0.9.0 - [changed] Removed eval()-based fallback for JSON parsing, allowing SDK to be used in environments that prohibit eval(). - [feature] Added a garbage collection process to on-disk persistence that diff --git a/packages/firestore/src/core/firestore_client.ts b/packages/firestore/src/core/firestore_client.ts index fa97bdfd29a..d3e1f51caaa 100644 --- a/packages/firestore/src/core/firestore_client.ts +++ b/packages/firestore/src/core/firestore_client.ts @@ -393,7 +393,6 @@ export class FirestoreClient { this.asyncQueue, this.localStore ); - this.lruScheduler.start(); } const serializer = this.platform.newSerializer( this.databaseInfo.databaseId @@ -444,9 +443,16 @@ export class FirestoreClient { // NOTE: This will immediately call the listener, so we make sure to // set it after localStore / remoteStore are started. - await this.persistence.setPrimaryStateListener(isPrimary => - this.syncEngine.applyPrimaryState(isPrimary) - ); + await this.persistence.setPrimaryStateListener(async isPrimary => { + await this.syncEngine.applyPrimaryState(isPrimary); + if (this.lruScheduler) { + if (isPrimary && !this.lruScheduler.started) { + this.lruScheduler.start(); + } else if (!isPrimary) { + this.lruScheduler.stop(); + } + } + }); }); } diff --git a/packages/firestore/src/core/sync_engine.ts b/packages/firestore/src/core/sync_engine.ts index 5a2e67adca9..b6b62bd7679 100644 --- a/packages/firestore/src/core/sync_engine.ts +++ b/packages/firestore/src/core/sync_engine.ts @@ -40,7 +40,7 @@ import { Deferred } from '../util/promise'; import { SortedMap } from '../util/sorted_map'; import { isNullOrUndefined } from '../util/types'; -import { isPrimaryLeaseLostError } from '../local/indexeddb_persistence'; +import { ignoreIfPrimaryLeaseLoss } from '../local/indexeddb_persistence'; import { isDocumentChangeMissingError } from '../local/indexeddb_remote_document_cache'; import { ClientId, SharedClientState } from '../local/shared_client_state'; import { @@ -324,7 +324,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer { this.remoteStore.unlisten(queryView.targetId); this.removeAndCleanupQuery(queryView); }) - .catch(err => this.ignoreIfPrimaryLeaseLoss(err)); + .catch(ignoreIfPrimaryLeaseLoss); } } else { this.removeAndCleanupQuery(queryView); @@ -464,7 +464,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer { ); return this.emitNewSnapsAndNotifyLocalStore(changes, remoteEvent); }) - .catch(err => this.ignoreIfPrimaryLeaseLoss(err)); + .catch(ignoreIfPrimaryLeaseLoss); } /** @@ -547,7 +547,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer { await this.localStore .releaseQuery(queryView.query, /* keepPersistedQueryData */ false) .then(() => this.removeAndCleanupQuery(queryView)) - .catch(err => this.ignoreIfPrimaryLeaseLoss(err)); + .catch(ignoreIfPrimaryLeaseLoss); this.syncEngineListener!.onWatchError(queryView.query, err); } } @@ -609,7 +609,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer { this.sharedClientState.updateMutationState(batchId, 'acknowledged'); return this.emitNewSnapsAndNotifyLocalStore(changes); }) - .catch(err => this.ignoreIfPrimaryLeaseLoss(err)); + .catch(ignoreIfPrimaryLeaseLoss); } rejectFailedWrite(batchId: BatchId, error: FirestoreError): Promise { @@ -627,7 +627,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer { this.sharedClientState.updateMutationState(batchId, 'rejected', error); return this.emitNewSnapsAndNotifyLocalStore(changes); }) - .catch(err => this.ignoreIfPrimaryLeaseLoss(err)); + .catch(ignoreIfPrimaryLeaseLoss); } private addMutationCallback( @@ -815,24 +815,6 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer { await this.localStore.notifyLocalViewChanges(docChangesInAllViews); } - /** - * Verifies the error thrown by an LocalStore operation. If a LocalStore - * operation fails because the primary lease has been taken by another client, - * we ignore the error (the persistence layer will immediately call - * `applyPrimaryLease` to propagate the primary state change). All other - * errors are re-thrown. - * - * @param err An error returned by a LocalStore operation. - * @return A Promise that resolves after we recovered, or the original error. - */ - private async ignoreIfPrimaryLeaseLoss(err: FirestoreError): Promise { - if (isPrimaryLeaseLostError(err)) { - log.debug(LOG_TAG, 'Unexpectedly lost primary lease'); - } else { - throw err; - } - } - private assertSubscribed(fnName: string): void { assert( this.syncEngineListener !== null, @@ -1068,7 +1050,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer { this.remoteStore.unlisten(targetId); this.removeAndCleanupQuery(queryView); }) - .catch(err => this.ignoreIfPrimaryLeaseLoss(err)); + .catch(ignoreIfPrimaryLeaseLoss); } } } diff --git a/packages/firestore/src/local/indexeddb_persistence.ts b/packages/firestore/src/local/indexeddb_persistence.ts index 50b833cf990..624158e2ed4 100644 --- a/packages/firestore/src/local/indexeddb_persistence.ts +++ b/packages/firestore/src/local/indexeddb_persistence.ts @@ -1045,12 +1045,33 @@ export class IndexedDbPersistence implements Persistence { } } -export function isPrimaryLeaseLostError(err: FirestoreError): boolean { +function isPrimaryLeaseLostError(err: FirestoreError): boolean { return ( err.code === Code.FAILED_PRECONDITION && err.message === PRIMARY_LEASE_LOST_ERROR_MSG ); } + +/** + * Verifies the error thrown by a LocalStore operation. If a LocalStore + * operation fails because the primary lease has been taken by another client, + * we ignore the error (the persistence layer will immediately call + * `applyPrimaryLease` to propagate the primary state change). All other errors + * are re-thrown. + * + * @param err An error returned by a LocalStore operation. + * @return A Promise that resolves after we recovered, or the original error. + */ +export async function ignoreIfPrimaryLeaseLoss( + err: FirestoreError +): Promise { + if (isPrimaryLeaseLostError(err)) { + log.debug(LOG_TAG, 'Unexpectedly lost primary lease'); + } else { + throw err; + } +} + /** * Helper to get a typed SimpleDbStore for the primary client object store. */ diff --git a/packages/firestore/src/local/lru_garbage_collector.ts b/packages/firestore/src/local/lru_garbage_collector.ts index 7088189f5e7..15de0370047 100644 --- a/packages/firestore/src/local/lru_garbage_collector.ts +++ b/packages/firestore/src/local/lru_garbage_collector.ts @@ -23,6 +23,7 @@ import * as log from '../util/log'; import { AnyJs, primitiveComparator } from '../util/misc'; import { CancelablePromise } from '../util/promise'; import { SortedSet } from '../util/sorted_set'; +import { ignoreIfPrimaryLeaseLoss } from './indexeddb_persistence'; import { LocalStore } from './local_store'; import { PersistenceTransaction } from './persistence'; import { PersistencePromise } from './persistence_promise'; @@ -247,6 +248,10 @@ export class LruScheduler { } } + get started(): boolean { + return this.gcTask !== null; + } + private scheduleGC(): void { assert(this.gcTask === null, 'Cannot schedule GC while a task is pending'); const delay = this.hasRun ? REGULAR_GC_DELAY_MS : INITIAL_GC_DELAY_MS; @@ -262,7 +267,8 @@ export class LruScheduler { this.hasRun = true; return this.localStore .collectGarbage(this.garbageCollector) - .then(() => this.scheduleGC()); + .then(() => this.scheduleGC()) + .catch(ignoreIfPrimaryLeaseLoss); } ); } diff --git a/packages/firestore/src/remote/remote_store.ts b/packages/firestore/src/remote/remote_store.ts index 8e88caf6849..97d54cee614 100644 --- a/packages/firestore/src/remote/remote_store.ts +++ b/packages/firestore/src/remote/remote_store.ts @@ -31,7 +31,7 @@ import { Code, FirestoreError } from '../util/error'; import * as log from '../util/log'; import * as objUtils from '../util/obj'; -import { isPrimaryLeaseLostError } from '../local/indexeddb_persistence'; +import { ignoreIfPrimaryLeaseLoss } from '../local/indexeddb_persistence'; import { DocumentKeySet } from '../model/collections'; import { AsyncQueue } from '../util/async_queue'; import { Datastore } from './datastore'; @@ -564,23 +564,7 @@ export class RemoteStore implements TargetMetadataProvider { this.writeStream.writeMutations(batch.mutations); } }) - .catch(err => this.ignoreIfPrimaryLeaseLoss(err)); - } - - /** - * Verifies the error thrown by an LocalStore operation. If a LocalStore - * operation fails because the primary lease has been taken by another client, - * we ignore the error. All other errors are re-thrown. - * - * @param err An error returned by a LocalStore operation. - * @return A Promise that resolves after we recovered, or the original error. - */ - private ignoreIfPrimaryLeaseLoss(err: FirestoreError): void { - if (isPrimaryLeaseLostError(err)) { - log.debug(LOG_TAG, 'Unexpectedly lost primary lease'); - } else { - throw err; - } + .catch(ignoreIfPrimaryLeaseLoss); } private onMutationResult( @@ -656,7 +640,7 @@ export class RemoteStore implements TargetMetadataProvider { return this.localStore .setLastStreamToken(emptyByteString()) - .catch(err => this.ignoreIfPrimaryLeaseLoss(err)); + .catch(ignoreIfPrimaryLeaseLoss); } else { // Some other error, don't reset stream token. Our stream logic will // just retry with exponential backoff.