From ed9fedff4028247a0761e06c7a220b4ae8c269b3 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Wed, 26 Oct 2022 14:25:17 -0700 Subject: [PATCH 1/4] add 'already-exists' to isRetryableTransactionError --- packages/firestore/src/core/transaction_runner.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/firestore/src/core/transaction_runner.ts b/packages/firestore/src/core/transaction_runner.ts index 30ef5ea0b32..d9e679321b5 100644 --- a/packages/firestore/src/core/transaction_runner.ts +++ b/packages/firestore/src/core/transaction_runner.ts @@ -120,6 +120,7 @@ export class TransactionRunner { return ( code === 'aborted' || code === 'failed-precondition' || + code === 'already-exists' || !isPermanentError(code) ); } From f86f7de090ae7f61b795d0dbbe9b3765590e0286 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Thu, 27 Oct 2022 11:55:22 -0700 Subject: [PATCH 2/4] Update transactions.test.ts --- .../test/integration/api/transactions.test.ts | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/packages/firestore/test/integration/api/transactions.test.ts b/packages/firestore/test/integration/api/transactions.test.ts index 3c26f085042..2d7f56596f1 100644 --- a/packages/firestore/test/integration/api/transactions.test.ts +++ b/packages/firestore/test/integration/api/transactions.test.ts @@ -629,6 +629,30 @@ apiDescribe('Database transactions', (persistence: boolean) => { }); }); + it('retries when document already exists', () => { + return withTestDb(persistence, async db => { + let retryCounter = 0; + const docRef = doc(collection(db, 'nonexistent')); + + await runTransaction(db, async transaction => { + ++retryCounter; + const snap1 = await transaction.get(docRef); + + if (retryCounter === 1) { + expect(snap1.exists()).to.be.false; + // On the first attemp, create a doc before transaction.set(), so that + // the transaction fails with "already-exists" error, and retries. + await setDoc(docRef, { count: 1 }); + } + + transaction.set(docRef, { count: 2 }); + }); + expect(retryCounter).to.equal(2); + const result = await getDoc(docRef); + expect(result.get('count')).to.equal(2); + }); + }); + it('are successful with no transaction operations', () => { return withTestDb(persistence, db => runTransaction(db, async () => {})); }); From d9e30a24ef8e6084c6a35cc11a9a1178810b55e1 Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Thu, 27 Oct 2022 12:08:07 -0700 Subject: [PATCH 3/4] add changelog --- .changeset/young-knives-shake.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/young-knives-shake.md diff --git a/.changeset/young-knives-shake.md b/.changeset/young-knives-shake.md new file mode 100644 index 00000000000..3b3813ae018 --- /dev/null +++ b/.changeset/young-knives-shake.md @@ -0,0 +1,6 @@ +--- +'@firebase/firestore': patch +'firebase': patch +--- + +Fix transaction.set() failure without retry on "already-exists" error. From d93d965d426fb5f6dc29c44cf6502cbb006c80ba Mon Sep 17 00:00:00 2001 From: milaGGL <107142260+milaGGL@users.noreply.github.com> Date: Thu, 27 Oct 2022 12:36:34 -0700 Subject: [PATCH 4/4] resolve comments --- .../test/integration/api/transactions.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/firestore/test/integration/api/transactions.test.ts b/packages/firestore/test/integration/api/transactions.test.ts index 2d7f56596f1..6126e9f9cb8 100644 --- a/packages/firestore/test/integration/api/transactions.test.ts +++ b/packages/firestore/test/integration/api/transactions.test.ts @@ -636,11 +636,11 @@ apiDescribe('Database transactions', (persistence: boolean) => { await runTransaction(db, async transaction => { ++retryCounter; - const snap1 = await transaction.get(docRef); + const snap = await transaction.get(docRef); if (retryCounter === 1) { - expect(snap1.exists()).to.be.false; - // On the first attemp, create a doc before transaction.set(), so that + expect(snap.exists()).to.be.false; + // On the first attempt, create a doc before transaction.set(), so that // the transaction fails with "already-exists" error, and retries. await setDoc(docRef, { count: 1 }); } @@ -648,8 +648,8 @@ apiDescribe('Database transactions', (persistence: boolean) => { transaction.set(docRef, { count: 2 }); }); expect(retryCounter).to.equal(2); - const result = await getDoc(docRef); - expect(result.get('count')).to.equal(2); + const snap = await getDoc(docRef); + expect(snap.get('count')).to.equal(2); }); });