Skip to content

docs(NODE-6223): timeoutMS does not govern auto-connect #4280

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Oct 23, 2024
Merged
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
14 changes: 14 additions & 0 deletions src/mongo_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,13 @@ export class MongoClient extends TypedEventEmitter<MongoClientEvents> implements
/**
* Connect to MongoDB using a url
*
* @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.
*
* @see docs.mongodb.org/manual/reference/connection-string/
*/
async connect(): Promise<this> {
Expand Down Expand Up @@ -710,6 +717,13 @@ export class MongoClient extends TypedEventEmitter<MongoClientEvents> implements
* Connect to MongoDB using a url
*
* @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 programmatically provided options take precedence over the URI options.
*
* @see https://www.mongodb.com/docs/manual/reference/connection-string/
Expand Down
104 changes: 103 additions & 1 deletion test/integration/node-specific/auto_connect.test.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
import { expect } from 'chai';
import { once } from 'events';
import * as sinon from 'sinon';

import {
BSONType,
type ChangeStream,
ClientSession,
type Collection,
type MongoClient,
MongoClient,
MongoNotConnectedError,
ProfilingLevel,
Topology,
TopologyType
} from '../../mongodb';
import { type FailPoint, sleep } from '../../tools/utils';

describe('When executing an operation for the first time', () => {
let client: MongoClient;
Expand Down Expand Up @@ -821,4 +823,104 @@ describe('When executing an operation for the first time', () => {
});
});
});

describe('when CSOT is enabled', function () {
let client: MongoClient;

beforeEach(async function () {
client = this.configuration.newClient({ timeoutMS: 500 });
});

afterEach(async function () {
await client.close();
});

describe('when nothing is wrong', function () {
it('connects the client', async function () {
await client.connect();
expect(client).to.have.property('topology').that.is.instanceOf(Topology);
});
});

describe(
'when the server requires auth and ping is delayed',
{ requires: { auth: 'enabled', mongodb: '>=4.4' } },
function () {
beforeEach(async function () {
// set failpoint to delay ping
// create new util client to avoid affecting the test client
const utilClient = this.configuration.newClient();
await utilClient.db('admin').command({
configureFailPoint: 'failCommand',
mode: { times: 1 },
data: { failCommands: ['ping'], blockConnection: true, blockTimeMS: 1000 }
} as FailPoint);
await utilClient.close();
});

it('timeoutMS from the client is not used for the internal `ping`', async function () {
const start = performance.now();
const returnedClient = await client.connect();
const end = performance.now();
expect(returnedClient).to.equal(client);
expect(end - start).to.be.within(1000, 1500); // timeoutMS is 1000, did not apply.
});
}
);

describe(
'when server selection takes longer than the timeout',
{ requires: { auth: 'enabled', mongodb: '>=4.4' } },
function () {
beforeEach(async function () {
const selectServerStub = sinon
.stub(Topology.prototype, 'selectServer')
.callsFake(async function (selector, options) {
await sleep(1000);
const result = selectServerStub.wrappedMethod.call(this, selector, options);
sinon.restore(); // restore after connect selection
return result;
});
});

// restore sinon stub after test
afterEach(() => {
sinon.restore();
});

it('client.connect() takes as long as selectServer is delayed for and does not throw 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 end = performance.now();
expect(res).to.have.property('acknowledged', true);
expect(end - start).to.be.within(1000, 1500); // timeoutMS is 1000, did not apply.
});
}
);

describe('when auto connect is used and connect() takes longer than timeoutMS', function () {
// This test stubs the connect method to check that connect() does not get timed out
// vs. the test above makes sure that the `ping` does not inherit the client's timeoutMS setting
beforeEach(async function () {
const connectStub = sinon
.stub(MongoClient.prototype, 'connect')
.callsFake(async function () {
await sleep(1000);
const result = connectStub.wrappedMethod.call(this);
sinon.restore(); // restore after connect selection
return result;
});
});

it('the operation succeeds', 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 }); // auto-connect
const end = performance.now();
expect(res).to.have.property('acknowledged', true);
expect(end - start).to.be.within(1000, 1500); // timeoutMS is 1000, did not apply.
});
});
});
});