Merge branch 'main' into NODE-6090

Author: W-A-James

src/bulk/common.ts

@@ -617,8 +617,8 @@ function handleMongoWriteConcernError(
   callback(
     new MongoBulkWriteError(
       {
-        message: err.result?.writeConcernError.errmsg,
-        code: err.result?.writeConcernError.result
+        message: err.result.writeConcernError.errmsg,
+        code: err.result.writeConcernError.code
       },
       new BulkWriteResult(bulkResult, isOrdered)
     )

src/cmap/connect.ts

@@ -164,7 +164,7 @@ export async function performInitialHandshake(
     } catch (error) {
       if (error instanceof MongoError) {
         error.addErrorLabel(MongoErrorLabel.HandshakeError);
-        if (needsRetryableWriteLabel(error, response.maxWireVersion)) {
+        if (needsRetryableWriteLabel(error, response.maxWireVersion, conn.description.type)) {
           error.addErrorLabel(MongoErrorLabel.RetryableWriteError);
         }
       }

src/error.ts

@@ -1,4 +1,5 @@
 import type { Document } from './bson';
+import type { ServerType } from './sdam/common';
 import type { TopologyVersion } from './sdam/server_description';
 import type { TopologyDescription } from './sdam/topology_description';
 
@@ -1167,6 +1168,23 @@ export class MongoServerSelectionError extends MongoSystemError {
   }
 }
 
+/**
+ * The type of the result property of MongoWriteConcernError
+ * @public
+ */
+export interface WriteConcernErrorResult {
+  writeConcernError: {
+    code: number;
+    errmsg: string;
+    codeName?: string;
+    errInfo?: Document;
+  };
+  ok: number;
+  code?: number;
+  errorLabels?: string[];
+  [x: string | number]: unknown;
+}
+
 /**
  * An error thrown when the server reports a writeConcernError
  * @public
@@ -1187,16 +1205,8 @@ export class MongoWriteConcernError extends MongoServerError {
    *
    * @public
    **/
-  constructor(result: {
-    writeConcernError: {
-      code: number;
-      errmsg: string;
-      codeName?: string;
-      errInfo?: Document;
-    };
-    errorLabels?: string[];
-  }) {
-    super({ ...result, ...result.writeConcernError });
+  constructor(result: WriteConcernErrorResult) {
+    super({ ...result.writeConcernError, ...result });
     this.errInfo = result.writeConcernError.errInfo;
     this.result = result;
   }
@@ -1226,7 +1236,11 @@ const RETRYABLE_READ_ERROR_CODES = new Set<number>([
 // see: https://github.com/mongodb/specifications/blob/master/source/retryable-writes/retryable-writes.rst#terms
 const RETRYABLE_WRITE_ERROR_CODES = RETRYABLE_READ_ERROR_CODES;
 
-export function needsRetryableWriteLabel(error: Error, maxWireVersion: number): boolean {
+export function needsRetryableWriteLabel(
+  error: Error,
+  maxWireVersion: number,
+  serverType: ServerType
+): boolean {
   // pre-4.4 server, then the driver adds an error label for every valid case
   // execute operation will only inspect the label, code/message logic is handled here
   if (error instanceof MongoNetworkError) {
@@ -1246,11 +1260,17 @@ export function needsRetryableWriteLabel(error: Error, maxWireVersion: number):
   }
 
   if (error instanceof MongoWriteConcernError) {
-    return RETRYABLE_WRITE_ERROR_CODES.has(error.result?.code ?? error.code ?? 0);
+    if (serverType === 'Mongos' && maxWireVersion < 9) {
+      // use original top-level code from server response
+      return RETRYABLE_WRITE_ERROR_CODES.has(error.result.code ?? 0);
+    }
+    return RETRYABLE_WRITE_ERROR_CODES.has(
+      error.result.writeConcernError.code ?? Number(error.code) ?? 0
+    );
   }
 
-  if (error instanceof MongoError && typeof error.code === 'number') {
-    return RETRYABLE_WRITE_ERROR_CODES.has(error.code);
+  if (error instanceof MongoError) {
+    return RETRYABLE_WRITE_ERROR_CODES.has(Number(error.code));
   }
 
   const isNotWritablePrimaryError = LEGACY_NOT_WRITABLE_PRIMARY_ERROR_MESSAGE.test(error.message);

src/index.ts

@@ -74,7 +74,8 @@ export {
   MongoTopologyClosedError,
   MongoTransactionError,
   MongoUnexpectedServerResponseError,
-  MongoWriteConcernError
+  MongoWriteConcernError,
+  WriteConcernErrorResult
 } from './error';
 export {
   AbstractCursor,

src/sdam/server.ts

@@ -460,7 +460,7 @@ export class Server extends TypedEventEmitter<ServerEvents> {
     } else {
       if (
         (isRetryableWritesEnabled(this.topology) || isTransactionCommand(cmd)) &&
-        needsRetryableWriteLabel(error, maxWireVersion(this)) &&
+        needsRetryableWriteLabel(error, maxWireVersion(this), this.description.type) &&
         !inActiveTransaction(session, cmd)
       ) {
         error.addErrorLabel(MongoErrorLabel.RetryableWriteError);

test/integration/retryable-writes/retryable_writes.spec.test.ts

@@ -19,9 +19,6 @@ describe('Retryable Writes (unified)', function () {
   runUnifiedSuite(loadSpecTests(path.join('retryable-writes', 'unified')), ({ description }) => {
     return clientBulkWriteTests.includes(description)
       ? `TODO(NODE-6257): implement client-level bulk write.`
-      : description ===
-        'RetryableWriteError label is not added based on writeConcernError in pre-4.4 mongos response'
-      ? 'TODO(NODE-5720)'
       : false;
   });
 });

test/integration/unified-test-format/unified_test_format.spec.test.ts

@@ -41,7 +41,11 @@ const filter: TestFilter = ({ description }) => {
   return false;
 };
 
-describe('Unified test format runner', function unifiedTestRunner() {
+describe('Unified test format runner (valid-pass)', function unifiedTestRunner() {
   // Valid tests that should pass
   runUnifiedSuite(loadSpecTests('unified-test-format/valid-pass'), filter);
 });
+
+describe('Unified test format runner (valid-fail)', function unifiedTestRunner() {
+  runUnifiedSuite(loadSpecTests('unified-test-format/valid-fail'), () => false, true);
+});