fix(NODE-5720): on pre-4.4 sharded servers, the node driver uses `err…

…or.writeConcern.code` to determine retryability (#4155)
mongodb · Jul 30, 2024 · b26c328 · b26c328
1 parent e902584
commit b26c328
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 9 deletions.
diff --git a/src/cmap/connect.ts b/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);
         }
       }

diff --git a/src/error.ts b/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';
 
@@ -1226,7 +1227,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 +1251,17 @@ export function needsRetryableWriteLabel(error: Error, maxWireVersion: number):
   }
 
   if (error instanceof MongoWriteConcernError) {
-    return RETRYABLE_WRITE_ERROR_CODES.has(error.result.writeConcernError.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);

diff --git a/src/sdam/server.ts b/src/sdam/server.ts
@@ -453,7 +453,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);

diff --git a/test/integration/retryable-writes/retryable_writes.spec.test.ts b/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;
   });
 });