fix(NODE-6613): Update error messages when primaries go stale (mongodb#4397)

W-A-James · web-flow · commit 6528c8db3acf · 2025-02-06T20:05:13.000-05:00
diff --git a/src/error.ts b/src/error.ts
@@ -1,10 +1,10 @@
-import type { Document, ObjectId } from './bson';
+import type { Document } from './bson';
 import {
   type ClientBulkWriteError,
   type ClientBulkWriteResult
 } from './operations/client_bulk_write/common';
 import type { ServerType } from './sdam/common';
-import type { ServerDescription, TopologyVersion } from './sdam/server_description';
+import type { TopologyVersion } from './sdam/server_description';
 import type { TopologyDescription } from './sdam/topology_description';
 
 /** @public */
@@ -355,16 +355,8 @@ export class MongoStalePrimaryError extends MongoRuntimeError {
    *
    * @public
    **/
-  constructor(
-    serverDescription: ServerDescription,
-    maxSetVersion: number | null,
-    maxElectionId: ObjectId | null,
-    options?: { cause?: Error }
-  ) {
-    super(
-      `primary marked stale due to electionId/setVersion mismatch: server setVersion: ${serverDescription.setVersion}, server electionId: ${serverDescription.electionId}, topology setVersion: ${maxSetVersion}, topology electionId: ${maxElectionId}`,
-      options
-    );
+  constructor(message: string, options?: { cause?: Error }) {
+    super(message, options);
   }
 
   override get name(): string {
diff --git a/src/sdam/topology_description.ts b/src/sdam/topology_description.ts
@@ -376,6 +376,19 @@ function updateRsFromPrimary(
   maxSetVersion: number | null = null,
   maxElectionId: ObjectId | null = null
 ): [TopologyType, string | null, number | null, ObjectId | null] {
+  const setVersionElectionIdMismatch = (
+    serverDescription: ServerDescription,
+    maxSetVersion: number | null,
+    maxElectionId: ObjectId | null
+  ) => {
+    return (
+      `primary marked stale due to electionId/setVersion mismatch:` +
+      ` server setVersion: ${serverDescription.setVersion},` +
+      ` server electionId: ${serverDescription.electionId},` +
+      ` topology setVersion: ${maxSetVersion},` +
+      ` topology electionId: ${maxElectionId}`
+    );
+  };
   setName = setName || serverDescription.setName;
   if (setName !== serverDescription.setName) {
     serverDescriptions.delete(serverDescription.address);
@@ -401,7 +414,9 @@ function updateRsFromPrimary(
       serverDescriptions.set(
         serverDescription.address,
         new ServerDescription(serverDescription.address, undefined, {
-          error: new MongoStalePrimaryError(serverDescription, maxSetVersion, maxElectionId)
+          error: new MongoStalePrimaryError(
+            setVersionElectionIdMismatch(serverDescription, maxSetVersion, maxElectionId)
+          )
         })
       );
 
@@ -419,7 +434,9 @@ function updateRsFromPrimary(
           serverDescriptions.set(
             serverDescription.address,
             new ServerDescription(serverDescription.address, undefined, {
-              error: new MongoStalePrimaryError(serverDescription, maxSetVersion, maxElectionId)
+              error: new MongoStalePrimaryError(
+                setVersionElectionIdMismatch(serverDescription, maxSetVersion, maxElectionId)
+              )
             })
           );
 
@@ -445,7 +462,9 @@ function updateRsFromPrimary(
       serverDescriptions.set(
         address,
         new ServerDescription(server.address, undefined, {
-          error: new MongoStalePrimaryError(serverDescription, maxSetVersion, maxElectionId)
+          error: new MongoStalePrimaryError(
+            'primary marked stale due to discovery of newer primary'
+          )
         })
       );
 
diff --git a/test/spec/server-discovery-and-monitoring/rs/new_primary.json b/test/spec/server-discovery-and-monitoring/rs/new_primary.json
@@ -58,7 +58,8 @@
         "servers": {
           "a:27017": {
             "type": "Unknown",
-            "setName": null
+            "setName": null,
+            "error": "primary marked stale due to discovery of newer primary"
           },
           "b:27017": {
             "type": "RSPrimary",
diff --git a/test/spec/server-discovery-and-monitoring/rs/new_primary.yml b/test/spec/server-discovery-and-monitoring/rs/new_primary.yml
@@ -63,7 +63,8 @@ phases: [
                 "a:27017": {
 
                     type: "Unknown",
-                    setName:
+                    setName:,
+                    error: "primary marked stale due to discovery of newer primary"
                 },
 
                 "b:27017": {
diff --git a/test/spec/server-discovery-and-monitoring/rs/new_primary_new_electionid.json b/test/spec/server-discovery-and-monitoring/rs/new_primary_new_electionid.json
@@ -77,7 +77,7 @@
             "type": "Unknown",
             "setName": null,
             "electionId": null,
-            "error": "primary marked stale due to electionId/setVersion mismatch"
+            "error": "primary marked stale due to discovery of newer primary"
           },
           "b:27017": {
             "type": "RSPrimary",
diff --git a/test/spec/server-discovery-and-monitoring/rs/new_primary_new_electionid.yml b/test/spec/server-discovery-and-monitoring/rs/new_primary_new_electionid.yml
@@ -64,7 +64,7 @@ phases: [
                     type: "Unknown",
                     setName: ,
                     electionId: ,
-                    error: "primary marked stale due to electionId/setVersion mismatch"
+                    error: "primary marked stale due to discovery of newer primary"
                 },
                 "b:27017": {
                     type: "RSPrimary",
diff --git a/test/spec/server-discovery-and-monitoring/rs/new_primary_new_setversion.json b/test/spec/server-discovery-and-monitoring/rs/new_primary_new_setversion.json
@@ -77,7 +77,7 @@
             "type": "Unknown",
             "setName": null,
             "electionId": null,
-            "error": "primary marked stale due to electionId/setVersion mismatch"
+            "error": "primary marked stale due to discovery of newer primary"
           },
           "b:27017": {
             "type": "RSPrimary",
diff --git a/test/spec/server-discovery-and-monitoring/rs/new_primary_new_setversion.yml b/test/spec/server-discovery-and-monitoring/rs/new_primary_new_setversion.yml
@@ -64,7 +64,7 @@ phases: [
                     type: "Unknown",
                     setName: ,
                     electionId:,
-                    error: "primary marked stale due to electionId/setVersion mismatch"
+                    error: "primary marked stale due to discovery of newer primary"
                 },
                 "b:27017": {
                     type: "RSPrimary",
diff --git a/test/spec/server-discovery-and-monitoring/rs/primary_disconnect_electionid.json b/test/spec/server-discovery-and-monitoring/rs/primary_disconnect_electionid.json
@@ -49,7 +49,7 @@
             "type": "Unknown",
             "setName": null,
             "electionId": null,
-            "error": "primary marked stale due to electionId/setVersion mismatch"
+            "error": "primary marked stale due to discovery of newer primary"
           },
           "b:27017": {
             "type": "RSPrimary",
@@ -125,6 +125,7 @@
           "a:27017": {
             "type": "Unknown",
             "setName": null,
+            "error": "primary marked stale due to electionId/setVersion mismatch",
             "electionId": null
           },
           "b:27017": {
diff --git a/test/spec/server-discovery-and-monitoring/rs/primary_disconnect_electionid.yml b/test/spec/server-discovery-and-monitoring/rs/primary_disconnect_electionid.yml
@@ -37,7 +37,7 @@ phases: [
                     type: "Unknown",
                     setName: ,
                     electionId:,
-                    error: "primary marked stale due to electionId/setVersion mismatch"
+                    error: "primary marked stale due to discovery of newer primary"
                 },
                 "b:27017": {
                     type: "RSPrimary",
@@ -100,6 +100,7 @@ phases: [
                 "a:27017": {
                     type: "Unknown",
                     setName: ,
+                    error: "primary marked stale due to electionId/setVersion mismatch",
                     electionId:
                 },
                 "b:27017": {
diff --git a/test/spec/server-discovery-and-monitoring/rs/primary_disconnect_setversion.json b/test/spec/server-discovery-and-monitoring/rs/primary_disconnect_setversion.json
@@ -49,7 +49,7 @@
             "type": "Unknown",
             "setName": null,
             "electionId": null,
-            "error": "primary marked stale due to electionId/setVersion mismatch"
+            "error": "primary marked stale due to discovery of newer primary"
           },
           "b:27017": {
             "type": "RSPrimary",
@@ -125,6 +125,7 @@
           "a:27017": {
             "type": "Unknown",
             "setName": null,
+            "error": "primary marked stale due to electionId/setVersion mismatch",
             "electionId": null
           },
           "b:27017": {
diff --git a/test/spec/server-discovery-and-monitoring/rs/primary_disconnect_setversion.yml b/test/spec/server-discovery-and-monitoring/rs/primary_disconnect_setversion.yml
@@ -37,7 +37,7 @@ phases: [
                     type: "Unknown",
                     setName: ,
                     electionId:,
-                    error: "primary marked stale due to electionId/setVersion mismatch"
+                    error: "primary marked stale due to discovery of newer primary"
                 },
                 "b:27017": {
                     type: "RSPrimary",
@@ -100,6 +100,7 @@ phases: [
                 "a:27017": {
                     type: "Unknown",
                     setName: ,
+                    error: "primary marked stale due to electionId/setVersion mismatch",
                     electionId:
                 },
                 "b:27017": {
diff --git a/test/spec/server-discovery-and-monitoring/rs/setversion_greaterthan_max_without_electionid.json b/test/spec/server-discovery-and-monitoring/rs/setversion_greaterthan_max_without_electionid.json
@@ -66,7 +66,7 @@
             "type": "Unknown",
             "setName": null,
             "electionId": null,
-            "error": "primary marked stale due to electionId/setVersion mismatch"
+            "error": "primary marked stale due to discovery of newer primary"
           },
           "b:27017": {
             "type": "RSPrimary",
diff --git a/test/spec/server-discovery-and-monitoring/rs/setversion_greaterthan_max_without_electionid.yml b/test/spec/server-discovery-and-monitoring/rs/setversion_greaterthan_max_without_electionid.yml
@@ -62,7 +62,7 @@ phases: [
                     type: "Unknown",
                     setName: ,
                     electionId:,
-                    error: "primary marked stale due to electionId/setVersion mismatch"
+                    error: "primary marked stale due to discovery of newer primary"
                 },
                 "b:27017": {
                     type: "RSPrimary",
diff --git a/test/spec/server-discovery-and-monitoring/rs/setversion_without_electionid-pre-6.0.json b/test/spec/server-discovery-and-monitoring/rs/setversion_without_electionid-pre-6.0.json
@@ -66,7 +66,7 @@
             "type": "Unknown",
             "setName": null,
             "electionId": null,
-            "error": "primary marked stale due to electionId/setVersion mismatch"
+            "error": "primary marked stale due to discovery of newer primary"
           },
           "b:27017": {
             "type": "RSPrimary",
diff --git a/test/spec/server-discovery-and-monitoring/rs/setversion_without_electionid-pre-6.0.yml b/test/spec/server-discovery-and-monitoring/rs/setversion_without_electionid-pre-6.0.yml
@@ -62,7 +62,7 @@ phases: [
                     type: "Unknown",
                     setName: ,
                     electionId:,
-                    error: "primary marked stale due to electionId/setVersion mismatch"
+                    error: "primary marked stale due to discovery of newer primary"
                 },
                 "b:27017": {
                     type: "RSPrimary",
diff --git a/test/spec/server-discovery-and-monitoring/rs/use_setversion_without_electionid-pre-6.0.json b/test/spec/server-discovery-and-monitoring/rs/use_setversion_without_electionid-pre-6.0.json
@@ -74,7 +74,7 @@
             "type": "Unknown",
             "setName": null,
             "electionId": null,
-            "error": "primary marked stale due to electionId/setVersion mismatch"
+            "error": "primary marked stale due to discovery of newer primary"
           },
           "b:27017": {
             "type": "RSPrimary",
diff --git a/test/spec/server-discovery-and-monitoring/rs/use_setversion_without_electionid-pre-6.0.yml b/test/spec/server-discovery-and-monitoring/rs/use_setversion_without_electionid-pre-6.0.yml
@@ -63,7 +63,7 @@ phases: [
                     type: "Unknown",
                     setName: ,
                     electionId:,
-                    error: "primary marked stale due to electionId/setVersion mismatch"
+                    error: "primary marked stale due to discovery of newer primary"
                 },
                 "b:27017": {
                     type: "RSPrimary",
diff --git a/test/unit/assorted/server_discovery_and_monitoring.test.ts b/test/unit/assorted/server_discovery_and_monitoring.test.ts
@@ -0,0 +1,123 @@
+import { expect } from 'chai';
+import { type TopologyDescription } from 'mongodb-legacy';
+import * as sinon from 'sinon';
+
+import {
+  MongoClient,
+  ObjectId,
+  Server,
+  ServerDescription,
+  Topology,
+  type TopologyDescriptionChangedEvent
+} from '../../mongodb';
+
+describe('Server Discovery and Monitoring', function () {
+  let serverConnect: sinon.SinonStub;
+  let topologySelectServer: sinon.SinonStub;
+  let client: MongoClient;
+  let events: TopologyDescriptionChangedEvent[];
+
+  function getNewDescription() {
+    const topologyDescriptionChanged = events[events.length - 1];
+    return topologyDescriptionChanged.newDescription;
+  }
+
+  beforeEach(async function () {
+    serverConnect = sinon.stub(Server.prototype, 'connect').callsFake(function () {
+      this.s.state = 'connected';
+      this.emit('connect');
+    });
+
+    topologySelectServer = sinon
+      .stub(Topology.prototype, 'selectServer')
+      .callsFake(async function (_selector, _options) {
+        topologySelectServer.restore();
+
+        const fakeServer = { s: { state: 'connected' }, removeListener: () => true };
+        return fakeServer;
+      });
+
+    events = [];
+    client = new MongoClient('mongodb://a/?replicaSet=rs');
+    client.on('topologyDescriptionChanged', event => events.push(event));
+    await client.connect();
+
+    // Start with a as primary
+    client.topology.serverUpdateHandler(
+      new ServerDescription('a:27017', {
+        ok: 1,
+        helloOk: true,
+        isWritablePrimary: true,
+        hosts: ['a:27017', 'b:27017'],
+        setName: 'rs',
+        setVersion: 1,
+        electionId: ObjectId.createFromHexString('000000000000000000000001'),
+        minWireVersion: 0,
+        maxWireVersion: 21
+      })
+    );
+
+    // b is elected as primary, a gets marked stale
+    client.topology.serverUpdateHandler(
+      new ServerDescription('b:27017', {
+        ok: 1,
+        helloOk: true,
+        isWritablePrimary: true,
+        hosts: ['a:27017', 'b:27017'],
+        setName: 'rs',
+        setVersion: 2,
+        electionId: ObjectId.createFromHexString('000000000000000000000001'),
+        minWireVersion: 0,
+        maxWireVersion: 21
+      })
+    );
+  });
+
+  afterEach(async function () {
+    serverConnect.restore();
+    await client.close().catch(() => null);
+  });
+
+  let newDescription: TopologyDescription;
+
+  describe('when a newer primary is detected', function () {
+    it('steps down original primary to unknown server description with appropriate error message', function () {
+      newDescription = getNewDescription();
+
+      const aOutcome = newDescription.servers.get('a:27017');
+      const bOutcome = newDescription.servers.get('b:27017');
+      expect(aOutcome.type).to.equal('Unknown');
+      expect(aOutcome.error).to.match(/primary marked stale due to discovery of newer primary/);
+
+      expect(bOutcome.type).to.equal('RSPrimary');
+    });
+  });
+
+  describe('when a stale primary still reports itself as primary', function () {
+    it('gets marked as unknown with an error message with the new and old replicaSetVersion and electionId', function () {
+      // a still incorrectly reports as primary
+      client.topology.serverUpdateHandler(
+        new ServerDescription('a:27017', {
+          ok: 1,
+          helloOk: true,
+          isWritablePrimary: true,
+          hosts: ['a:27017', 'b:27017'],
+          setName: 'rs',
+          setVersion: 1,
+          electionId: ObjectId.createFromHexString('000000000000000000000001'),
+          minWireVersion: 0,
+          maxWireVersion: 21
+        })
+      );
+
+      newDescription = getNewDescription();
+
+      const aOutcome = newDescription.servers.get('a:27017');
+
+      expect(aOutcome.type).to.equal('Unknown');
+      expect(aOutcome.error).to.match(
+        /primary marked stale due to electionId\/setVersion mismatch: server setVersion: \d+, server electionId: \d{24}, topology setVersion: \d+, topology electionId: \d{24}/
+      );
+    });
+  });
+});
