Skip to content
Open
Show file tree
Hide file tree
Changes from 5 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.
Copy link
Contributor

Choose a reason for hiding this comment

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

suggest just copying the doc comment from MongoClient.connect(). Or reworking this a bit.

But the phrasing "to verify a typical set of preconditions" feels odd (and also - typical set of preconditions for who and what, exactly?).

Alternatively, since I expect users primarily use the instance connect and not the static connect, we could just move the docs from the static method here and just link the static method's documentation to this.

Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did try to link and api-extractor throws because the syntax is abiguous, you have to use tsdoc syntax which means you need to add a new flag to typedoc to get it to link properly. Probably something worth looking into adding if linking around to other docs would be valuable.

I can move the docs to the instance or the static or copy them on to both?

updated the preconditions bit

*
* @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) {
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
2 changes: 1 addition & 1 deletion src/sdam/topology.ts
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,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);
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
29 changes: 27 additions & 2 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,17 @@ describe('class MongoClient', function () {
);

it(
'does not checkout connection when authentication is disabled',
'checks out connection to confirm connectivity even when authentication is disabled',
Copy link
Contributor

Choose a reason for hiding this comment

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

can we just combine this test with the one above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what you mean by combine? because one is auth disabled and the other one is enabled, how are they combinable unless you want to move the metadata filter check into the test?

Copy link
Contributor

Choose a reason for hiding this comment

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

There is no longer a difference in behavior when auth is enabled or disabled, right? So we don't need two tests. One test that runs with both auth enabled and disabled is sufficient.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh right of course it'll run in both without the filter

{ 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
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,14 @@ describe('Server Discovery and Monitoring (spec)', function () {
.callsFake(async function (_selector, _options) {
topologySelectServers.restore();

const fakeServer = { s: { state: 'connected' }, removeListener: () => true };
const fakeServer = {
s: { state: 'connected' },
removeListener: () => true,
pool: {
checkOut: async () => ({}),
checkIn: () => undefined
}
};
return fakeServer;
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,14 @@ describe('Server Discovery and Monitoring', function () {
.callsFake(async function (_selector, _options) {
topologySelectServer.restore();

const fakeServer = { s: { state: 'connected' }, removeListener: () => true };
const fakeServer = {
s: { state: 'connected' },
removeListener: () => true,
pool: {
checkOut: async () => ({}),
checkIn: () => undefined
}
};
return fakeServer;
});

Expand Down
9 changes: 8 additions & 1 deletion test/unit/assorted/server_selection_spec_helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,14 @@ export async function executeServerSelectionTest(testDefinition) {
.callsFake(async function () {
topologySelectServers.restore();

const fakeServer = { s: { state: 'connected' }, removeListener: () => {} };
const fakeServer = {
s: { state: 'connected' },
removeListener: () => true,
pool: {
checkOut: async () => ({}),
checkIn: () => undefined
}
};
return fakeServer;
});

Expand Down
37 changes: 19 additions & 18 deletions test/unit/sdam/topology.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -444,31 +445,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
Expand Down Expand Up @@ -516,12 +509,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
Expand Down