Skip to content

Move ignoreIfPrimaryLeaseLoss to local_store.ts #2544

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.

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 2 commits into from
Jan 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 1 addition & 2 deletions packages/firestore/src/core/sync_engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
*/

import { User } from '../auth/user';
import { LocalStore } from '../local/local_store';
import { ignoreIfPrimaryLeaseLoss, LocalStore } from '../local/local_store';
import { LocalViewChanges } from '../local/local_view_changes';
import { ReferenceSet } from '../local/reference_set';
import { TargetData, TargetPurpose } from '../local/target_data';
Expand All @@ -40,7 +40,6 @@ import { ObjectMap } from '../util/obj_map';
import { Deferred } from '../util/promise';
import { SortedMap } from '../util/sorted_map';

import { ignoreIfPrimaryLeaseLoss } from '../local/indexeddb_persistence';
import { ClientId, SharedClientState } from '../local/shared_client_state';
import {
QueryTargetState,
Expand Down
31 changes: 1 addition & 30 deletions packages/firestore/src/local/indexeddb_persistence.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ import {
Persistence,
PersistenceTransaction,
PersistenceTransactionMode,
PRIMARY_LEASE_LOST_ERROR_MSG,
PrimaryStateListener,
ReferenceDelegate
} from './persistence';
Expand Down Expand Up @@ -97,9 +98,6 @@ const MAX_PRIMARY_ELIGIBLE_AGE_MS = 5000;
*/
const CLIENT_METADATA_REFRESH_INTERVAL_MS = 4000;
/** User-facing error when the primary lease is required but not available. */
const PRIMARY_LEASE_LOST_ERROR_MSG =
'The current tab is not in the required state to perform this operation. ' +
'It might be necessary to refresh the browser tab.';
const PRIMARY_LEASE_EXCLUSIVE_ERROR_MSG =
'Another tab has exclusive access to the persistence layer. ' +
'To allow shared access, make sure to invoke ' +
Expand Down Expand Up @@ -1052,33 +1050,6 @@ export class IndexedDbPersistence implements Persistence {
}
}

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<void> {
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.
*/
Expand Down
30 changes: 29 additions & 1 deletion packages/firestore/src/local/local_store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import {
} from '../model/mutation_batch';
import { RemoteEvent, TargetChange } from '../remote/remote_event';
import { assert } from '../util/assert';
import { Code, FirestoreError } from '../util/error';
import * as log from '../util/log';
import { primitiveComparator } from '../util/misc';
import * as objUtils from '../util/obj';
Expand All @@ -49,7 +50,11 @@ import { LocalViewChanges } from './local_view_changes';
import { LruGarbageCollector, LruResults } from './lru_garbage_collector';
import { IndexedDbRemoteDocumentCache } from './indexeddb_remote_document_cache';
import { MutationQueue } from './mutation_queue';
import { Persistence, PersistenceTransaction } from './persistence';
import {
Persistence,
PersistenceTransaction,
PRIMARY_LEASE_LOST_ERROR_MSG
} from './persistence';
import { PersistencePromise } from './persistence_promise';
import { TargetCache } from './target_cache';
import { QueryEngine } from './query_engine';
Expand Down Expand Up @@ -1121,3 +1126,26 @@ export class LocalStore {
}
}
}

/**
* 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<void> {
if (
err.code === Code.FAILED_PRECONDITION &&
err.message === PRIMARY_LEASE_LOST_ERROR_MSG
) {
log.debug(LOG_TAG, 'Unexpectedly lost primary lease');
} else {
throw err;
}
}
3 changes: 1 addition & 2 deletions packages/firestore/src/local/lru_garbage_collector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ import { primitiveComparator } from '../util/misc';
import { CancelablePromise } from '../util/promise';
import { SortedMap } from '../util/sorted_map';
import { SortedSet } from '../util/sorted_set';
import { ignoreIfPrimaryLeaseLoss } from './indexeddb_persistence';
import { LocalStore } from './local_store';
import { ignoreIfPrimaryLeaseLoss, LocalStore } from './local_store';
import { PersistenceTransaction } from './persistence';
import { PersistencePromise } from './persistence_promise';
import { TargetData } from './target_data';
Expand Down
4 changes: 4 additions & 0 deletions packages/firestore/src/local/persistence.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ import { RemoteDocumentCache } from './remote_document_cache';
import { ClientId } from './shared_client_state';
import { TargetData } from './target_data';

export const PRIMARY_LEASE_LOST_ERROR_MSG =
'The current tab is not in the required state to perform this operation. ' +
'It might be necessary to refresh the browser tab.';

/**
* A base class representing a persistence transaction, encapsulating both the
* transaction's sequence numbers as well as a list of onCommitted listeners.
Expand Down
3 changes: 1 addition & 2 deletions packages/firestore/src/remote/remote_store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import { SnapshotVersion } from '../core/snapshot_version';
import { Transaction } from '../core/transaction';
import { OnlineState, TargetId } from '../core/types';
import { LocalStore } from '../local/local_store';
import { ignoreIfPrimaryLeaseLoss, LocalStore } from '../local/local_store';
import { TargetData, TargetPurpose } from '../local/target_data';
import { MutationResult } from '../model/mutation';
import {
Expand All @@ -32,7 +32,6 @@ import { FirestoreError } from '../util/error';
import * as log from '../util/log';
import * as objUtils from '../util/obj';

import { ignoreIfPrimaryLeaseLoss } from '../local/indexeddb_persistence';
import { DocumentKeySet } from '../model/collections';
import { AsyncQueue } from '../util/async_queue';
import { ConnectivityMonitor, NetworkStatus } from './connectivity_monitor';
Expand Down
2 changes: 1 addition & 1 deletion packages/firestore/test/integration/api/database.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -814,7 +814,7 @@ apiDescribe('Database', (persistence: boolean) => {
// eslint-disable-next-line no-restricted-properties
describe.skip('Listens are rejected remotely:', () => {
//eslint-disable-next-line @typescript-eslint/no-explicit-any
const queryForRejection : firestore.Query = null as any;
const queryForRejection: firestore.Query = null as any;

it('will reject listens', () => {
const deferred = new Deferred();
Expand Down