fix(NODE-7430): throw timeout error when withTransaction retries exceed deadline

src/sessions.ts

@@ -17,6 +17,7 @@ import {
   MongoErrorLabel,
   MongoExpiredSessionError,
   MongoInvalidArgumentError,
+  MongoOperationTimeoutError,
   MongoRuntimeError,
   MongoServerError,
   MongoTransactionError,
@@ -777,14 +778,15 @@ export class ClientSession
           const willExceedTransactionDeadline =
             (this.timeoutContext?.csotEnabled() &&
               backoffMS > this.timeoutContext.remainingTimeMS) ||
-            processTimeMS() + backoffMS > startTime + MAX_TIMEOUT;
+            (!this.timeoutContext?.csotEnabled() &&
+              processTimeMS() + backoffMS > startTime + MAX_TIMEOUT);
 
           if (willExceedTransactionDeadline) {
-            throw (
+            throw makeWithTransactionTimeoutError(
               lastError ??
-              new MongoRuntimeError(
-                `Transaction retry did not record an error: should never occur. Please file a bug.`
-              )
+                new MongoRuntimeError(
+                  `Transaction retry did not record an error: should never occur. Please file a bug.`
+                )
             );
           }
 
@@ -827,6 +829,8 @@ export class ClientSession
             throw fnError;
           }
 
+          lastError = fnError;
+
           if (
             this.transaction.state === TxnState.STARTING_TRANSACTION ||
             this.transaction.state === TxnState.TRANSACTION_IN_PROGRESS
@@ -836,14 +840,15 @@ export class ClientSession
             await this.abortTransaction();
           }
 
-          if (
-            fnError.hasErrorLabel(MongoErrorLabel.TransientTransactionError) &&
-            (this.timeoutContext?.csotEnabled() || processTimeMS() - startTime < MAX_TIMEOUT)
-          ) {
-            // 7.ii If the callback's error includes a "TransientTransactionError" label and the elapsed time of `withTransaction`
-            // is less than 120 seconds, jump back to step two.
-            lastError = fnError;
-            continue retryTransaction;
+          if (fnError.hasErrorLabel(MongoErrorLabel.TransientTransactionError)) {
+            if (this.timeoutContext?.csotEnabled() || processTimeMS() - startTime < MAX_TIMEOUT) {
+              // 7.ii If the callback's error includes a "TransientTransactionError" label and the elapsed time of `withTransaction`
+              // is less than TIMEOUT_MS, jump back to step two.
+              continue retryTransaction;
+            } else {
+              // 7.ii (cont.) If timeout has been exceeded, raise a timeout error wrapping the transient error.
+              throw makeWithTransactionTimeoutError(fnError);
+            }
           }
 
           // 7.iii If the callback's error includes a "UnknownTransactionCommitResult" label, the callback must have manually committed a transaction,
@@ -865,37 +870,39 @@ export class ClientSession
             committed = true;
             // 10. If commitTransaction reported an error:
           } catch (commitError) {
-            // If CSOT is enabled, we repeatedly retry until timeoutMS expires.  This is enforced by providing a
-            // timeoutContext to each async API, which know how to cancel themselves (i.e., the next retry will
-            // abort the withTransaction call).
-            // If CSOT is not enabled, do we still have time remaining or have we timed out?
+            lastError = commitError;
+
+            // Check if the withTransaction timeout has been exceeded.
+            // With CSOT: check remaining time from the timeout context.
+            // Without CSOT: check if we've exceeded the 120-second timeout.
             const hasTimedOut =
-              !this.timeoutContext?.csotEnabled() && processTimeMS() - startTime >= MAX_TIMEOUT;
-
-            if (!hasTimedOut) {
-              /*
-               * Note: a maxTimeMS error will have the MaxTimeMSExpired
-               * code (50) and can be reported as a top-level error or
-               * inside writeConcernError, ex.
-               * { ok:0, code: 50, codeName: 'MaxTimeMSExpired' }
-               * { ok:1, writeConcernError: { code: 50, codeName: 'MaxTimeMSExpired' } }
-               */
-              if (
-                !isMaxTimeMSExpiredError(commitError) &&
-                commitError.hasErrorLabel(MongoErrorLabel.UnknownTransactionCommitResult)
-              ) {
-                // 10.i If the `commitTransaction` error includes a "UnknownTransactionCommitResult" label and the error is not
-                // MaxTimeMSExpired and the elapsed time of `withTransaction` is less than 120 seconds, jump back to step eight.
-                continue retryCommit;
-              }
-
-              if (commitError.hasErrorLabel(MongoErrorLabel.TransientTransactionError)) {
-                // 10.ii If the commitTransaction error includes a "TransientTransactionError" label
-                // and the elapsed time of withTransaction is less than 120 seconds, jump back to step two.
-                lastError = commitError;
-
-                continue retryTransaction;
-              }
+              (this.timeoutContext?.csotEnabled() && this.timeoutContext.remainingTimeMS <= 0) ||
+              (!this.timeoutContext?.csotEnabled() && processTimeMS() - startTime >= MAX_TIMEOUT);
+
+            if (hasTimedOut) {
+              throw makeWithTransactionTimeoutError(commitError);
+            }
+
+            /*
+             * Note: a maxTimeMS error will have the MaxTimeMSExpired
+             * code (50) and can be reported as a top-level error or
+             * inside writeConcernError, ex.
+             * { ok:0, code: 50, codeName: 'MaxTimeMSExpired' }
+             * { ok:1, writeConcernError: { code: 50, codeName: 'MaxTimeMSExpired' } }
+             */
+            if (
+              !isMaxTimeMSExpiredError(commitError) &&
+              commitError.hasErrorLabel(MongoErrorLabel.UnknownTransactionCommitResult)
+            ) {
+              // 10.i If the `commitTransaction` error includes a "UnknownTransactionCommitResult" label and the error is not
+              // MaxTimeMSExpired and the elapsed time of `withTransaction` is less than TIMEOUT_MS, jump back to step eight.
+              continue retryCommit;
+            }
+
+            if (commitError.hasErrorLabel(MongoErrorLabel.TransientTransactionError)) {
+              // 10.ii If the commitTransaction error includes a "TransientTransactionError" label
+              // and the elapsed time of withTransaction is less than TIMEOUT_MS, jump back to step two.
+              continue retryTransaction;
             }
 
             // 10.iii Otherwise, propagate the commitTransaction error to the caller of withTransaction and return immediately.
@@ -912,6 +919,18 @@ export class ClientSession
   }
 }
 
+function makeWithTransactionTimeoutError(cause: Error): MongoOperationTimeoutError {
+  const timeoutError = new MongoOperationTimeoutError('Timed out during withTransaction', {
+    cause
+  });
+  if (cause instanceof MongoError) {
+    for (const label of cause.errorLabels) {
+      timeoutError.addErrorLabel(label);
+    }
+  }
+  return timeoutError;
+}
+
 const NON_DETERMINISTIC_WRITE_CONCERN_ERRORS = new Set([
   'CannotSatisfyWriteConcern',
   'UnknownReplWriteConcern',

test/integration/transactions-convenient-api/transactions-convenient-api.prose.test.ts

@@ -2,8 +2,18 @@ import { expect } from 'chai';
 import { test } from 'mocha';
 import * as sinon from 'sinon';
 
-import { type ClientSession, type Collection, type MongoClient } from '../../mongodb';
-import { configureFailPoint, type FailCommandFailPoint, measureDuration } from '../../tools/utils';
+import {
+  type ClientSession,
+  type Collection,
+  type MongoClient,
+  MongoOperationTimeoutError
+} from '../../mongodb';
+import {
+  clearFailPoint,
+  configureFailPoint,
+  type FailCommandFailPoint,
+  measureDuration
+} from '../../tools/utils';
 
 const failCommand: FailCommandFailPoint = {
   configureFailPoint: 'failCommand',
@@ -85,3 +95,161 @@ describe('Retry Backoff is Enforced', function () {
     }
   );
 });
+
+describe('Retry Timeout is Enforced', function () {
+  // Drivers should test that withTransaction enforces a non-configurable timeout before retrying
+  // both commits and entire transactions.
+  //
+  // We stub performance.now() to simulate elapsed time exceeding the 120-second retry limit,
+  // as recommended by the spec: "This might be done by internally modifying the timeout value
+  // used by withTransaction with some private API or using a mock timer."
+  //
+  // The error SHOULD be propagated as a timeout error if the language allows to expose the
+  // underlying error as a cause of a timeout error.
+
+  let client: MongoClient;
+  let collection: Collection;
+  let timeOffset: number;
+
+  beforeEach(async function () {
+    client = this.configuration.newClient();
+    collection = client.db('foo').collection('bar');
+
+    timeOffset = 0;
+    const originalNow = performance.now.bind(performance);
+    sinon.stub(performance, 'now').callsFake(() => originalNow() + timeOffset);
+  });
+
+  afterEach(async function () {
+    sinon.restore();
+    await clearFailPoint(this.configuration);
+    await client?.close();
+  });
+
+  // Case 1: If the callback raises an error with the TransientTransactionError label and the retry
+  // timeout has been exceeded, withTransaction should propagate the error to its caller.
+  test(
+    'callback TransientTransactionError propagated as timeout error when retry timeout exceeded',
+    {
+      requires: {
+        mongodb: '>=4.4',
+        topology: '!single'
+      }
+    },
+    async function () {
+      // 1. Configure a failpoint that fails insert with TransientTransactionError.
+      await configureFailPoint(this.configuration, {
+        configureFailPoint: 'failCommand',
+        mode: { times: 1 },
+        data: {
+          failCommands: ['insert'],
+          errorCode: 24,
+          errorLabels: ['TransientTransactionError']
+        }
+      });
+
+      // 2. Run withTransaction. The callback advances the clock past the 120-second retry
+      //    limit before the insert fails, so the timeout is detected immediately.
+      const { result } = await measureDuration(() => {
+        return client.withSession(async s => {
+          await s.withTransaction(async session => {
+            timeOffset = 120_000;
+            await collection.insertOne({}, { session });
+          });
+        });
+      });
+
+      // 3. Assert that the error is a timeout error wrapping the TransientTransactionError.
+      expect(result).to.be.instanceOf(MongoOperationTimeoutError);
+      expect((result as MongoOperationTimeoutError).cause).to.be.an('error');
+      expect((result as MongoOperationTimeoutError).hasErrorLabel('TransientTransactionError')).to
+        .be.true;
+    }
+  );
+
+  // Case 2: If committing raises an error with the UnknownTransactionCommitResult label, and the
+  // retry timeout has been exceeded, withTransaction should propagate the error to
+  // its caller.
+  test(
+    'commit UnknownTransactionCommitResult propagated as timeout error when retry timeout exceeded',
+    {
+      requires: {
+        mongodb: '>=4.4',
+        topology: '!single'
+      }
+    },
+    async function () {
+      // 1. Configure a failpoint that fails commitTransaction with UnknownTransactionCommitResult.
+      await configureFailPoint(this.configuration, {
+        configureFailPoint: 'failCommand',
+        mode: { times: 1 },
+        data: {
+          failCommands: ['commitTransaction'],
+          errorCode: 64,
+          errorLabels: ['UnknownTransactionCommitResult']
+        }
+      });
+
+      // 2. Run withTransaction. The callback advances the clock past the 120-second retry
+      //    limit. The insert succeeds, but the commit fails and the timeout is detected.
+      const { result } = await measureDuration(() => {
+        return client.withSession(async s => {
+          await s.withTransaction(async session => {
+            timeOffset = 120_000;
+            await collection.insertOne({}, { session });
+          });
+        });
+      });
+
+      // 3. Assert that the error is a timeout error wrapping the commit error.
+      expect(result).to.be.instanceOf(MongoOperationTimeoutError);
+      expect((result as MongoOperationTimeoutError).cause).to.be.an('error');
+      expect((result as MongoOperationTimeoutError).hasErrorLabel('UnknownTransactionCommitResult'))
+        .to.be.true;
+    }
+  );
+
+  // Case 3: If committing raises an error with the TransientTransactionError label and the retry
+  // timeout has been exceeded, withTransaction should propagate the error to its
+  // caller. This case may occur if the commit was internally retried against a new primary after a
+  // failover and the second primary returned a NoSuchTransaction error response.
+  test(
+    'commit TransientTransactionError propagated as timeout error when retry timeout exceeded',
+    {
+      requires: {
+        mongodb: '>=4.4',
+        topology: '!single'
+      }
+    },
+    async function () {
+      // 1. Configure a failpoint that fails commitTransaction with TransientTransactionError
+      //    (errorCode 251 = NoSuchTransaction).
+      await configureFailPoint(this.configuration, {
+        configureFailPoint: 'failCommand',
+        mode: { times: 1 },
+        data: {
+          failCommands: ['commitTransaction'],
+          errorCode: 251,
+          errorLabels: ['TransientTransactionError']
+        }
+      });
+
+      // 2. Run withTransaction. The callback advances the clock past the 120-second retry
+      //    limit. The insert succeeds, but the commit fails and the timeout is detected.
+      const { result } = await measureDuration(() => {
+        return client.withSession(async s => {
+          await s.withTransaction(async session => {
+            timeOffset = 120_000;
+            await collection.insertOne({}, { session });
+          });
+        });
+      });
+
+      // 3. Assert that the error is a timeout error wrapping the TransientTransactionError.
+      expect(result).to.be.instanceOf(MongoOperationTimeoutError);
+      expect((result as MongoOperationTimeoutError).cause).to.be.an('error');
+      expect((result as MongoOperationTimeoutError).hasErrorLabel('TransientTransactionError')).to
+        .be.true;
+    }
+  );
+});