fix(NODE-6469): pool is cleared before connection checkin on error (#…

…4296)
mongodb · Nov 19, 2024 · 06a2e2c · 06a2e2c
1 parent aa986f8
commit 06a2e2c
Show file tree

Hide file tree

Showing 14 changed files with 1,109 additions and 249 deletions.
diff --git a/src/cmap/connect.ts b/src/cmap/connect.ts
@@ -113,7 +113,8 @@ export async function performInitialHandshake(
   }
 
   const start = new Date().getTime();
-  const response = await conn.command(ns('admin.$cmd'), handshakeDoc, handshakeOptions);
+
+  const response = await executeHandshake(handshakeDoc, handshakeOptions);
 
   if (!('isWritablePrimary' in response)) {
     // Provide hello-style response document.
@@ -175,6 +176,22 @@ export async function performInitialHandshake(
   // Connection establishment is socket creation (tcp handshake, tls handshake, MongoDB handshake (saslStart, saslContinue))
   // Once connection is established, command logging can log events (if enabled)
   conn.established = true;
+
+  async function executeHandshake(handshakeDoc: Document, handshakeOptions: CommandOptions) {
+    try {
+      const handshakeResponse = await conn.command(
+        ns('admin.$cmd'),
+        handshakeDoc,
+        handshakeOptions
+      );
+      return handshakeResponse;
+    } catch (error) {
+      if (error instanceof MongoError) {
+        error.addErrorLabel(MongoErrorLabel.HandshakeError);
+      }
+      throw error;
+    }
+  }
 }
 
 /**

diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts
@@ -685,6 +685,7 @@ export class ConnectionPool extends TypedEventEmitter<ConnectionPoolEvents> {
       },
       error => {
         this[kPending]--;
+        this[kServer].handleError(error);
         this.emitAndLog(
           ConnectionPool.CONNECTION_CLOSED,
           new ConnectionClosedEvent(
@@ -719,9 +720,6 @@ export class ConnectionPool extends TypedEventEmitter<ConnectionPoolEvents> {
       // connection permits because that potentially delays the availability of
       // the connection to a checkout request
       this.createConnection((err, connection) => {
-        if (err) {
-          this[kServer].handleError(err);
-        }
         if (!err && connection) {
           this[kConnections].push(connection);
           process.nextTick(() => this.processWaitQueue());

diff --git a/...egration/connection-monitoring-and-pooling/connection_monitoring_and_pooling.spec.test.ts b/...egration/connection-monitoring-and-pooling/connection_monitoring_and_pooling.spec.test.ts
@@ -69,9 +69,6 @@ describe('Connection Monitoring and Pooling Spec Tests (Integration) - logging',
     ) {
       return 'not applicable: waitQueueSize not supported';
     }
-    if (test.description === 'Connection checkout fails due to error establishing connection') {
-      return 'TODO(NODE-5230): unskip this once event ordering issue is resolved';
-    }
     return false;
   });
 });
diff --git a/test/integration/connection-monitoring-and-pooling/connection_monitoring_and_pooling.test.ts b/test/integration/connection-monitoring-and-pooling/connection_monitoring_and_pooling.test.ts
@@ -1,6 +1,5 @@
 import { loadSpecTests } from '../../spec';
 import { type CmapTest, runCmapTestSuite } from '../../tools/cmap_spec_runner';
-import { runUnifiedSuite } from '../../tools/unified-spec-runner/runner';
 
 describe('Connection Monitoring and Pooling (Node Driver)', function () {
   const cmapTests: CmapTest[] = loadSpecTests(
@@ -17,10 +16,4 @@ describe('Connection Monitoring and Pooling (Node Driver)', function () {
       }
     ]
   });
-
-  // TODO(NODE-5230): Remove this once the actual unified tests (test/spec/connection-monitoring-and-pooling/logging) are passing
-  const unifiedTests = loadSpecTests(
-    '../integration/connection-monitoring-and-pooling/unified-cmap-node-specs'
-  );
-  runUnifiedSuite(unifiedTests);
 });
diff --git a/...gration/connection-monitoring-and-pooling/unified-cmap-node-specs/connection-logging.json b/...gration/connection-monitoring-and-pooling/unified-cmap-node-specs/connection-logging.json
diff --git a/test/spec/connection-monitoring-and-pooling/cmap-format/pool-create-min-size-error.json b/test/spec/connection-monitoring-and-pooling/cmap-format/pool-create-min-size-error.json
@@ -49,15 +49,15 @@
       "type": "ConnectionCreated",
       "address": 42
     },
+    {
+      "type": "ConnectionPoolCleared",
+      "address": 42
+    },
     {
       "type": "ConnectionClosed",
       "address": 42,
       "connectionId": 42,
       "reason": "error"
-    },
-    {
-      "type": "ConnectionPoolCleared",
-      "address": 42
     }
   ],
   "ignore": [

diff --git a/test/spec/connection-monitoring-and-pooling/cmap-format/pool-create-min-size-error.yml b/test/spec/connection-monitoring-and-pooling/cmap-format/pool-create-min-size-error.yml
@@ -30,11 +30,11 @@ events:
     address: 42
   - type: ConnectionCreated
     address: 42
+  - type: ConnectionPoolCleared
+    address: 42
   - type: ConnectionClosed
     address: 42
     connectionId: 42
     reason: error
-  - type: ConnectionPoolCleared
-    address: 42
 ignore:
   - ConnectionPoolCreated