feat(NODE-7223): run checkout on connect regardless of credentials (#4715)

Co-authored-by: Durran Jordan <[email protected]>

Author: nbbeeken

src/mongo_client.ts

@@ -569,20 +569,13 @@ export class MongoClient extends TypedEventEmitter<MongoClientEvents> implements
   }
 
   /**
-   * Connect to MongoDB using a url
+   * An optional method to verify a handful of assumptions that are generally useful at application boot-time before using a MongoClient.
+   * For detailed information about the connect process see the MongoClient.connect static method documentation.
    *
-   * @remarks
-   * Calling `connect` is optional since the first operation you perform will call `connect` if it's needed.
-   * `timeoutMS` will bound the time any operation can take before throwing a timeout error.
-   * However, when the operation being run is automatically connecting your `MongoClient` the `timeoutMS` will not apply to the time taken to connect the MongoClient.
-   * This means the time to setup the `MongoClient` does not count against `timeoutMS`.
-   * If you are using `timeoutMS` we recommend connecting your client explicitly in advance of any operation to avoid this inconsistent execution time.
-   *
-   * @remarks
-   * The driver will look up corresponding SRV and TXT records if the connection string starts with `mongodb+srv://`.
-   * If those look ups throw a DNS Timeout error, the driver will retry the look up once.
+   * @param url - The MongoDB connection string (supports `mongodb://` and `mongodb+srv://` schemes)
+   * @param options - Optional configuration options for the client
    *
-   * @see docs.mongodb.org/manual/reference/connection-string/
+   * @see https://www.mongodb.com/docs/manual/reference/connection-string/
    */
   async connect(): Promise<this> {
     if (this.connectionLock) {
@@ -843,21 +836,35 @@ export class MongoClient extends TypedEventEmitter<MongoClientEvents> implements
   }
 
   /**
-   * Connect to MongoDB using a url
+   * Creates a new MongoClient instance and immediately connects it to MongoDB.
+   * This convenience method combines `new MongoClient(url, options)` and `client.connect()` in a single step.
+   *
+   * Connect can be helpful to detect configuration issues early by validating:
+   * - **DNS Resolution**: Verifies that SRV records and hostnames in the connection string resolve DNS entries
+   * - **Network Connectivity**: Confirms that host addresses are reachable and ports are open
+   * - **TLS Configuration**: Validates SSL/TLS certificates, CA files, and encryption settings are correct
+   * - **Authentication**: Verifies that provided credentials are valid
+   * - **Server Compatibility**: Ensures the MongoDB server version is supported by this driver version
+   * - **Load Balancer Setup**: For load-balanced deployments, confirms the service is properly configured
+   *
+   * @returns A promise that resolves to the same MongoClient instance once connected
    *
    * @remarks
-   * Calling `connect` is optional since the first operation you perform will call `connect` if it's needed.
-   * `timeoutMS` will bound the time any operation can take before throwing a timeout error.
-   * However, when the operation being run is automatically connecting your `MongoClient` the `timeoutMS` will not apply to the time taken to connect the MongoClient.
-   * This means the time to setup the `MongoClient` does not count against `timeoutMS`.
-   * If you are using `timeoutMS` we recommend connecting your client explicitly in advance of any operation to avoid this inconsistent execution time.
+   * **Connection is Optional:** Calling `connect` is optional since any operation method (`find`, `insertOne`, etc.)
+   * will automatically perform these same validation steps if the client is not already connected.
+   * However, explicitly calling `connect` can make sense for:
+   * - **Fail-fast Error Detection**: Non-transient connection issues (hostname unresolved, port refused connection) are discovered immediately rather than during your first operation
+   * - **Predictable Performance**: Eliminates first connection overhead from your first database operation
    *
    * @remarks
-   * The programmatically provided options take precedence over the URI options.
+   * **Connection Pooling Impact:** Calling `connect` will populate the connection pool with one connection
+   * to a server selected by the client's configured `readPreference` (defaults to primary).
    *
    * @remarks
-   * The driver will look up corresponding SRV and TXT records if the connection string starts with `mongodb+srv://`.
-   * If those look ups throw a DNS Timeout error, the driver will retry the look up once.
+   * **Timeout Behavior:** When using `timeoutMS`, the connection establishment time does not count against
+   * the timeout for subsequent operations. This means `connect` runs without a `timeoutMS` limit, while
+   * your database operations will still respect the configured timeout. If you need predictable operation
+   * timing with `timeoutMS`, call `connect` explicitly before performing operations.
    *
    * @see https://www.mongodb.com/docs/manual/reference/connection-string/
    */

src/sdam/topology.ts

@@ -465,7 +465,7 @@ export class Topology extends TypedEventEmitter<TopologyEvents> {
       );
 
       const skipPingOnConnect = this.s.options.__skipPingOnConnect === true;
-      if (!skipPingOnConnect && this.s.credentials) {
+      if (!skipPingOnConnect) {
         const connection = await server.pool.checkOut({ timeoutContext: timeoutContext });
         server.pool.checkIn(connection);
         stateTransition(this, STATE_CONNECTED);

test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts

@@ -2,7 +2,7 @@ import { expect } from 'chai';
 import * as dns from 'dns';
 import * as sinon from 'sinon';
 
-import { MongoAPIError, Server, ServerDescription, Topology } from '../../mongodb';
+import { ConnectionPool, MongoAPIError, Server, ServerDescription, Topology } from '../../mongodb';
 import { topologyWithPlaceholderClient } from '../../tools/utils';
 
 describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
@@ -41,6 +41,14 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
             {} as any
           );
         });
