fix(NODE-3711): retry txn end on retryable write (#3047)

mongodb · Dec 1, 2021 · 1595140 · 1595140
1 parent 9b980c4
commit 1595140
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 9 deletions.
diff --git a/lib/core/error.js b/lib/core/error.js
@@ -246,6 +246,10 @@ const RETRYABLE_WRITE_ERROR_CODES = new Set([
   MONGODB_ERROR_CODES.ExceededTimeLimit
 ]);
 
+function isRetryableEndTransactionError(error) {
+  return error.hasErrorLabel('RetryableWriteError');
+}
+
 function isRetryableWriteError(error) {
   if (error instanceof MongoWriteConcernError) {
     return (
@@ -347,5 +351,6 @@ module.exports = {
   isSDAMUnrecoverableError,
   isNodeShuttingDownError,
   isRetryableWriteError,
-  isNetworkErrorBeforeHandshake
+  isNetworkErrorBeforeHandshake,
+  isRetryableEndTransactionError
 };
diff --git a/lib/core/sessions.js b/lib/core/sessions.js
@@ -7,6 +7,7 @@ const Binary = BSON.Binary;
 const uuidV4 = require('./utils').uuidV4;
 const MongoError = require('./error').MongoError;
 const isRetryableError = require('././error').isRetryableError;
+const isRetryableEndTransactionError = require('././error').isRetryableEndTransactionError;
 const MongoNetworkError = require('./error').MongoNetworkError;
 const MongoWriteConcernError = require('./error').MongoWriteConcernError;
 const Transaction = require('./transactions').Transaction;
@@ -511,7 +512,7 @@ function endTransaction(session, commandName, callback) {
 
   // send the command
   session.topology.command('admin.$cmd', command, { session }, (err, reply) => {
-    if (err && isRetryableError(err)) {
+    if (err && isRetryableEndTransactionError(err)) {
       // SPEC-1185: apply majority write concern when retrying commitTransaction
       if (command.commitTransaction) {
         // per txns spec, must unpin session in this case

diff --git a/test/functional/transactions.test.js b/test/functional/transactions.test.js
@@ -131,13 +131,7 @@ describe('Transactions', function() {
 
           // Will be implemented as part of NODE-2034
           'Client side error in command starting transaction',
-          'Client side error when transaction is in progress',
-
-          // Will be implemented as part of NODE-2538
-          'abortTransaction only retries once with RetryableWriteError from server',
-          'abortTransaction does not retry without RetryableWriteError label',
-          'commitTransaction does not retry error without RetryableWriteError label',
-          'commitTransaction retries once with RetryableWriteError from server'
+          'Client side error when transaction is in progress'
         ];
 
         return SKIP_TESTS.indexOf(spec.description) === -1;

diff --git a/test/unit/error.test.js b/test/unit/error.test.js
@@ -2,6 +2,8 @@
 
 const expect = require('chai').expect;
 const MongoNetworkError = require('../../lib/core/error').MongoNetworkError;
+const isRetryableEndTransactionError = require('../../lib/core/error')
+  .isRetryableEndTransactionError;
 
 describe('MongoErrors', function() {
   describe('MongoNetworkError', function() {
@@ -19,4 +21,32 @@ describe('MongoErrors', function() {
       expect(Object.getOwnPropertySymbols(errorWithoutOption).length).to.equal(0);
     });
   });
+
+  describe('#isRetryableEndTransactionError', function() {
+    context('when the error has a RetryableWriteError label', function() {
+      const error = new MongoNetworkError('');
+      error.addErrorLabel('RetryableWriteError');
+
+      it('returns true', function() {
+        expect(isRetryableEndTransactionError(error)).to.be.true;
+      });
+    });
+
+    context('when the error does not have a RetryableWriteError label', function() {
+      const error = new MongoNetworkError('');
+      error.addErrorLabel('InvalidLabel');
+
+      it('returns false', function() {
+        expect(isRetryableEndTransactionError(error)).to.be.false;
+      });
+    });
+
+    context('when the error does not have any label', function() {
+      const error = new MongoNetworkError('');
+
+      it('returns false', function() {
+        expect(isRetryableEndTransactionError(error)).to.be.false;
+      });
+    });
+  });
 });