From 5c77e0cbdb1ac0a940337d6bed13135fadfc6b9b Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 4 Mar 2020 15:55:16 -0800 Subject: [PATCH 1/4] feat: use error codes for transaction retries --- dev/src/transaction.ts | 61 +++++-- dev/system-test/firestore.ts | 73 ++++++-- dev/test/transaction.ts | 336 +++++++++++++++++++++++------------ 3 files changed, 321 insertions(+), 149 deletions(-) diff --git a/dev/src/transaction.ts b/dev/src/transaction.ts index 925ec73e5..4965661ae 100644 --- a/dev/src/transaction.ts +++ b/dev/src/transaction.ts @@ -18,6 +18,7 @@ import {GoogleError, Status} from 'google-gax'; import * as proto from '../protos/firestore_v1_proto_api'; +import {ExponentialBackoff} from './backoff'; import {DocumentSnapshot, Precondition} from './document'; import {Firestore, WriteBatch} from './index'; import {logger} from './logger'; @@ -64,6 +65,7 @@ const READ_AFTER_WRITE_ERROR_MSG = export class Transaction { private _firestore: Firestore; private _writeBatch: WriteBatch; + private _backoff: ExponentialBackoff; private _requestTag: string; private _transactionId?: Uint8Array; @@ -78,6 +80,7 @@ export class Transaction { this._firestore = firestore; this._writeBatch = firestore.batch(); this._requestTag = requestTag; + this._backoff = new ExponentialBackoff(); } /** @@ -407,7 +410,7 @@ export class Transaction { maxAttempts: number ): Promise { let result: T; - let lastError: Error | undefined = undefined; + let lastError: GoogleError | undefined = undefined; for (let attempt = 0; attempt < maxAttempts; ++attempt) { if (lastError) { @@ -419,6 +422,9 @@ export class Transaction { ); } + this._writeBatch._reset(); + await this.maybeBackoff(lastError); + await this.begin(); try { @@ -429,6 +435,8 @@ export class Transaction { ); } result = await promise; + await this.commit(); + return result; } catch (err) { logger( 'Firestore.runTransaction', @@ -441,19 +449,10 @@ export class Transaction { if (isRetryableTransactionError(err)) { lastError = err; - continue; // Retry full transaction } else { return Promise.reject(err); // Callback failed w/ non-retryable error } } - - try { - await this.commit(); - return result; // Success - } catch (err) { - lastError = err; - this._writeBatch._reset(); - } } logger( @@ -464,6 +463,25 @@ export class Transaction { ); return Promise.reject(lastError); } + + /** + * Delays further operations based on the provided error. + * + * @private + * @return A Promise that resolves after the delay expired. + */ + private async maybeBackoff(error?: GoogleError) { + if (error) { + if (error.code === Status.RESOURCE_EXHAUSTED) { + this._backoff.resetToMax(); + } else if (error.code === Status.ABORTED) { + // We don't backoff for ABORTED to avoid starvation + this._backoff.reset(); + } + } + + await this._backoff.backoffAndWait(); + } } /** @@ -562,13 +580,22 @@ function validateReadOptions( } } -function isRetryableTransactionError(error: Error): boolean { - if (error instanceof GoogleError || 'code' in error) { - // In transactions, the backend returns code ABORTED for reads that fail - // with contention. These errors should be retried for both GoogleError - // and GoogleError-alike errors (in case the prototype hierarchy gets - // stripped somewhere). - return error.code === Status.ABORTED; +function isRetryableTransactionError(error: GoogleError): boolean { + if (error.code !== undefined) { + // This list is based on https://github.com/firebase/firebase-js-sdk/blob/master/packages/firestore/src/core/transaction_runner.ts#L112 + switch (error.code) { + case Status.ABORTED: + case Status.CANCELLED: + case Status.UNKNOWN: + case Status.DEADLINE_EXCEEDED: + case Status.INTERNAL: + case Status.UNAVAILABLE: + case Status.UNAUTHENTICATED: + case Status.RESOURCE_EXHAUSTED: + return true; + default: + return false; + } } return false; } diff --git a/dev/system-test/firestore.ts b/dev/system-test/firestore.ts index be23e3ad4..b54eeb493 100644 --- a/dev/system-test/firestore.ts +++ b/dev/system-test/firestore.ts @@ -12,12 +12,12 @@ // See the License for the specific language governing permissions and // limitations under the License. -import {expect} from 'chai'; +import {expect, use} from 'chai'; +import * as chaiAsPromised from 'chai-as-promised'; import { CollectionReference, DocumentData, - DocumentReference, DocumentSnapshot, FieldPath, FieldValue, @@ -33,6 +33,8 @@ import { import {autoId, Deferred} from '../src/util'; import {Post, postConverter, verifyInstance} from '../test/util/helpers'; +use(chaiAsPromised); + const version = require('../../package.json').version; class DeferredPromise { @@ -1991,21 +1993,6 @@ describe('Transaction class', () => { }); }); - it('enforces that updated document exists', () => { - const ref = firestore.collection('col').doc(); - return firestore - .runTransaction(updateFunction => { - updateFunction.update(ref, {foo: 'b'}); - return Promise.resolve(); - }) - .then(() => { - expect.fail(); - }) - .catch(err => { - expect(err.message).to.match(/No document to update/); - }); - }); - it('has delete() method', () => { let success = false; const ref = randomCol.doc('doc'); @@ -2026,6 +2013,58 @@ describe('Transaction class', () => { expect(result.exists).to.be.false; }); }); + + it('does not retry transaction that fail with FAILED_PRECONDITION', async () => { + const ref = firestore.collection('col').doc(); + + let attempts = 0; + + await expect( + firestore.runTransaction(async transaction => { + ++attempts; + transaction.update(ref, {foo: 'b'}); + }) + ).to.eventually.be.rejectedWith('No document to update'); + + expect(attempts).to.equal(1); + }); + + it('retries transactions that fail with contention', async () => { + const ref = randomCol.doc('doc'); + + let firstTransaction, secondTransaction: Promise; + let firstTransactionAttempts = 0, + secondTransactionAttempts = 0; + + // Create two transactions that both read and update the same document. + // `contentionPromise` is used to ensure that both transactions are active + // when we try to commit, which causes the second transaction to fail with + // Code ABORTED and be retried. + const contentionPromise = new Deferred(); + + firstTransaction = firestore.runTransaction(async transaction => { + ++firstTransactionAttempts; + await transaction.get(ref); + await contentionPromise.promise; + transaction.set(ref, {first: true}, {merge: true}); + }); + + secondTransaction = firestore.runTransaction(async transaction => { + ++secondTransactionAttempts; + await transaction.get(ref); + contentionPromise.resolve(); + transaction.set(ref, {second: true}, {merge: true}); + }); + + await firstTransaction; + await secondTransaction; + + expect(firstTransactionAttempts).to.equal(1); + expect(secondTransactionAttempts).to.equal(2); + + const finalSnapshot = await ref.get(); + expect(finalSnapshot.data()).to.deep.equal({first: true, second: true}); + }); }); describe('WriteBatch class', () => { diff --git a/dev/test/transaction.ts b/dev/test/transaction.ts index e25f400f9..1e5e94032 100644 --- a/dev/test/transaction.ts +++ b/dev/test/transaction.ts @@ -22,6 +22,7 @@ import * as through2 from 'through2'; import * as proto from '../protos/firestore_v1_proto_api'; import * as Firestore from '../src'; import {DocumentReference, FieldPath, Transaction} from '../src'; +import {setTimeoutHandler} from '../src/backoff'; import { ApiOverride, createInstance, @@ -36,7 +37,8 @@ use(chaiAsPromised); const PROJECT_ID = 'test-project'; const DATABASE_ROOT = `projects/${PROJECT_ID}/databases/(default)`; const COLLECTION_ROOT = `${DATABASE_ROOT}/documents/collectionId`; -const DOCUMENT_NAME = `${COLLECTION_ROOT}/documentId`; +const DOCUMENT_ID = 'documentId'; +const DOCUMENT_NAME = `${COLLECTION_ROOT}/${DOCUMENT_ID}`; // Change the argument to 'console.log' to enable debug output. Firestore.setLogFunction(() => {}); @@ -57,8 +59,9 @@ function transactionId(transaction?: Uint8Array | string): Uint8Array { * format defines an expected request and its expected response or error code. */ interface TransactionStep { - type: 'begin' | 'getDocument' | 'query' | 'commit' | 'rollback'; - request: + type: 'begin' | 'getDocument' | 'query' | 'commit' | 'rollback' | 'backoff'; + delay?: 'exponential' | 'max'; + request?: | api.ICommitRequest | api.IBeginTransactionRequest | api.IRunQueryRequest; @@ -148,43 +151,16 @@ function begin( }; } -function getDocument( +function getAll( + docs: string[], + fieldMask?: string[], transaction?: Uint8Array | string, error?: Error ): TransactionStep { - const request = { - database: DATABASE_ROOT, - documents: [DOCUMENT_NAME], - transaction: transactionId(transaction), - }; - - const stream = through2.obj(); - - setImmediate(() => { - stream.push({ - found: { - name: DOCUMENT_NAME, - createTime: {seconds: 1, nanos: 2}, - updateTime: {seconds: 3, nanos: 4}, - }, - readTime: {seconds: 5, nanos: 6}, - }); - stream.push(null); - }); - - return { - type: 'getDocument', - request, - error, - stream, - }; -} - -function getAll(docs: string[], fieldMask?: string[]): TransactionStep { const request: api.IBatchGetDocumentsRequest = { database: DATABASE_ROOT, documents: [], - transaction: Buffer.from('foo'), + transaction: transactionId(transaction), }; if (fieldMask) { @@ -210,17 +186,32 @@ function getAll(docs: string[], fieldMask?: string[]): TransactionStep { } setImmediate(() => { - stream.push(null); + if (error) { + stream.destroy(error); + } else { + stream.push(null); + } }); return { type: 'getDocument', request, + error, stream, }; } -function query(transaction?: Uint8Array): TransactionStep { +function getDocument( + transaction?: Uint8Array | string, + error?: Error +): TransactionStep { + return getAll([DOCUMENT_ID], undefined, transaction, error); +} + +function query( + transaction?: Uint8Array | string, + error?: Error +): TransactionStep { const request = { parent: `${DATABASE_ROOT}/documents`, structuredQuery: { @@ -241,12 +232,14 @@ function query(transaction?: Uint8Array): TransactionStep { }, }, }, - transaction: transaction || Buffer.from('foo'), + transaction: transactionId(transaction), }; const stream = through2.obj(); setImmediate(() => { + // Push a single result even for errored queries, as this avoids implicit + // stream retries. stream.push({ document: { name: DOCUMENT_NAME, @@ -256,7 +249,11 @@ function query(transaction?: Uint8Array): TransactionStep { readTime: {seconds: 5, nanos: 6}, }); - stream.push(null); + if (error) { + stream.destroy(error); + } else { + stream.push(null); + } }); return { @@ -266,6 +263,13 @@ function query(transaction?: Uint8Array): TransactionStep { }; } +function backoff(maxDelay?: boolean): TransactionStep { + return { + type: 'backoff', + delay: maxDelay ? 'max' : 'exponential', + }; +} + /** * Asserts that the given transaction function issues the expected requests. */ @@ -274,8 +278,11 @@ function runTransaction( transaction: Transaction, docRef: DocumentReference ) => Promise, - ...expectedRequests: TransactionStep[] + ...expectedRequests: Array ) { + // Filter out empty steps + expectedRequests = expectedRequests.filter(v => v !== undefined); + const overrides: ApiOverride = { beginTransaction: actual => { const request = expectedRequests.shift()!; @@ -311,11 +318,7 @@ function runTransaction( const request = expectedRequests.shift()!; expect(request.type).to.equal('getDocument'); expect(actual).to.deep.eq(request.request); - if (request.error) { - throw request.error; - } else { - return request.stream!; - } + return request.stream!; }, runQuery: actual => { const request = expectedRequests.shift()!; @@ -326,20 +329,31 @@ function runTransaction( }, }; - return createInstance(overrides).then(firestore => { - return firestore - .runTransaction(transaction => { + return createInstance(overrides).then(async firestore => { + const defaultTimeoutHandler = setTimeout; + + try { + setTimeoutHandler((callback, timeout) => { + if (timeout > 0) { + const request = expectedRequests.shift()!; + expect(request.type).to.equal('backoff'); + if (request.delay === 'max') { + // Make sure that the delay is at least 30 seconds, which is based + // on the maximum delay of 60 second and a jitter factor of 50%. + expect(timeout).to.not.be.lessThan(30 * 1000); + } + } + callback(); + }); + + return await firestore.runTransaction(transaction => { const docRef = firestore.doc('collectionId/documentId'); return transactionCallback(transaction, docRef); - }) - .then(val => { - expect(expectedRequests.length).to.equal(0); - return val; - }) - .catch(err => { - expect(expectedRequests.length).to.equal(0); - return Promise.reject(err); }); + } finally { + expect(expectedRequests.length).to.equal(0); + setTimeoutHandler(defaultTimeoutHandler); + } }); } @@ -368,6 +382,132 @@ describe('successful transactions', () => { }); describe('failed transactions', () => { + type TestCase = {retry: false} | {retry: true; backoff: boolean}; + + const errorBehavior: {[code: number]: TestCase} = { + [Status.CANCELLED]: {retry: true, backoff: true}, + [Status.UNKNOWN]: {retry: true, backoff: true}, + [Status.INVALID_ARGUMENT]: {retry: false}, + [Status.DEADLINE_EXCEEDED]: {retry: true, backoff: true}, + [Status.NOT_FOUND]: {retry: false}, + [Status.ALREADY_EXISTS]: {retry: false}, + [Status.RESOURCE_EXHAUSTED]: {retry: true, backoff: true}, + [Status.FAILED_PRECONDITION]: {retry: false}, + [Status.ABORTED]: {retry: true, backoff: false}, + [Status.OUT_OF_RANGE]: {retry: false}, + [Status.UNIMPLEMENTED]: {retry: false}, + [Status.INTERNAL]: {retry: true, backoff: true}, + [Status.UNAVAILABLE]: {retry: true, backoff: true}, + [Status.DATA_LOSS]: {retry: false}, + [Status.UNAUTHENTICATED]: {retry: true, backoff: true}, + }; + + it('retries commit based on error code', async () => { + const transactionFunction = () => Promise.resolve(); + + for (const errorCode of Object.keys(errorBehavior)) { + const retryBehavior = errorBehavior[Number(errorCode)]; + const serverError = new GoogleError('Test Error'); + serverError.code = Number(errorCode) as Status; + + if (retryBehavior.retry) { + await runTransaction( + transactionFunction, + begin('foo1'), + commit('foo1', undefined, serverError), + rollback('foo1'), + retryBehavior.backoff ? backoff() : undefined, + begin('foo2', 'foo1'), + commit('foo2') + ); + } else { + await expect( + runTransaction( + transactionFunction, + begin('foo1'), + commit('foo1', undefined, serverError), + rollback('foo1') + ) + ).to.eventually.be.rejected; + } + } + }); + + it('retries runQuery based on error code', async () => { + const transactionFunction = ( + transaction: Transaction, + docRef: DocumentReference + ) => { + const query = docRef.parent.where('foo', '==', 'bar'); + return transaction.get(query); + }; + + for (const errorCode of Object.keys(errorBehavior)) { + const retryBehavior = errorBehavior[Number(errorCode)]; + const serverError = new GoogleError('Test Error'); + serverError.code = Number(errorCode) as Status; + + if (retryBehavior.retry) { + await runTransaction( + transactionFunction, + begin('foo1'), + query('foo1', serverError), + rollback('foo1'), + retryBehavior.backoff ? backoff() : undefined, + begin('foo2', 'foo1'), + query('foo2'), + commit('foo2') + ); + } else { + await expect( + runTransaction( + transactionFunction, + begin('foo1'), + query('foo1', serverError), + rollback('foo1') + ) + ).to.eventually.be.rejected; + } + } + }); + + it('retries batchGetDocuments based on error code', async () => { + const transactionFunction = ( + transaction: Transaction, + docRef: DocumentReference + ) => { + return transaction.get(docRef); + }; + + for (const errorCode of Object.keys(errorBehavior)) { + const retryBehavior = errorBehavior[Number(errorCode)]; + const serverError = new GoogleError('Test Error'); + serverError.code = Number(errorCode) as Status; + + if (retryBehavior.retry) { + await runTransaction( + transactionFunction, + begin('foo1'), + getDocument('foo1', serverError), + rollback('foo1'), + retryBehavior.backoff ? backoff() : undefined, + begin('foo2', 'foo1'), + getDocument('foo2'), + commit('foo2') + ); + } else { + await expect( + runTransaction( + transactionFunction, + begin('foo1'), + getDocument('foo1', serverError), + rollback('foo1') + ) + ).to.eventually.be.rejected; + } + } + }); + it('requires update function', () => { const overrides: ApiOverride = { beginTransaction: () => Promise.reject(), @@ -424,43 +564,6 @@ describe('failed transactions', () => { }); }); - it('retries GRPC exceptions with code ABORTED in callback', () => { - const retryableError = new GoogleError('Aborted'); - retryableError.code = Status.ABORTED; - - return runTransaction( - async (transaction, docRef) => { - await transaction.get(docRef); - return 'success'; - }, - begin('foo1'), - getDocument('foo1', retryableError), - rollback('foo1'), - begin('foo2', 'foo1'), - getDocument('foo2'), - commit('foo2') - ).then(res => { - expect(res).to.equal('success'); - }); - }); - - it("doesn't retry GRPC exceptions with code FAILED_PRECONDITION in callback", () => { - const nonRetryableError = new GoogleError('Failed Precondition'); - nonRetryableError.code = Status.FAILED_PRECONDITION; - - return expect( - runTransaction( - async (transaction, docRef) => { - await transaction.get(docRef); - return 'failure'; - }, - begin('foo'), - getDocument('foo', nonRetryableError), - rollback('foo') - ) - ).to.eventually.be.rejectedWith('Failed Precondition'); - }); - it("doesn't retry custom user exceptions in callback", () => { return expect( runTransaction( @@ -473,48 +576,51 @@ describe('failed transactions', () => { ).to.eventually.be.rejectedWith('request exception'); }); - it('retries on commit failure', () => { - const userResult = ['failure', 'failure', 'success']; - const serverError = new Error('Retryable error'); - - return runTransaction( - () => { - return Promise.resolve(userResult.shift()); - }, - begin('foo1'), - commit('foo1', [], serverError), - begin('foo2', 'foo1'), - commit('foo2', [], serverError), - begin('foo3', 'foo2'), - commit('foo3') - ).then(res => { - expect(res).to.equal('success'); - }); - }); - it('limits the retry attempts', () => { const err = new GoogleError('Server disconnect'); err.code = Status.UNAVAILABLE; return expect( runTransaction( - () => { - return Promise.resolve('success'); - }, + () => Promise.resolve(), begin('foo1'), commit('foo1', [], err), + rollback('foo1'), + backoff(), begin('foo2', 'foo1'), commit('foo2', [], err), + rollback('foo2'), + backoff(), begin('foo3', 'foo2'), commit('foo3', [], err), + rollback('foo3'), + backoff(), begin('foo4', 'foo3'), commit('foo4', [], err), + rollback('foo4'), + backoff(), begin('foo5', 'foo4'), - commit('foo5', [], new Error('Final exception')) + commit('foo5', [], new Error('Final exception')), + rollback('foo5') ) ).to.eventually.be.rejectedWith('Final exception'); }); + it('uses maximum backoff for RESOURCE_EXHAUSTED', () => { + const err = new GoogleError('Server disconnect'); + err.code = Status.RESOURCE_EXHAUSTED; + + return runTransaction( + async () => {}, + begin('foo1'), + commit('foo1', [], err), + rollback('foo1'), + backoff(/* maxDelay= */ true), + begin('foo2', 'foo1'), + commit('foo2') + ); + }); + it('fails on rollback', () => { return expect( runTransaction( From 9d170ce1d225401c1d7b84661fc86ed382e9e5c6 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 4 Mar 2020 19:00:23 -0800 Subject: [PATCH 2/4] Fix test --- dev/system-test/firestore.ts | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/dev/system-test/firestore.ts b/dev/system-test/firestore.ts index b54eeb493..905eb0b33 100644 --- a/dev/system-test/firestore.ts +++ b/dev/system-test/firestore.ts @@ -2033,24 +2033,23 @@ describe('Transaction class', () => { const ref = randomCol.doc('doc'); let firstTransaction, secondTransaction: Promise; - let firstTransactionAttempts = 0, - secondTransactionAttempts = 0; + let attempts = 0; // Create two transactions that both read and update the same document. // `contentionPromise` is used to ensure that both transactions are active - // when we try to commit, which causes the second transaction to fail with - // Code ABORTED and be retried. + // on commit, which causes one of transactions to fail with Code ABORTED + // and be retried. const contentionPromise = new Deferred(); firstTransaction = firestore.runTransaction(async transaction => { - ++firstTransactionAttempts; + ++attempts; await transaction.get(ref); await contentionPromise.promise; transaction.set(ref, {first: true}, {merge: true}); }); secondTransaction = firestore.runTransaction(async transaction => { - ++secondTransactionAttempts; + ++attempts; await transaction.get(ref); contentionPromise.resolve(); transaction.set(ref, {second: true}, {merge: true}); @@ -2059,8 +2058,7 @@ describe('Transaction class', () => { await firstTransaction; await secondTransaction; - expect(firstTransactionAttempts).to.equal(1); - expect(secondTransactionAttempts).to.equal(2); + expect(attempts).to.equal(3); const finalSnapshot = await ref.get(); expect(finalSnapshot.data()).to.deep.equal({first: true, second: true}); From 42b3ac8e020de24cdf16a97c31b5dbd39de79b03 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 4 Mar 2020 19:57:46 -0800 Subject: [PATCH 3/4] Typo --- dev/test/transaction.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/test/transaction.ts b/dev/test/transaction.ts index 1e5e94032..099113b9c 100644 --- a/dev/test/transaction.ts +++ b/dev/test/transaction.ts @@ -339,7 +339,7 @@ function runTransaction( expect(request.type).to.equal('backoff'); if (request.delay === 'max') { // Make sure that the delay is at least 30 seconds, which is based - // on the maximum delay of 60 second and a jitter factor of 50%. + // on the maximum delay of 60 seconds and a jitter factor of 50%. expect(timeout).to.not.be.lessThan(30 * 1000); } } From 5ea2f30f444fb34681f55b0b2ddd8246abfede1c Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Fri, 6 Mar 2020 14:01:19 -0800 Subject: [PATCH 4/4] Backoff for ABORTED --- dev/src/transaction.ts | 10 ++----- dev/test/transaction.ts | 60 ++++++++++++++++++----------------------- 2 files changed, 28 insertions(+), 42 deletions(-) diff --git a/dev/src/transaction.ts b/dev/src/transaction.ts index 4965661ae..833db2a59 100644 --- a/dev/src/transaction.ts +++ b/dev/src/transaction.ts @@ -471,15 +471,9 @@ export class Transaction { * @return A Promise that resolves after the delay expired. */ private async maybeBackoff(error?: GoogleError) { - if (error) { - if (error.code === Status.RESOURCE_EXHAUSTED) { - this._backoff.resetToMax(); - } else if (error.code === Status.ABORTED) { - // We don't backoff for ABORTED to avoid starvation - this._backoff.reset(); - } + if (error && error.code === Status.RESOURCE_EXHAUSTED) { + this._backoff.resetToMax(); } - await this._backoff.backoffAndWait(); } } diff --git a/dev/test/transaction.ts b/dev/test/transaction.ts index 099113b9c..b6925f8bb 100644 --- a/dev/test/transaction.ts +++ b/dev/test/transaction.ts @@ -278,11 +278,8 @@ function runTransaction( transaction: Transaction, docRef: DocumentReference ) => Promise, - ...expectedRequests: Array + ...expectedRequests: TransactionStep[] ) { - // Filter out empty steps - expectedRequests = expectedRequests.filter(v => v !== undefined); - const overrides: ApiOverride = { beginTransaction: actual => { const request = expectedRequests.shift()!; @@ -382,41 +379,38 @@ describe('successful transactions', () => { }); describe('failed transactions', () => { - type TestCase = {retry: false} | {retry: true; backoff: boolean}; - - const errorBehavior: {[code: number]: TestCase} = { - [Status.CANCELLED]: {retry: true, backoff: true}, - [Status.UNKNOWN]: {retry: true, backoff: true}, - [Status.INVALID_ARGUMENT]: {retry: false}, - [Status.DEADLINE_EXCEEDED]: {retry: true, backoff: true}, - [Status.NOT_FOUND]: {retry: false}, - [Status.ALREADY_EXISTS]: {retry: false}, - [Status.RESOURCE_EXHAUSTED]: {retry: true, backoff: true}, - [Status.FAILED_PRECONDITION]: {retry: false}, - [Status.ABORTED]: {retry: true, backoff: false}, - [Status.OUT_OF_RANGE]: {retry: false}, - [Status.UNIMPLEMENTED]: {retry: false}, - [Status.INTERNAL]: {retry: true, backoff: true}, - [Status.UNAVAILABLE]: {retry: true, backoff: true}, - [Status.DATA_LOSS]: {retry: false}, - [Status.UNAUTHENTICATED]: {retry: true, backoff: true}, + const retryBehavior: {[code: number]: boolean} = { + [Status.CANCELLED]: true, + [Status.UNKNOWN]: true, + [Status.INVALID_ARGUMENT]: false, + [Status.DEADLINE_EXCEEDED]: true, + [Status.NOT_FOUND]: false, + [Status.ALREADY_EXISTS]: false, + [Status.RESOURCE_EXHAUSTED]: true, + [Status.FAILED_PRECONDITION]: false, + [Status.ABORTED]: true, + [Status.OUT_OF_RANGE]: false, + [Status.UNIMPLEMENTED]: false, + [Status.INTERNAL]: true, + [Status.UNAVAILABLE]: true, + [Status.DATA_LOSS]: false, + [Status.UNAUTHENTICATED]: true, }; it('retries commit based on error code', async () => { const transactionFunction = () => Promise.resolve(); - for (const errorCode of Object.keys(errorBehavior)) { - const retryBehavior = errorBehavior[Number(errorCode)]; + for (const [errorCode, retry] of Object.entries(retryBehavior)) { const serverError = new GoogleError('Test Error'); serverError.code = Number(errorCode) as Status; - if (retryBehavior.retry) { + if (retry) { await runTransaction( transactionFunction, begin('foo1'), commit('foo1', undefined, serverError), rollback('foo1'), - retryBehavior.backoff ? backoff() : undefined, + backoff(), begin('foo2', 'foo1'), commit('foo2') ); @@ -442,18 +436,17 @@ describe('failed transactions', () => { return transaction.get(query); }; - for (const errorCode of Object.keys(errorBehavior)) { - const retryBehavior = errorBehavior[Number(errorCode)]; + for (const [errorCode, retry] of Object.entries(retryBehavior)) { const serverError = new GoogleError('Test Error'); serverError.code = Number(errorCode) as Status; - if (retryBehavior.retry) { + if (retry) { await runTransaction( transactionFunction, begin('foo1'), query('foo1', serverError), rollback('foo1'), - retryBehavior.backoff ? backoff() : undefined, + backoff(), begin('foo2', 'foo1'), query('foo2'), commit('foo2') @@ -479,18 +472,17 @@ describe('failed transactions', () => { return transaction.get(docRef); }; - for (const errorCode of Object.keys(errorBehavior)) { - const retryBehavior = errorBehavior[Number(errorCode)]; + for (const [errorCode, retry] of Object.entries(retryBehavior)) { const serverError = new GoogleError('Test Error'); serverError.code = Number(errorCode) as Status; - if (retryBehavior.retry) { + if (retry) { await runTransaction( transactionFunction, begin('foo1'), getDocument('foo1', serverError), rollback('foo1'), - retryBehavior.backoff ? backoff() : undefined, + backoff(), begin('foo2', 'foo1'), getDocument('foo2'), commit('foo2')