From 15951403bd595842c872f0b0ba9f3b782b1c43ec Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Wed, 1 Dec 2021 20:23:16 +0100 Subject: [PATCH] fix(NODE-3711): retry txn end on retryable write (#3047) --- lib/core/error.js | 7 ++++++- lib/core/sessions.js | 3 ++- test/functional/transactions.test.js | 8 +------- test/unit/error.test.js | 30 ++++++++++++++++++++++++++++ 4 files changed, 39 insertions(+), 9 deletions(-) diff --git a/lib/core/error.js b/lib/core/error.js index 350b31c5a2..c89a66b2d5 100644 --- 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 index 686517144d..9609db9538 100644 --- 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 index f59264795c..b2a51558f1 100644 --- 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 index fd173d308c..2716eb961a 100644 --- 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; + }); + }); + }); });