fix(NODE-7229): remove duplicate server selection when auto-connecting

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) {
+        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/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,23 @@ 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);
+        if (process.platform !== 'win32') {
+          // linux / mac, unix in general will have this errno set,
+          // which is generally helpful if this is kept elevated in the error message
+          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/auto_connect.test.ts

@@ -9,6 +9,7 @@ import {
   type Collection,
   MongoClient,
   MongoNotConnectedError,
+  MongoOperationTimeoutError,
   ProfilingLevel,
   Topology,
   TopologyType
@@ -862,12 +863,17 @@ describe('When executing an operation for the first time', () => {
           sinon.restore();
         });
 
-        it('client.connect() takes as long as selectServer is delayed for and does not throw a timeout error', async function () {
+        it('client.connect() takes as long as selectServer is delayed for and throws a timeout error', async function () {
           const start = performance.now();
           expect(client.topology).to.not.exist; // make sure not connected.
-          const res = await client.db().collection('test').insertOne({ a: 1 }, { timeoutMS: 500 }); // auto-connect
+          const error = await client
+            .db()
+            .collection('test')
+            .insertOne({ a: 1 }, { timeoutMS: 500 })
+            .catch(error => error);
           const end = performance.now();
-          expect(res).to.have.property('acknowledged', true);
+          expect(error).to.be.instanceOf(MongoOperationTimeoutError);
+          expect(error).to.match(/Timed out during server selection/);
           expect(end - start).to.be.within(1000, 1500); // timeoutMS is 1000, did not apply.
         });
       }

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

@@ -626,10 +626,6 @@ describe('class MongoClient', function () {
       '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();
         const checkout = await checkoutStarted;
@@ -725,6 +721,7 @@ describe('class MongoClient', function () {
         expect(result).to.be.instanceOf(MongoServerSelectionError);
         expect(client).to.be.instanceOf(MongoClient);
         expect(client).to.have.property('topology').that.is.instanceOf(Topology);
+        await client.close();
       }
     );
   });

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

@@ -3,18 +3,19 @@ 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 (
       [
         'Failed bulkWrite operation: log messages have operationIds',
-        'Successful bulkWrite operation: log messages have operationIds'
+        'Failed client bulkWrite operation: log messages have operationIds',
+        'Successful bulkWrite operation: log messages have operationIds',
+        'Successful client bulkWrite operation: log messages have operationIds'
       ].includes(test.description)
     ) {
       return 'not applicable: operationId not supported';
     }
     return false;
   });
-}).skipReason =
-  'TODO: unskip these tests - NODE-2471 (ping on connect) and NODE-5774 (duplicate server selection for bulkWrite and other wrapper operations';
+});