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

Author: nbbeeken

src/mongo_client.ts

@@ -594,20 +594,13 @@ export class MongoClient extends TypedEventEmitter<MongoClientEvents> implements
   }
 
   /**
-   * Connect to MongoDB using a url
+   * An optional method to verify a typical set of preconditions 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) {
@@ -868,21 +861,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/operations/execute_operation.ts

@@ -81,8 +81,6 @@ export async function executeOperation<
     session = client.startSession({ owner, explicit: false });
   } else if (session.hasEnded) {
     throw new MongoExpiredSessionError('Use of expired sessions is not permitted');
-  } else if (session.snapshotEnabled && !topology.capabilities.supportsSnapshotReads) {
-    throw new MongoCompatibilityError('Snapshot reads require MongoDB 5.0 or later');
   } else if (session.client !== client) {
     throw new MongoInvalidArgumentError('ClientSession must be from the same MongoClient');
   }
@@ -206,6 +204,10 @@ async function tryOperation<T extends AbstractOperation, TResult = ResultTypeFro
     signal: operation.options.signal
   });
 
+  if (session?.snapshotEnabled && !topology.capabilities.supportsSnapshotReads) {
+    throw new MongoCompatibilityError('Snapshot reads require MongoDB 5.0 or later');
+  }
+
   const hasReadAspect = operation.hasAspect(Aspect.READ_OPERATION);
   const hasWriteAspect = operation.hasAspect(Aspect.WRITE_OPERATION);
   const inTransaction = session?.inTransaction() ?? false;

src/sdam/topology.ts

@@ -464,20 +464,14 @@ export class Topology extends TypedEventEmitter<TopologyEvents> {
     };
 
     try {
-      const server = await this.selectServer(
-        readPreferenceServerSelector(readPreference),
-        selectServerOptions
-      );
-
       const skipPingOnConnect = this.s.options.__skipPingOnConnect === true;
-      if (!skipPingOnConnect && this.s.credentials) {
+      if (!skipPingOnConnect) {
+        const server = await this.selectServer(
+          readPreferenceServerSelector(readPreference),
+          selectServerOptions
+        );
         const connection = await server.pool.checkOut({ timeoutContext: timeoutContext });
         server.pool.checkIn(connection);
-        stateTransition(this, STATE_CONNECTED);
-        this.emit(Topology.OPEN, this);
-        this.emit(Topology.CONNECT, this);
-
-        return this;
       }
 
       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/abort_signal.test.ts

@@ -18,10 +18,12 @@ import {
   type Log,
   type MongoClient,
   MongoServerError,
+  MongoServerSelectionError,
   promiseWithResolvers,
   ReadPreference,
   setDifference,
-  StateMachine
+  StateMachine,
+  Topology
 } from '../../mongodb';
 import {
   clearFailPoint,
@@ -612,35 +614,22 @@ describe('AbortSignal support', () => {
     let client: MongoClient;
     let db: Db;
     let collection: Collection<{ a: number; ssn: string }>;
-    const logs: Log[] = [];
     let connectStarted;
     let controller: AbortController;
     let signal: AbortSignal;
     let cursor: AbstractCursor<{ a: number }>;
 
     describe('when connect succeeds', () => {
       beforeEach(async function () {
-        logs.length = 0;
-
         const promise = promiseWithResolvers<void>();
         connectStarted = promise.promise;
 
-        client = this.configuration.newClient(
-          {},
-          {
-            mongodbLogComponentSeverities: { serverSelection: 'debug' },
-            mongodbLogPath: {
-              write: log => {
-                if (log.c === 'serverSelection' && log.operation === 'handshake') {
-                  controller.abort();
-                  promise.resolve();
-                }
-                logs.push(log);
-              }
-            },
-            serverSelectionTimeoutMS: 1000
-          }
-        );
+        client = this.configuration.newClient({}, { serverSelectionTimeoutMS: 1000 });
+
+        client.once('open', () => {
+          controller.abort();
+          promise.resolve();
+        });
         db = client.db('abortSignal');
         collection = db.collection('support');
 
@@ -651,7 +640,6 @@ describe('AbortSignal support', () => {
       });
 
       afterEach(async function () {
-        logs.length = 0;
         await client?.close();
       });
 
@@ -667,22 +655,18 @@ describe('AbortSignal support', () => {
 
     describe('when connect fails', () => {
       beforeEach(async function () {
-        logs.length = 0;
-
         const promise = promiseWithResolvers<void>();
         connectStarted = promise.promise;
 
+        const selectServerStub = sinon
+          .stub(Topology.prototype, 'selectServer')
+          .callsFake(async function (...args) {
+            controller.abort();
+            promise.resolve();
+            return selectServerStub.wrappedMethod.call(this, ...args);
+          });
+
         client = this.configuration.newClient('mongodb://iLoveJavaScript', {
-          mongodbLogComponentSeverities: { serverSelection: 'debug' },
-          mongodbLogPath: {
-            write: log => {
-              if (log.c === 'serverSelection' && log.operation === 'handshake') {
-                controller.abort();
-                promise.resolve();
-              }
-              logs.push(log);
-            }
-          },
           serverSelectionTimeoutMS: 200,
           maxPoolSize: 1
         });
@@ -696,18 +680,19 @@ describe('AbortSignal support', () => {
       });
 
       afterEach(async function () {
-        logs.length = 0;
+        sinon.restore();
         await client?.close();
       });
 
-      it('escapes auto connect without interrupting it', async () => {
+      it('server selection error is thrown before reaching signal abort state check', async () => {
         const toArray = cursor.toArray().catch(error => error);
         await connectStarted;
-        expect(await toArray).to.be.instanceOf(DOMException);
+        const findError = await toArray;
+        expect(findError).to.be.instanceOf(MongoServerSelectionError);
+        expect(findError).to.match(/ENOTFOUND/);
         await sleep(500);
         expect(client.topology).to.exist;
         expect(client.topology.description).to.have.property('type', 'Unknown');
-        expect(findLast(logs, l => l.message.includes('Server selection failed'))).to.exist;
       });
     });
   });

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

@@ -12,6 +12,7 @@ import {
   Db,
   getTopology,
   MongoClient,
+  MongoNetworkError,
   MongoNotConnectedError,
   MongoServerSelectionError,
   ReadPreference,
@@ -333,6 +334,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', {
@@ -600,15 +623,13 @@ describe('class MongoClient', function () {
     );
 
     it(
-      'does not checkout connection when authentication is disabled',
+      'checks out connection to confirm connectivity even when authentication is disabled',
       { requires: { auth: 'disabled' } },
       async function () {
-        const checkoutStartedEvents = [];
-        client.on('connectionCheckOutStarted', event => {
-          checkoutStartedEvents.push(event);
-        });
+        const checkoutStarted = once(client, 'connectionCheckOutStarted');
         await client.connect();
-        expect(checkoutStartedEvents).to.be.empty;
+        const checkout = await checkoutStarted;
+        expect(checkout).to.exist;
         expect(client).to.have.property('topology').that.is.instanceOf(Topology);
       }
     );

test/integration/server-selection/server_selection.spec.test.ts

@@ -3,7 +3,7 @@ import * as path from 'path';
 import { loadSpecTests } from '../../spec';
 import { runUnifiedSuite } from '../../tools/unified-spec-runner/runner';
 
-describe.skip('Server Selection Unified Tests (Spec)', function () {
+describe('Server Selection Unified Tests (Spec)', function () {
   const tests = loadSpecTests(path.join('server-selection', 'logging'));
   runUnifiedSuite(tests, test => {
     if (
@@ -16,5 +16,4 @@ describe.skip('Server Selection Unified Tests (Spec)', function () {
     }
     return false;
   });
-}).skipReason =
-  'TODO: unskip these tests - NODE-2471 (ping on connect) and NODE-5774 (duplicate server selection for bulkWrite and other wrapper operations';
+});