Using Transaction Result from last retry (#111)

googleapis · Jan 22, 2018 · b3219b6 · b3219b6
1 parent 7cdc1c5
commit b3219b6
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 24 deletions.
diff --git a/src/index.js b/src/index.js
@@ -338,8 +338,8 @@ class Firestore extends commonGrpc.Service {
    * 'Proto3 JSON' and 'Protobuf JS' encoded data.
    *
    * @private
-   * @param {object} documentOrName - The Firestore 'Document' proto or the
-   * resource name of a missing document.
+   * @param {object|string} documentOrName - The Firestore 'Document' proto or
+   * the resource name of a missing document.
    * @param {object=} readTime - A 'Timestamp' proto indicating the time this
    * document was read.
    * @param {string=} encoding - One of 'json' or 'protobufJS'. Applies to both
@@ -487,27 +487,27 @@ class Firestore extends commonGrpc.Service {
         });
       })
       .then(() => {
-        return transaction.commit().catch(err => {
-          if (attemptsRemaining > 0) {
+        return transaction
+          .commit()
+          .then(() => result)
+          .catch(err => {
+            if (attemptsRemaining > 0) {
+              Firestore.log(
+                'Firestore.runTransaction',
+                `Retrying transaction after error: ${JSON.stringify(err)}.`
+              );
+              return this.runTransaction(updateFunction, {
+                previousTransaction: transaction,
+                maxAttempts: attemptsRemaining,
+              });
+            }
             Firestore.log(
               'Firestore.runTransaction',
-              `Retrying transaction after error: ${JSON.stringify(err)}.`
+              'Exhausted transaction retries, returning error: %s',
+              err
             );
-            return this.runTransaction(updateFunction, {
-              previousTransaction: transaction,
-              maxAttempts: attemptsRemaining,
-            });
-          }
-          Firestore.log(
-            'Firestore.runTransaction',
-            'Exhausted transaction retries, returning error: %s',
-            err
-          );
-          return Promise.reject(err);
-        });
-      })
-      .then(() => {
-        return result;
+            return Promise.reject(err);
+          });
       });
   }
 

diff --git a/test/transaction.js b/test/transaction.js
@@ -341,16 +341,17 @@ describe('failed transactions', function() {
   });
 
   it('retries on commit failure', function() {
-    let err = new Error('Retryable error');
+    let userResult = ['failure', 'failure', 'success'];
+    let serverError = new Error('Retryable error');
 
     return runTransaction(
       () => {
-        return Promise.resolve('success');
+        return Promise.resolve(userResult.shift());
       },
       begin('foo1'),
-      commit('foo1', [], err),
+      commit('foo1', [], serverError),
       begin('foo2', 'foo1'),
-      commit('foo2', [], err),
+      commit('foo2', [], serverError),
       begin('foo3', 'foo2'),
       commit('foo3')
     ).then(red => {