add logic to prefer timeoutMS over wtimeoutMS

mongodb · Oct 15, 2024 · 6edcc1b · 6edcc1b
1 parent 54c72aa
commit 6edcc1b
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 35 deletions.
diff --git a/src/sessions.ts b/src/sessions.ts
@@ -482,13 +482,28 @@ export class ClientSession
       maxTimeMS?: number;
     } = { commitTransaction: 1 };
 
+    const timeoutMS =
+      typeof options?.timeoutMS === 'number'
+        ? options.timeoutMS
+        : typeof this.timeoutMS === 'number'
+          ? this.timeoutMS
+          : null;
+
     const wc = this.transaction.options.writeConcern ?? this.clientOptions?.writeConcern;
     if (wc != null) {
-      WriteConcern.apply(command, { wtimeoutMS: 10000, w: 'majority', ...wc });
+      if (timeoutMS == null && this.timeoutContext == null) {
+        WriteConcern.apply(command, { wtimeoutMS: 10000, w: 'majority', ...wc });
+      } else {
+        WriteConcern.apply(command, { w: 'majority', ...wc, wtimeoutMS: undefined });
+      }
     }
 
     if (this.transaction.state === TxnState.TRANSACTION_COMMITTED) {
-      WriteConcern.apply(command, { wtimeoutMS: 10000, ...wc, w: 'majority' });
+      if (timeoutMS == null && this.timeoutContext == null) {
+        WriteConcern.apply(command, { wtimeoutMS: 10000, ...wc, w: 'majority' });
+      } else {
+        WriteConcern.apply(command, { w: 'majority', ...wc, wtimeoutMS: undefined });
+      }
     }
 
     if (typeof this.transaction.options.maxTimeMS === 'number') {
@@ -505,13 +520,6 @@ export class ClientSession
       bypassPinningCheck: true
     });
 
-    const timeoutMS =
-      typeof options?.timeoutMS === 'number'
-        ? options.timeoutMS
-        : typeof this.timeoutMS === 'number'
-          ? this.timeoutMS
-          : null;
-
     const timeoutContext =
       this.timeoutContext ??
       (typeof timeoutMS === 'number'
@@ -601,21 +609,6 @@ export class ClientSession
       recoveryToken?: Document;
     } = { abortTransaction: 1 };
 
-    const wc = this.transaction.options.writeConcern ?? this.clientOptions?.writeConcern;
-    if (wc != null) {
-      WriteConcern.apply(command, { wtimeoutMS: 10000, w: 'majority', ...wc });
-    }
-
-    if (this.transaction.recoveryToken) {
-      command.recoveryToken = this.transaction.recoveryToken;
-    }
-
-    const operation = new RunAdminCommandOperation(command, {
-      session: this,
-      readPreference: ReadPreference.primary,
-      bypassPinningCheck: true
-    });
-
     const timeoutMS =
       typeof options?.timeoutMS === 'number'
         ? options.timeoutMS
@@ -634,6 +627,21 @@ export class ClientSession
           })
         : null;
 
+    const wc = this.transaction.options.writeConcern ?? this.clientOptions?.writeConcern;
+    if (wc != null && timeoutMS == null) {
+      WriteConcern.apply(command, { wtimeoutMS: 10000, w: 'majority', ...wc });
+    }
+
+    if (this.transaction.recoveryToken) {
+      command.recoveryToken = this.transaction.recoveryToken;
+    }
+
+    const operation = new RunAdminCommandOperation(command, {
+      session: this,
+      readPreference: ReadPreference.primary,
+      bypassPinningCheck: true
+    });
+
     try {
       await executeOperation(this.client, operation, timeoutContext);
       this.unpin();

diff --git a/src/utils.ts b/src/utils.ts
@@ -542,20 +542,36 @@ export function resolveOptions<T extends CommandOperationOptions>(
 ): T {
   const result: T = Object.assign({}, options, resolveBSONOptions(options, parent));
 
+  const timeoutMS = options?.timeoutMS ?? parent?.timeoutMS;
   // Users cannot pass a readConcern/writeConcern to operations in a transaction
   const session = options?.session;
+
   if (!session?.inTransaction()) {
     const readConcern = ReadConcern.fromOptions(options) ?? parent?.readConcern;
     if (readConcern) {
       result.readConcern = readConcern;
     }
 
-    const writeConcern = WriteConcern.fromOptions(options) ?? parent?.writeConcern;
+    let writeConcern = WriteConcern.fromOptions(options) ?? parent?.writeConcern;
     if (writeConcern) {
-      result.writeConcern = writeConcern;
+      if (timeoutMS != null) {
+        result.writeConcern = writeConcern;
+      } else {
+        const matchOptions = new Set(['wtimeout', 'wtimeoutMS']);
+        const writeConcernKeys = Object.keys(writeConcern);
+        if (writeConcernKeys.length > 2 && writeConcernKeys.some(k => !matchOptions.has(k))) {
+          writeConcern = WriteConcern.fromOptions({
+            ...writeConcern,
+            wtimeout: undefined,
+            wtimeoutMS: undefined
+          });
+        }
+      }
     }
   }
 
+  result.timeoutMS = timeoutMS;
+
   const readPreference = ReadPreference.fromOptions(options) ?? parent?.readPreference;
   if (readPreference) {
     result.readPreference = readPreference;
@@ -568,15 +584,6 @@ export function resolveOptions<T extends CommandOperationOptions>(
     );
   }
 
-  result.timeoutMS = options?.timeoutMS ?? parent?.timeoutMS;
-  if (result.timeoutMS != null && result.writeConcern) {
-    const matchOptions = new Set(['wtimeout', 'wtimeoutMS']);
-    const writeConcernKeys = Object.keys(result.writeConcern);
-    if (writeConcernKeys.length <= 2 && writeConcernKeys.every(k => matchOptions.has(k))) {
-      delete result.writeConcern;
-    }
-  }
-
   return result;
 }