Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 28 additions & 21 deletions src/mongo_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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/
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dunno if this matters but I get redirected when I visit the original, so figured we could skip the redirect?

*/
async connect(): Promise<this> {
if (this.connectionLock) {
Expand Down Expand Up @@ -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/
*/
Expand Down
6 changes: 4 additions & 2 deletions src/operations/execute_operation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
Expand Down Expand Up @@ -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');
}
Comment on lines +207 to +209
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that server selection may not run above, we have stale "supportsSnapshotReads" information, however, this is always been true when relying upon this value before running server selection which may trigger monitoring checks to get the latest topology info.


const hasReadAspect = operation.hasAspect(Aspect.READ_OPERATION);
const hasWriteAspect = operation.hasAspect(Aspect.WRITE_OPERATION);
const inTransaction = session?.inTransaction() ?? false;
Expand Down
16 changes: 5 additions & 11 deletions src/sdam/topology.ts
Original file line number Diff line number Diff line change
Expand Up @@ -464,20 +464,14 @@ export class Topology extends TypedEventEmitter<TopologyEvents> {
};

try {
const server = await this.selectServer(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this change necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This introduces latency while the driver attempts to discover the topology for a different read preference than the auto-connecting operation is using. Since this selectServer uses MongoClient ReadPreference or default primary and the operation can use secondary or specific tagSets we block in auto-connect for longer than needed or end up throwing MongoServerSelectionError for an operation that may have a selectable server.

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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)', () => {
Expand Down Expand Up @@ -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 () {
Expand Down
63 changes: 26 additions & 37 deletions test/integration/node-specific/abort_signal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@ import {
type Log,
type MongoClient,
MongoServerError,
MongoServerSelectionError,
promiseWithResolvers,
ReadPreference,
setDifference,
StateMachine
StateMachine,
Topology
} from '../../mongodb';
import {
clearFailPoint,
Expand Down Expand Up @@ -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');

Expand All @@ -651,7 +640,6 @@ describe('AbortSignal support', () => {
});

afterEach(async function () {
logs.length = 0;
await client?.close();
});

Expand All @@ -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
});
Expand All @@ -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;
});
});
});
Expand Down
12 changes: 9 additions & 3 deletions test/integration/node-specific/auto_connect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
type Collection,
MongoClient,
MongoNotConnectedError,
MongoOperationTimeoutError,
ProfilingLevel,
Topology,
TopologyType
Expand Down Expand Up @@ -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);
Comment on lines +869 to +873
Copy link
Contributor Author

@nbbeeken nbbeeken Oct 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is a bit weird but maybe this is just a test clean up? Auto-connect has not started respecting timeoutMS with this change, rather insertOne is now paying the cost of being the first to select a server, which we simulate with sinon, generally the first serverSelection in a MongoClient's lifecycle is going to be the slowest as discovery is run.

So this is correct for the change we're making, the bit about not double server selecting, but the outcome is that auto-connecting operations are now subject to the higher cost of discovery (which can also be true for any operation at any time, but arguably less likely).

Citing our docs:

If you need predictable operation timing with timeoutMS, call connect explicitly before performing operations.

I think this change makes sense and I can make it clear in the release notes here.

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.
});
}
Expand Down
34 changes: 28 additions & 6 deletions test/integration/node-specific/mongo_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
Db,
getTopology,
MongoClient,
MongoNetworkError,
MongoNotConnectedError,
MongoServerSelectionError,
ReadPreference,
Expand Down Expand Up @@ -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', {
Expand Down Expand Up @@ -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);
}
);
Expand Down Expand Up @@ -700,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();
}
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Comment on lines +12 to +14
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Old bulkWrite we knew we didn't have operationId support but by nature of copying out Node.js specific versions of these tests did we miss adding support to client.bulkWrite too? I have not looked into what it is or does, so maybe it doesn't fit into our API well.

].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';
});
12 changes: 0 additions & 12 deletions test/integration/server-selection/server_selection.test.ts

This file was deleted.

Loading