Skip to content
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

Fix transaction.set() failure without retry on "already-exists" error #6729

Merged

Conversation

milaGGL
Copy link
Contributor

@milaGGL milaGGL commented Oct 26, 2022

Problem

Transaction.get(ref) followed by transaction.set(ref, {...}) is causing the transaction to fail with error 'Document already exists' if the document was created by another app between the get and set.

Cause

'already-exists' is considered to be a permanent error for transactions, therefore will not be retried.

private isRetryableTransactionError(error: Error): boolean {

Fix Issue: #6659

@changeset-bot
Copy link

changeset-bot bot commented Oct 26, 2022

🦋 Changeset detected

Latest commit: d93d965

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@firebase/firestore Patch
firebase Patch
@firebase/firestore-compat Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@milaGGL milaGGL changed the title add 'already-exists' to isRetryableTransactionError Fix transaction.set() failure on document created by others without retry Oct 26, 2022
@milaGGL milaGGL changed the title Fix transaction.set() failure on document created by others without retry Fix transaction.set() failure without retry on document created by others Oct 26, 2022
@google-oss-bot
Copy link
Contributor

google-oss-bot commented Oct 26, 2022

Size Report 1

Affected Products

  • @firebase/database

    TypeBase (3f1354f)Merge (c571eb1)Diff
    browser248 kB248 kB+1 B (+0.0%)
    esm5276 kB276 kB+1 B (+0.0%)
    main281 kB281 kB+1 B (+0.0%)
    module248 kB248 kB+1 B (+0.0%)
  • @firebase/database-compat

    TypeBase (3f1354f)Merge (c571eb1)Diff
    browser18.0 kB18.0 kB+1 B (+0.0%)
    esm521.1 kB21.1 kB+1 B (+0.0%)
    main21.8 kB21.8 kB+1 B (+0.0%)
    module18.0 kB18.0 kB+1 B (+0.0%)
  • @firebase/firestore

    TypeBase (3f1354f)Merge (c571eb1)Diff
    browser266 kB266 kB+106 B (+0.0%)
    esm5330 kB330 kB+106 B (+0.0%)
    main531 kB531 kB+36 B (+0.0%)
    module266 kB266 kB+106 B (+0.0%)
    react-native266 kB266 kB+106 B (+0.0%)
  • @firebase/firestore-lite

    TypeBase (3f1354f)Merge (c571eb1)Diff
    browser82.3 kB82.4 kB+160 B (+0.2%)
    esm598.5 kB98.6 kB+179 B (+0.2%)
    main139 kB139 kB+328 B (+0.2%)
    module82.3 kB82.4 kB+160 B (+0.2%)
    react-native82.4 kB82.6 kB+160 B (+0.2%)
  • bundle

    21 size changes

    TypeBase (3f1354f)Merge (c571eb1)Diff
    database (Append to a list of data)148 kB148 kB+1 B (+0.0%)
    database (Filtering data)147 kB147 kB+1 B (+0.0%)
    database (Listen for child events)163 kB163 kB+1 B (+0.0%)
    database (Listen for value events + Detach listeners)163 kB163 kB+1 B (+0.0%)
    database (Listen for value events)163 kB163 kB+1 B (+0.0%)
    database (Read data once)162 kB162 kB+1 B (+0.0%)
    database (Save data as transactions)165 kB165 kB+1 B (+0.0%)
    database (Sort data)148 kB148 kB+1 B (+0.0%)
    database (Write data)147 kB147 kB+1 B (+0.0%)
    firestore (Persistence)276 kB276 kB+73 B (+0.0%)
    firestore (Query Cursors)213 kB213 kB+84 B (+0.0%)
    firestore (Query)214 kB214 kB+84 B (+0.0%)
    firestore (Read data once)202 kB202 kB+84 B (+0.0%)
    firestore (Realtime updates)205 kB205 kB+84 B (+0.0%)
    firestore (Transaction)186 kB186 kB+95 B (+0.1%)
    firestore (Write data)186 kB186 kB+73 B (+0.0%)
    firestore-lite (Query Cursors)71.4 kB71.5 kB+138 B (+0.2%)
    firestore-lite (Query)74.6 kB74.7 kB+138 B (+0.2%)
    firestore-lite (Read data once)59.0 kB59.1 kB+138 B (+0.2%)
    firestore-lite (Transaction)83.6 kB83.7 kB+160 B (+0.2%)
    firestore-lite (Write data)68.7 kB68.9 kB+138 B (+0.2%)

  • firebase

    TypeBase (3f1354f)Merge (c571eb1)Diff
    firebase-compat.js740 kB740 kB+116 B (+0.0%)
    firebase-database-compat.js166 kB166 kB+7 B (+0.0%)
    firebase-database.js154 kB154 kB+2 B (+0.0%)
    firebase-firestore-compat.js313 kB314 kB+114 B (+0.0%)
    firebase-firestore-lite.js89.0 kB89.1 kB+160 B (+0.2%)
    firebase-firestore.js314 kB314 kB+106 B (+0.0%)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/IUh5GNCrFQ.html

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Oct 26, 2022

Size Analysis Report 1

Affected Products

  • @firebase/database

    • DataSnapshot

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size117 kB117 kB+1 B (+0.0%)
      size-with-ext-deps138 kB138 kB+1 B (+0.0%)
    • Database

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size115 kB115 kB+1 B (+0.0%)
      size-with-ext-deps137 kB137 kB+1 B (+0.0%)
    • OnDisconnect

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size119 kB119 kB+1 B (+0.0%)
      size-with-ext-deps141 kB141 kB+1 B (+0.0%)
    • QueryConstraint

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size115 kB115 kB+1 B (+0.0%)
      size-with-ext-deps137 kB137 kB+1 B (+0.0%)
    • TransactionResult

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size116 kB116 kB+1 B (+0.0%)
      size-with-ext-deps137 kB137 kB+1 B (+0.0%)
    • _QueryImpl

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size115 kB115 kB+1 B (+0.0%)
      size-with-ext-deps137 kB137 kB+1 B (+0.0%)
    • _QueryParams

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size115 kB115 kB+1 B (+0.0%)
      size-with-ext-deps137 kB137 kB+1 B (+0.0%)
    • _ReferenceImpl

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size115 kB115 kB+1 B (+0.0%)
      size-with-ext-deps137 kB137 kB+1 B (+0.0%)
    • _TEST_ACCESS_forceRestClient

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size115 kB115 kB+1 B (+0.0%)
      size-with-ext-deps137 kB137 kB+1 B (+0.0%)
    • _TEST_ACCESS_hijackHash

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size116 kB116 kB+1 B (+0.0%)
      size-with-ext-deps137 kB137 kB+1 B (+0.0%)
    • _repoManagerDatabaseFromApp

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size115 kB115 kB+1 B (+0.0%)
      size-with-ext-deps137 kB137 kB+1 B (+0.0%)
    • _setSDKVersion

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size115 kB115 kB+1 B (+0.0%)
      size-with-ext-deps137 kB137 kB+1 B (+0.0%)
    • _validatePathString

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size116 kB116 kB+1 B (+0.0%)
      size-with-ext-deps137 kB137 kB+1 B (+0.0%)
    • _validateWritablePath

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size116 kB116 kB+1 B (+0.0%)
      size-with-ext-deps137 kB137 kB+1 B (+0.0%)
    • child

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size116 kB116 kB+1 B (+0.0%)
      size-with-ext-deps137 kB137 kB+1 B (+0.0%)
    • connectDatabaseEmulator

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size116 kB116 kB+1 B (+0.0%)
      size-with-ext-deps138 kB138 kB+1 B (+0.0%)
    • enableLogging

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size115 kB115 kB+1 B (+0.0%)
      size-with-ext-deps137 kB137 kB+1 B (+0.0%)
    • endAt

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size118 kB118 kB+1 B (+0.0%)
      size-with-ext-deps139 kB139 kB+1 B (+0.0%)
    • endBefore

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size119 kB119 kB+1 B (+0.0%)
      size-with-ext-deps140 kB140 kB+1 B (+0.0%)
    • equalTo

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size119 kB119 kB+1 B (+0.0%)
      size-with-ext-deps140 kB140 kB+1 B (+0.0%)
    • forceLongPolling

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size116 kB116 kB+1 B (+0.0%)
      size-with-ext-deps137 kB137 kB+1 B (+0.0%)
    • forceWebSockets

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size116 kB116 kB+1 B (+0.0%)
      size-with-ext-deps137 kB137 kB+1 B (+0.0%)
    • get

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size132 kB132 kB+1 B (+0.0%)
      size-with-ext-deps153 kB153 kB+1 B (+0.0%)
    • getDatabase

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size116 kB116 kB+1 B (+0.0%)
      size-with-ext-deps145 kB145 kB+1 B (+0.0%)
    • goOffline

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size115 kB115 kB+1 B (+0.0%)
      size-with-ext-deps137 kB137 kB+1 B (+0.0%)
    • goOnline

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size116 kB116 kB+1 B (+0.0%)
      size-with-ext-deps137 kB137 kB+1 B (+0.0%)
    • increment

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size115 kB115 kB+1 B (+0.0%)
      size-with-ext-deps137 kB137 kB+1 B (+0.0%)
    • limitToFirst

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size116 kB116 kB+1 B (+0.0%)
      size-with-ext-deps137 kB137 kB+1 B (+0.0%)
    • limitToLast

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size116 kB116 kB+1 B (+0.0%)
      size-with-ext-deps137 kB137 kB+1 B (+0.0%)
    • off

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size124 kB124 kB+1 B (+0.0%)
      size-with-ext-deps145 kB145 kB+1 B (+0.0%)
    • onChildAdded

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size132 kB132 kB+1 B (+0.0%)
      size-with-ext-deps154 kB154 kB+1 B (+0.0%)
    • onChildChanged

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size132 kB132 kB+1 B (+0.0%)
      size-with-ext-deps154 kB154 kB+1 B (+0.0%)
    • onChildMoved

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size132 kB132 kB+1 B (+0.0%)
      size-with-ext-deps154 kB154 kB+1 B (+0.0%)
    • onChildRemoved

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size132 kB132 kB+1 B (+0.0%)
      size-with-ext-deps154 kB154 kB+1 B (+0.0%)
    • onDisconnect

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size119 kB119 kB+1 B (+0.0%)
      size-with-ext-deps141 kB141 kB+1 B (+0.0%)
    • onValue

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size132 kB132 kB+1 B (+0.0%)
      size-with-ext-deps154 kB154 kB+1 B (+0.0%)
    • orderByChild

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size118 kB118 kB+1 B (+0.0%)
      size-with-ext-deps139 kB139 kB+1 B (+0.0%)
    • orderByKey

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size117 kB117 kB+1 B (+0.0%)
      size-with-ext-deps138 kB138 kB+1 B (+0.0%)
    • orderByPriority

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size117 kB117 kB+1 B (+0.0%)
      size-with-ext-deps138 kB138 kB+1 B (+0.0%)
    • orderByValue

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size117 kB117 kB+1 B (+0.0%)
      size-with-ext-deps138 kB138 kB+1 B (+0.0%)
    • push

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size118 kB118 kB+1 B (+0.0%)
      size-with-ext-deps139 kB139 kB+1 B (+0.0%)
    • query

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size115 kB115 kB+1 B (+0.0%)
      size-with-ext-deps137 kB137 kB+1 B (+0.0%)
    • ref

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size116 kB116 kB+1 B (+0.0%)
      size-with-ext-deps137 kB137 kB+1 B (+0.0%)
    • refFromURL

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size116 kB116 kB+1 B (+0.0%)
      size-with-ext-deps138 kB138 kB+1 B (+0.0%)
    • remove

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size116 kB116 kB+1 B (+0.0%)
      size-with-ext-deps138 kB138 kB+1 B (+0.0%)
    • runTransaction

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size134 kB134 kB+1 B (+0.0%)
      size-with-ext-deps156 kB156 kB+1 B (+0.0%)
    • serverTimestamp

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size115 kB115 kB+1 B (+0.0%)
      size-with-ext-deps137 kB137 kB+1 B (+0.0%)
    • set

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size116 kB116 kB+1 B (+0.0%)
      size-with-ext-deps138 kB138 kB+1 B (+0.0%)
    • setPriority

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size117 kB117 kB+1 B (+0.0%)
      size-with-ext-deps138 kB138 kB+1 B (+0.0%)
    • setWithPriority

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size117 kB117 kB+1 B (+0.0%)
      size-with-ext-deps138 kB138 kB+1 B (+0.0%)
    • startAfter

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size119 kB119 kB+1 B (+0.0%)
      size-with-ext-deps140 kB140 kB+1 B (+0.0%)
    • startAt

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size118 kB118 kB+1 B (+0.0%)
      size-with-ext-deps139 kB139 kB+1 B (+0.0%)
    • update

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size118 kB118 kB+1 B (+0.0%)
      size-with-ext-deps139 kB139 kB+1 B (+0.0%)
  • @firebase/firestore

    • addDoc

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size116 kB116 kB+73 B (+0.1%)
      size-with-ext-deps176 kB176 kB+73 B (+0.0%)
    • deleteDoc

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size107 kB107 kB+73 B (+0.1%)
      size-with-ext-deps167 kB167 kB+73 B (+0.0%)
    • disableNetwork

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size95.7 kB95.8 kB+73 B (+0.1%)
      size-with-ext-deps155 kB155 kB+73 B (+0.0%)
    • enableIndexedDbPersistence

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size174 kB174 kB+73 B (+0.0%)
      size-with-ext-deps234 kB234 kB+73 B (+0.0%)
    • enableMultiTabIndexedDbPersistence

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size207 kB207 kB+73 B (+0.0%)
      size-with-ext-deps267 kB267 kB+73 B (+0.0%)
    • enableNetwork

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size95.7 kB95.8 kB+73 B (+0.1%)
      size-with-ext-deps155 kB155 kB+73 B (+0.0%)
    • executeWrite

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size106 kB106 kB+73 B (+0.1%)
      size-with-ext-deps166 kB166 kB+73 B (+0.0%)
    • getCountFromServer

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size102 kB102 kB+73 B (+0.1%)
      size-with-ext-deps162 kB162 kB+73 B (+0.0%)
    • getDoc

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size133 kB133 kB+84 B (+0.1%)
      size-with-ext-deps192 kB192 kB+84 B (+0.0%)
    • getDocFromServer

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size133 kB133 kB+84 B (+0.1%)
      size-with-ext-deps192 kB192 kB+84 B (+0.0%)
    • getDocs

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size134 kB134 kB+84 B (+0.1%)
      size-with-ext-deps194 kB194 kB+84 B (+0.0%)
    • getDocsFromServer

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size134 kB134 kB+84 B (+0.1%)
      size-with-ext-deps194 kB194 kB+84 B (+0.0%)
    • loadBundle

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size105 kB105 kB+73 B (+0.1%)
      size-with-ext-deps164 kB164 kB+73 B (+0.0%)
    • onSnapshot

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size135 kB135 kB+84 B (+0.1%)
      size-with-ext-deps194 kB194 kB+84 B (+0.0%)
    • onSnapshotsInSync

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size124 kB125 kB+84 B (+0.1%)
      size-with-ext-deps184 kB184 kB+84 B (+0.0%)
    • runTransaction

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size116 kB116 kB+95 B (+0.1%)
      size-with-ext-deps176 kB176 kB+95 B (+0.1%)
    • setDoc

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size115 kB115 kB+73 B (+0.1%)
      size-with-ext-deps175 kB175 kB+73 B (+0.0%)
    • updateDoc

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size116 kB116 kB+73 B (+0.1%)
      size-with-ext-deps175 kB175 kB+73 B (+0.0%)
    • waitForPendingWrites

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size96.1 kB96.2 kB+73 B (+0.1%)
      size-with-ext-deps156 kB156 kB+73 B (+0.0%)
    • writeBatch

      Size

      TypeBase (3f1354f)Merge (c571eb1)Diff
      size118 kB118 kB+73 B (+0.1%)
      size-with-ext-deps177 kB177 kB+73 B (+0.0%)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/CrIVKBtWXR.html

@milaGGL milaGGL changed the title Fix transaction.set() failure without retry on document created by others Fix transaction.set() failure without retry on "already-exists" error Oct 27, 2022
@milaGGL milaGGL marked this pull request as ready for review October 27, 2022 19:09
@milaGGL milaGGL requested a review from egilmorez as a code owner October 27, 2022 19:09
@milaGGL milaGGL self-assigned this Oct 27, 2022
Copy link
Contributor

@dconeybe dconeybe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Just two very minor comments!


if (retryCounter === 1) {
expect(snap1.exists()).to.be.false;
// On the first attemp, create a doc before transaction.set(), so that
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: "attemp" -> "attempt"

transaction.set(docRef, { count: 2 });
});
expect(retryCounter).to.equal(2);
const result = await getDoc(docRef);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Please rename result to snapshot.

Copy link
Contributor

@tom-andersen tom-andersen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

@milaGGL milaGGL merged commit e2a90bf into master Oct 28, 2022
@milaGGL milaGGL deleted the mila/change-ALREADY-EXISTS-to-retryable-error-for-transactions branch October 28, 2022 15:41
@google-oss-bot google-oss-bot mentioned this pull request Nov 8, 2022
@firebase firebase locked and limited conversation to collaborators Nov 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants