diff --git a/packages/firestore/src/local/indexeddb_persistence.ts b/packages/firestore/src/local/indexeddb_persistence.ts index 5a889e4a3c9..f6aabeba18f 100644 --- a/packages/firestore/src/local/indexeddb_persistence.ts +++ b/packages/firestore/src/local/indexeddb_persistence.ts @@ -121,6 +121,15 @@ export class IndexedDbTransaction extends PersistenceTransaction { } } +// The different modes supported by `IndexedDbPersistence.runTransaction()` +type IndexedDbTransactionMode = + | 'readonly' + | 'readwrite' + | 'readwrite-primary' + | 'readonly-idempotent' + | 'readwrite-idempotent' + | 'readwrite-primary-idempotent'; + /** * An IndexedDB-backed instance of Persistence. Data is stored persistently * across sessions. @@ -742,17 +751,28 @@ export class IndexedDbPersistence implements Persistence { runTransaction( action: string, - mode: 'readonly' | 'readwrite' | 'readwrite-primary', + mode: IndexedDbTransactionMode, transactionOperation: ( transaction: PersistenceTransaction ) => PersistencePromise ): Promise { log.debug(LOG_TAG, 'Starting transaction:', action); + // TODO(schmidt-sebastian): Simplify once all transactions are idempotent. + const idempotent = mode.endsWith('idempotent'); + const readonly = mode.startsWith('readonly'); + const simpleDbMode = readonly + ? idempotent + ? 'readonly-idempotent' + : 'readonly' + : idempotent + ? 'readwrite-idempotent' + : 'readwrite'; + // Do all transactions as readwrite against all object stores, since we // are the only reader/writer. return this.simpleDb.runTransaction( - mode === 'readonly' ? 'readonly' : 'readwrite', + simpleDbMode, ALL_STORES, simpleDbTxn => { if (mode === 'readwrite-primary') { diff --git a/packages/firestore/src/local/simple_db.ts b/packages/firestore/src/local/simple_db.ts index 70cfdc784bc..5899dffade6 100644 --- a/packages/firestore/src/local/simple_db.ts +++ b/packages/firestore/src/local/simple_db.ts @@ -25,6 +25,19 @@ import { PersistencePromise } from './persistence_promise'; const LOG_TAG = 'SimpleDb'; +/** + * The maximum number of retry attempts for an IndexedDb transaction that fails + * with a DOMException. + */ +const TRANSACTION_RETRY_COUNT = 3; + +// The different modes supported by `SimpleDb.runTransaction()` +type SimpleDbTransactionMode = + | 'readonly' + | 'readwrite' + | 'readonly-idempotent' + | 'readwrite-idempotent'; + export interface SimpleDbSchemaConverter { createOrUpgrade( db: IDBDatabase, @@ -242,32 +255,64 @@ export class SimpleDb { }; } - runTransaction( - mode: 'readonly' | 'readwrite', + async runTransaction( + mode: SimpleDbTransactionMode, objectStores: string[], transactionFn: (transaction: SimpleDbTransaction) => PersistencePromise ): Promise { - const transaction = SimpleDbTransaction.open(this.db, mode, objectStores); - const transactionFnResult = transactionFn(transaction) - .catch(error => { - // Abort the transaction if there was an error. - transaction.abort(error); - // We cannot actually recover, and calling `abort()` will cause the transaction's - // completion promise to be rejected. This in turn means that we won't use - // `transactionFnResult` below. We return a rejection here so that we don't add the - // possibility of returning `void` to the type of `transactionFnResult`. - return PersistencePromise.reject(error); - }) - .toPromise(); - - // As noted above, errors are propagated by aborting the transaction. So - // we swallow any error here to avoid the browser logging it as unhandled. - transactionFnResult.catch(() => {}); - - // Wait for the transaction to complete (i.e. IndexedDb's onsuccess event to - // fire), but still return the original transactionFnResult back to the - // caller. - return transaction.completionPromise.then(() => transactionFnResult); + const readonly = mode.startsWith('readonly'); + const idempotent = mode.endsWith('idempotent'); + let attemptNumber = 0; + + while (true) { + ++attemptNumber; + + const transaction = SimpleDbTransaction.open( + this.db, + readonly ? 'readonly' : 'readwrite', + objectStores + ); + try { + const transactionFnResult = transactionFn(transaction) + .catch(error => { + // Abort the transaction if there was an error. + transaction.abort(error); + // We cannot actually recover, and calling `abort()` will cause the transaction's + // completion promise to be rejected. This in turn means that we won't use + // `transactionFnResult` below. We return a rejection here so that we don't add the + // possibility of returning `void` to the type of `transactionFnResult`. + return PersistencePromise.reject(error); + }) + .toPromise(); + + // As noted above, errors are propagated by aborting the transaction. So + // we swallow any error here to avoid the browser logging it as unhandled. + transactionFnResult.catch(() => {}); + + // Wait for the transaction to complete (i.e. IndexedDb's onsuccess event to + // fire), but still return the original transactionFnResult back to the + // caller. + await transaction.completionPromise; + return transactionFnResult; + } catch (e) { + // TODO(schmidt-sebastian): We could probably be smarter about this and + // not retry exceptions that are likely unrecoverable (such as quota + // exceeded errors). + const retryable = + idempotent && + isDomException(e) && + attemptNumber < TRANSACTION_RETRY_COUNT; + debug( + 'Transaction failed with error: %s. Retrying: %s.', + e.message, + retryable + ); + + if (!retryable) { + return Promise.reject(e); + } + } + } } close(): void { @@ -755,3 +800,13 @@ function checkForAndReportiOSError(error: DOMException): Error { } return error; } + +/** Checks whether an error is a DOMException (e.g. as thrown by IndexedDb). */ +function isDomException(error: Error): boolean { + // DOMException is not a global type in Node with persistence, and hence we + // check the constructor name if the type in unknown. + return ( + (typeof DOMException !== 'undefined' && error instanceof DOMException) || + error.constructor.name === 'DOMException' + ); +} diff --git a/packages/firestore/test/unit/local/simple_db.test.ts b/packages/firestore/test/unit/local/simple_db.test.ts index 57df764657a..53d7421d36f 100644 --- a/packages/firestore/test/unit/local/simple_db.test.ts +++ b/packages/firestore/test/unit/local/simple_db.test.ts @@ -500,6 +500,84 @@ describe('SimpleDb', () => { }); }); + it('retries transactions marked as idempotent', async () => { + let attemptCount = 0; + + const result = await db.runTransaction( + 'readwrite-idempotent', + ['users'], + txn => { + ++attemptCount; + if (attemptCount === 1) { + const store = txn.store('users'); + return store + .add(dummyUser) + .next(() => { + return store.add(dummyUser); // Fails with a unique key violation + }) + .next(() => 'Aborted'); + } else { + return PersistencePromise.resolve('success'); + } + } + ); + + expect(result).to.equal('success'); + expect(attemptCount).to.equal(2); + }); + + it('retries transactions only three times', async () => { + let attemptCount = 0; + + await expect( + db.runTransaction('readwrite-idempotent', ['users'], txn => { + ++attemptCount; + const store = txn.store('users'); + return store + .add(dummyUser) + .next(() => { + return store.add(dummyUser); // Fails with a unique key violation + }) + .next(() => 'Aborted'); + }) + ).to.eventually.be.rejected; + + expect(attemptCount).to.equal(3); + }); + + it('does not retry explicitly aborted transactions', async () => { + let attemptCount = 0; + + await expect( + db.runTransaction('readwrite-idempotent', ['users'], txn => { + ++attemptCount; + txn.abort(); + return PersistencePromise.reject(new Error('Aborted')); + }) + ).to.eventually.be.rejected; + + expect(attemptCount).to.equal(1); + }); + + it('does not retry non-idempotent transactions', async () => { + let attemptCount = 0; + + await expect( + db.runTransaction('readwrite', ['users'], txn => { + ++attemptCount; + const store = txn.store('users'); + return store + .add(dummyUser) + .next(() => { + return store.add(dummyUser); // Fails with a unique key violation + }) + .next(() => 'Aborted'); + }) + ).to.eventually.be.rejected; + + expect(attemptCount).to.equal(1); + }); + // A little perf test for convenient benchmarking // eslint-disable-next-line no-restricted-properties it.skip('Perf', () => {