+
+        sinon.stub(ConnectionPool.prototype, 'checkOut').callsFake(async function () {
+          return {};
+        });
+
+        sinon.stub(ConnectionPool.prototype, 'checkIn').callsFake(function () {
+          return;
+        });
       });
 
       afterEach(async function () {

test/integration/node-specific/mongo_client.test.ts

@@ -10,6 +10,7 @@ import {
   type CommandSucceededEvent,
   Db,
   MongoClient,
+  MongoNetworkError,
   MongoNotConnectedError,
   MongoServerSelectionError,
   ReadPreference
@@ -315,6 +316,28 @@ describe('class MongoClient', function () {
         }
       });
     });
+
+    it('throws ENOTFOUND error when connecting to non-existent host with no auth and loadBalanced=true', async function () {
+      const configuration = this.configuration;
+      const client = configuration.newClient(
+        'mongodb://iLoveJavaScript:27017/test?loadBalanced=true',
+        { serverSelectionTimeoutMS: 100 }
+      );
+
+      const error = await client.connect().catch(error => error);
+      expect(error).to.be.instanceOf(MongoNetworkError); // not server selection like other topologies
+      expect(error.message).to.match(/ENOTFOUND/);
+    });
+
+    it('throws an error when srv is not a real record', async function () {
+      const client = this.configuration.newClient('mongodb+srv://iLoveJavaScript/test', {
+        serverSelectionTimeoutMS: 100
+      });
+
+      const error = await client.connect().catch(error => error);
+      expect(error).to.be.instanceOf(Error);
+      expect(error.message).to.match(/ENOTFOUND/);
+    });
   });
 
   it('Should correctly pass through appname', async function () {
@@ -528,31 +551,13 @@ describe('class MongoClient', function () {
       await client.close();
     });
 
-    it(
-      'creates topology and checks out connection when auth is enabled',
-      { requires: { auth: 'enabled' } },
-      async function () {
-        const checkoutStarted = once(client, 'connectionCheckOutStarted');
-        await client.connect();
-        const checkout = await checkoutStarted;
-        expect(checkout).to.exist;
-        expect(client).to.have.property('topology').that.is.instanceOf(Topology);
-      }
-    );
-
-    it(
-      'does not checkout connection when authentication is disabled',
-      { requires: { auth: 'disabled' } },
-      async function () {
-        const checkoutStartedEvents = [];
-        client.on('connectionCheckOutStarted', event => {
-          checkoutStartedEvents.push(event);
-        });
-        await client.connect();
-        expect(checkoutStartedEvents).to.be.empty;
-        expect(client).to.have.property('topology').that.is.instanceOf(Topology);
-      }
-    );
+    it('creates topology and checks out connection', async function () {
+      const checkoutStarted = once(client, 'connectionCheckOutStarted');
+      await client.connect();
+      const checkout = await checkoutStarted;
+      expect(checkout).to.exist;
+      expect(client).to.have.property('topology').that.is.instanceOf(Topology);
+    });
 
     it(
       'permits operations to be run after connect is called',

test/tools/utils.ts

@@ -463,6 +463,17 @@ export async function makeMultiResponseBatchModelArray(
   return models;
 }
 
+export function fakeServer() {
+  return {
+    s: { state: 'connected' },
+    removeListener: () => true,
+    pool: {
+      checkOut: async () => ({}),
+      checkIn: () => undefined
+    }
+  };
+}
+
 /**
  * A utility to measure the duration of an async function.  This is intended to be used for CSOT
  * testing, where we expect to timeout within a certain threshold and want to measure the duration

test/unit/assorted/server_discovery_and_monitoring.spec.test.ts

@@ -38,7 +38,7 @@ import { Server } from '../../../src/sdam/server';
 import { ServerDescription, type TopologyVersion } from '../../../src/sdam/server_description';
 import { Topology } from '../../../src/sdam/topology';
 import { isRecord, ns, squashError } from '../../../src/utils';
-import { ejson } from '../../tools/utils';
+import { ejson, fakeServer } from '../../tools/utils';
 
 const SDAM_EVENT_CLASSES = {
   ServerDescriptionChangedEvent,
@@ -214,9 +214,7 @@ describe('Server Discovery and Monitoring (spec)', function () {
           .stub(Topology.prototype, 'selectServer')
           .callsFake(async function (_selector, _options) {
             topologySelectServers.restore();
-
-            const fakeServer = { s: { state: 'connected' }, removeListener: () => true };
-            return fakeServer;
+            return fakeServer();
           });
       });
 

test/unit/assorted/server_discovery_and_monitoring.test.ts

@@ -8,6 +8,7 @@ import { Server } from '../../../src/sdam/server';
 import { ServerDescription } from '../../../src/sdam/server_description';
 import { Topology } from '../../../src/sdam/topology';
 import { type TopologyDescription } from '../../../src/sdam/topology_description';
+import { fakeServer } from '../../tools/utils';
 
 describe('Server Discovery and Monitoring', function () {
   let serverConnect: sinon.SinonStub;
@@ -30,9 +31,7 @@ describe('Server Discovery and Monitoring', function () {
       .stub(Topology.prototype, 'selectServer')
       .callsFake(async function (_selector, _options) {
         topologySelectServer.restore();
-
-        const fakeServer = { s: { state: 'connected' }, removeListener: () => true };
-        return fakeServer;
+        return fakeServer();
       });
 
     events = [];

test/unit/assorted/server_selection_spec_helper.js

@@ -9,7 +9,7 @@ const ServerSelectors = require('../../../src/sdam/server_selection');
 
 const sinon = require('sinon');
 const { expect } = require('chai');
-const { topologyWithPlaceholderClient } = require('../../tools/utils');
+const { fakeServer, topologyWithPlaceholderClient } = require('../../tools/utils');
 
 export function serverDescriptionFromDefinition(definition, hosts) {
   hosts = hosts || [];
@@ -105,9 +105,7 @@ export async function executeServerSelectionTest(testDefinition) {
     .stub(Topology.prototype, 'selectServer')
     .callsFake(async function () {
       topologySelectServers.restore();
-
-      const fakeServer = { s: { state: 'connected' }, removeListener: () => {} };
-      return fakeServer;
+      return fakeServer();
     });
 
   await topology.connect();

test/unit/sdam/topology.test.ts

@@ -25,6 +25,7 @@ import { Topology } from '../../../src/sdam/topology';
 import { TopologyDescription } from '../../../src/sdam/topology_description';
 import { TimeoutContext } from '../../../src/timeout';
 import { isHello, ns } from '../../../src/utils';
+import { ConnectionPool } from '../../mongodb';
 import * as mock from '../../tools/mongodb-mock/index';
 import { topologyWithPlaceholderClient } from '../../tools/utils';
 
@@ -445,31 +446,23 @@ describe('Topology (unit)', function () {
 
   describe('selectServer()', function () {
     it('should schedule monitoring if no suitable server is found', async function () {
-      const topology = topologyWithPlaceholderClient('someserver:27019', {});
+      const topology = topologyWithPlaceholderClient(
+        'someserver:27019',
+        {},
+        { serverSelectionTimeoutMS: 10 }
+      );
       const requestCheck = sinon.stub(Server.prototype, 'requestCheck');
 
-      // satisfy the initial connect, then restore the original method
-      const selectServer = sinon
-        .stub(Topology.prototype, 'selectServer')
-        .callsFake(async function () {
-          const server = Array.from(this.s.servers.values())[0];
-          selectServer.restore();
-          return server;
-        });
-
       sinon.stub(Server.prototype, 'connect').callsFake(function () {
         this.s.state = 'connected';
         this.emit('connect');
         return;
       });
 
-      await topology.connect();
-      const err = await topology
-        .selectServer(ReadPreference.secondary, { serverSelectionTimeoutMS: 1000 })
-        .then(
-          () => null,
-          e => e
-        );
+      const err = await topology.connect().then(
+        () => null,
+        e => e
+      );
       expect(err).to.match(/Server selection timed out/);
       expect(err).to.have.property('reason');
       // When server is created `connect` is called on the monitor. When server selection
@@ -517,12 +510,20 @@ describe('Topology (unit)', function () {
           this.emit('connect');
         });
 
+        sinon.stub(ConnectionPool.prototype, 'checkOut').callsFake(async function () {
+          return {};
+        });
+
+        sinon.stub(ConnectionPool.prototype, 'checkIn').callsFake(function (_) {
+          return;
+        });
+
         const toSelect = 10;
         let completed = 0;
         // methodology:
         //   - perform 9 server selections, a few with a selector that throws an error
         //   - ensure each selection immediately returns an empty result (gated by a boolean)
-        //     guaranteeing tha the queue will be full before the last selection
+        //     guaranteeing that the queue will be full before the last selection
         //   - make one last selection, but ensure that all selections are no longer blocked from
         //     returning their value
         //   - verify that 10 callbacks were called