diff --git a/src/collection.ts b/src/collection.ts index 8fcdf90bff..3665498995 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -106,6 +106,8 @@ export interface CollectionOptions extends BSONSerializeOptions, WriteConcernOpt readConcern?: ReadConcernLike; /** The preferred read preference (ReadPreference.PRIMARY, ReadPreference.PRIMARY_PREFERRED, ReadPreference.SECONDARY, ReadPreference.SECONDARY_PREFERRED, ReadPreference.NEAREST). */ readPreference?: ReadPreferenceLike; + /** @internal TODO(NODE-5688): make this public */ + timeoutMS?: number; } /** @internal */ diff --git a/src/cursor/abstract_cursor.ts b/src/cursor/abstract_cursor.ts index 51cf4ce700..10aa5eea5f 100644 --- a/src/cursor/abstract_cursor.ts +++ b/src/cursor/abstract_cursor.ts @@ -109,6 +109,8 @@ export interface AbstractCursorOptions extends BSONSerializeOptions { */ awaitData?: boolean; noCursorTimeout?: boolean; + /** @internal TODO(NODE-5688): make this public */ + timeoutMS?: number; } /** @internal */ @@ -184,6 +186,7 @@ export abstract class AbstractCursor< : ReadPreference.primary, ...pluckBSONSerializeOptions(options) }; + this[kOptions].timeoutMS = options.timeoutMS; const readConcern = ReadConcern.fromOptions(options); if (readConcern) { diff --git a/src/db.ts b/src/db.ts index 1dda4834c2..423979a0d4 100644 --- a/src/db.ts +++ b/src/db.ts @@ -66,7 +66,8 @@ const DB_OPTIONS_ALLOW_LIST = [ 'enableUtf8Validation', 'promoteValues', 'compression', - 'retryWrites' + 'retryWrites', + 'timeoutMS' ]; /** @internal */ @@ -94,6 +95,8 @@ export interface DbOptions extends BSONSerializeOptions, WriteConcernOptions { readConcern?: ReadConcern; /** Should retry failed writes */ retryWrites?: boolean; + /** @internal TODO(NODE-5688): make this public */ + timeoutMS?: number; } /** diff --git a/src/gridfs/download.ts b/src/gridfs/download.ts index 08659dfabf..6b0fc57fc6 100644 --- a/src/gridfs/download.ts +++ b/src/gridfs/download.ts @@ -28,6 +28,8 @@ export interface GridFSBucketReadStreamOptions { * to be returned by the stream. `end` is non-inclusive */ end?: number; + /** @internal TODO(NODE-5688): make this public */ + timeoutMS?: number; } /** @public */ diff --git a/src/gridfs/index.ts b/src/gridfs/index.ts index c7611c852b..51c32b7a01 100644 --- a/src/gridfs/index.ts +++ b/src/gridfs/index.ts @@ -36,6 +36,8 @@ export interface GridFSBucketOptions extends WriteConcernOptions { chunkSizeBytes?: number; /** Read preference to be passed to read operations */ readPreference?: ReadPreference; + /** @internal TODO(NODE-5688): make this public */ + timeoutMS?: number; } /** @internal */ diff --git a/src/gridfs/upload.ts b/src/gridfs/upload.ts index 80024d7d32..12440f2670 100644 --- a/src/gridfs/upload.ts +++ b/src/gridfs/upload.ts @@ -36,6 +36,8 @@ export interface GridFSBucketWriteStreamOptions extends WriteConcernOptions { * @deprecated Will be removed in the next major version. Add an aliases field to the metadata document instead. */ aliases?: string[]; + /** @internal TODO(NODE-5688): make this public */ + timeoutMS?: number; } /** diff --git a/src/mongo_client.ts b/src/mongo_client.ts index 98a9f151ac..1e21aefe35 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -120,7 +120,7 @@ export type SupportedNodeConnectionOptions = SupportedTLSConnectionOptions & export interface MongoClientOptions extends BSONSerializeOptions, SupportedNodeConnectionOptions { /** Specifies the name of the replica set, if the mongod is a member of a replica set. */ replicaSet?: string; - /** @internal This option is in development and currently has no behaviour. */ + /** @internal TODO(NODE-5688): This option is in development and currently has no behaviour. */ timeoutMS?: number; /** Enables or disables TLS/SSL for the connection. */ tls?: boolean; @@ -897,4 +897,6 @@ export interface MongoOptions * TODO: NODE-5671 - remove internal flag */ mongodbLogPath?: 'stderr' | 'stdout' | MongoDBLogWritable; + /** @internal TODO(NODE-5688): make this public */ + timeoutMS?: number; } diff --git a/src/operations/execute_operation.ts b/src/operations/execute_operation.ts index 59f58f8748..b2951170ad 100644 --- a/src/operations/execute_operation.ts +++ b/src/operations/execute_operation.ts @@ -110,6 +110,11 @@ export async function executeOperation< } else if (session.client !== client) { throw new MongoInvalidArgumentError('ClientSession must be from the same MongoClient'); } + if (session.explicit && session?.timeoutMS != null && operation.options.timeoutMS != null) { + throw new MongoInvalidArgumentError( + 'Do not specify timeoutMS on operation if already specified on an explicit session' + ); + } const readPreference = operation.readPreference ?? ReadPreference.primary; const inTransaction = !!session?.inTransaction(); diff --git a/src/operations/operation.ts b/src/operations/operation.ts index 7f564cbc27..e71baa44a9 100644 --- a/src/operations/operation.ts +++ b/src/operations/operation.ts @@ -34,6 +34,9 @@ export interface OperationOptions extends BSONSerializeOptions { /** @internal Hints to `executeOperation` that this operation should not unpin on an ended transaction */ bypassPinningCheck?: boolean; omitReadPreference?: boolean; + + /** @internal TODO(NODE-5688): make this public */ + timeoutMS?: number; } /** @internal */ diff --git a/src/sessions.ts b/src/sessions.ts index 4822f0483a..7ef4c3ba22 100644 --- a/src/sessions.ts +++ b/src/sessions.ts @@ -61,6 +61,9 @@ export interface ClientSessionOptions { snapshot?: boolean; /** The default TransactionOptions to use for transactions started on this session. */ defaultTransactionOptions?: TransactionOptions; + /** @internal + * The value of timeoutMS used for CSOT. Used to override client timeoutMS */ + defaultTimeoutMS?: number; /** @internal */ owner?: symbol | AbstractCursor; @@ -131,6 +134,8 @@ export class ClientSession extends TypedEventEmitter { [kPinnedConnection]?: Connection; /** @internal */ [kTxnNumberIncrement]: number; + /** @internal */ + timeoutMS?: number; /** * Create a client session. @@ -173,6 +178,7 @@ export class ClientSession extends TypedEventEmitter { this.sessionPool = sessionPool; this.hasEnded = false; this.clientOptions = clientOptions; + this.timeoutMS = options.defaultTimeoutMS ?? client.options?.timeoutMS; this.explicit = !!options.explicit; this[kServerSession] = this.explicit ? this.sessionPool.acquire() : null; diff --git a/test/integration/client-side-operations-timeout/node_csot.test.ts b/test/integration/client-side-operations-timeout/node_csot.test.ts index 06e8179daf..b6a936afbb 100644 --- a/test/integration/client-side-operations-timeout/node_csot.test.ts +++ b/test/integration/client-side-operations-timeout/node_csot.test.ts @@ -1,4 +1,97 @@ -/* eslint-disable @typescript-eslint/no-empty-function */ /* Anything javascript specific relating to timeouts */ +import { expect } from 'chai'; +import * as sinon from 'sinon'; -describe.skip('CSOT driver tests', () => {}); +import { + type ClientSession, + type Collection, + type Db, + type FindCursor, + type MongoClient +} from '../../mongodb'; + +describe('CSOT driver tests', () => { + afterEach(() => { + sinon.restore(); + }); + + describe('timeoutMS inheritance', () => { + let client: MongoClient; + let db: Db; + let coll: Collection; + + beforeEach(async function () { + client = this.configuration.newClient(undefined, { timeoutMS: 100 }); + db = client.db('test', { timeoutMS: 200 }); + }); + + afterEach(async () => { + await client?.close(); + }); + + describe('when timeoutMS is provided on an operation', () => { + beforeEach(() => { + coll = db.collection('test', { timeoutMS: 300 }); + }); + + describe('when in a session', () => { + let cursor: FindCursor; + let session: ClientSession; + + beforeEach(() => { + session = client.startSession({ defaultTimeoutMS: 400 }); + cursor = coll.find({}, { session, timeoutMS: 500 }); + }); + + afterEach(async () => { + await cursor?.close(); + await session?.endSession(); + await session.endSession(); + }); + + it('throws an error', async () => { + expect(cursor.cursorOptions).to.have.property('timeoutMS', 500); + }); + }); + + describe('when not in a session', () => { + let cursor: FindCursor; + + beforeEach(() => { + db = client.db('test', { timeoutMS: 200 }); + coll = db.collection('test', { timeoutMS: 300 }); + cursor = coll.find({}, { timeoutMS: 400 }); + }); + + afterEach(async () => { + await cursor?.close(); + }); + + it('overrides the value provided on the db', async () => { + expect(cursor.cursorOptions).to.have.property('timeoutMS', 400); + }); + }); + }); + + describe('when timeoutMS is provided on a collection', () => { + beforeEach(() => { + db = client.db('test', { timeoutMS: 200 }); + coll = db.collection('test', { timeoutMS: 300 }); + }); + + it('overrides the value provided on the db', () => { + expect(coll.s.options).to.have.property('timeoutMS', 300); + }); + + describe('when timeoutMS is provided on a db', () => { + beforeEach(() => { + db = client.db('test', { timeoutMS: 200 }); + }); + + it('overrides the value provided on the client', () => { + expect(db.s.options).to.have.property('timeoutMS', 200); + }); + }); + }); + }); +}); diff --git a/test/spec/uri-options/connection-options.json b/test/spec/uri-options/connection-options.json index 8bb05cc721..b2669b6cf1 100644 --- a/test/spec/uri-options/connection-options.json +++ b/test/spec/uri-options/connection-options.json @@ -2,7 +2,7 @@ "tests": [ { "description": "Valid connection and timeout options are parsed correctly", - "uri": "mongodb://example.com/?appname=URI-OPTIONS-SPEC-TEST&connectTimeoutMS=20000&heartbeatFrequencyMS=5000&localThresholdMS=3000&maxIdleTimeMS=50000&replicaSet=uri-options-spec&retryWrites=true&serverSelectionTimeoutMS=15000&socketTimeoutMS=7500", + "uri": "mongodb://example.com/?appname=URI-OPTIONS-SPEC-TEST&connectTimeoutMS=20000&heartbeatFrequencyMS=5000&localThresholdMS=3000&maxIdleTimeMS=50000&replicaSet=uri-options-spec&retryWrites=true&serverSelectionTimeoutMS=15000&socketTimeoutMS=7500&timeoutMS=100", "valid": true, "warning": false, "hosts": null, @@ -16,7 +16,8 @@ "replicaSet": "uri-options-spec", "retryWrites": true, "serverSelectionTimeoutMS": 15000, - "socketTimeoutMS": 7500 + "socketTimeoutMS": 7500, + "timeoutMS": 100 } }, { @@ -238,6 +239,35 @@ "hosts": null, "auth": null, "options": {} + }, + { + "description": "timeoutMS=0", + "uri": "mongodb://example.com/?timeoutMS=0", + "valid": true, + "warning": false, + "hosts": null, + "auth": null, + "options": { + "timeoutMS": 0 + } + }, + { + "description": "Non-numeric timeoutMS causes a warning", + "uri": "mongodb://example.com/?timeoutMS=invalid", + "valid": true, + "warning": true, + "hosts": null, + "auth": null, + "options": {} + }, + { + "description": "Too low timeoutMS causes a warning", + "uri": "mongodb://example.com/?timeoutMS=-2", + "valid": true, + "warning": true, + "hosts": null, + "auth": null, + "options": {} } ] } diff --git a/test/spec/uri-options/connection-options.yml b/test/spec/uri-options/connection-options.yml index 83f63daa1a..43ef9ec9b3 100644 --- a/test/spec/uri-options/connection-options.yml +++ b/test/spec/uri-options/connection-options.yml @@ -1,7 +1,7 @@ tests: - description: "Valid connection and timeout options are parsed correctly" - uri: "mongodb://example.com/?appname=URI-OPTIONS-SPEC-TEST&connectTimeoutMS=20000&heartbeatFrequencyMS=5000&localThresholdMS=3000&maxIdleTimeMS=50000&replicaSet=uri-options-spec&retryWrites=true&serverSelectionTimeoutMS=15000&socketTimeoutMS=7500" + uri: "mongodb://example.com/?appname=URI-OPTIONS-SPEC-TEST&connectTimeoutMS=20000&heartbeatFrequencyMS=5000&localThresholdMS=3000&maxIdleTimeMS=50000&replicaSet=uri-options-spec&retryWrites=true&serverSelectionTimeoutMS=15000&socketTimeoutMS=7500&timeoutMS=100" valid: true warning: false hosts: ~ @@ -16,6 +16,7 @@ tests: retryWrites: true serverSelectionTimeoutMS: 15000 socketTimeoutMS: 7500 + timeoutMS: 100 - description: "Non-numeric connectTimeoutMS causes a warning" uri: "mongodb://example.com/?connectTimeoutMS=invalid" @@ -64,7 +65,7 @@ tests: hosts: ~ auth: ~ options: {} - - + - description: "Invalid retryWrites causes a warning" uri: "mongodb://example.com/?retryWrites=invalid" valid: true @@ -207,3 +208,28 @@ tests: hosts: ~ auth: ~ options: {} + - + description: "timeoutMS=0" + uri: "mongodb://example.com/?timeoutMS=0" + valid: true + warning: false + hosts: ~ + auth: ~ + options: + timeoutMS: 0 + - + description: "Non-numeric timeoutMS causes a warning" + uri: "mongodb://example.com/?timeoutMS=invalid" + valid: true + warning: true + hosts: ~ + auth: ~ + options: {} + - + description: "Too low timeoutMS causes a warning" + uri: "mongodb://example.com/?timeoutMS=-2" + valid: true + warning: true + hosts: ~ + auth: ~ + options: {} diff --git a/test/spec/uri-options/connection-pool-options.yml b/test/spec/uri-options/connection-pool-options.yml index 65fbb7367c..0333d4e31e 100644 --- a/test/spec/uri-options/connection-pool-options.yml +++ b/test/spec/uri-options/connection-pool-options.yml @@ -65,4 +65,3 @@ tests: hosts: ~ auth: ~ options: {} - \ No newline at end of file diff --git a/test/spec/uri-options/read-preference-options.json b/test/spec/uri-options/read-preference-options.json index df8c0c0eb8..cdac6a63c3 100644 --- a/test/spec/uri-options/read-preference-options.json +++ b/test/spec/uri-options/read-preference-options.json @@ -23,7 +23,7 @@ }, { "description": "Single readPreferenceTags is parsed as array of size one", - "uri": "mongodb://example.com/?readPreferenceTags=dc:ny", + "uri": "mongodb://example.com/?readPreference=secondary&readPreferenceTags=dc:ny", "valid": true, "warning": false, "hosts": null, diff --git a/test/spec/uri-options/read-preference-options.yml b/test/spec/uri-options/read-preference-options.yml index cac8c6d89c..4ced669acf 100644 --- a/test/spec/uri-options/read-preference-options.yml +++ b/test/spec/uri-options/read-preference-options.yml @@ -17,7 +17,7 @@ tests: maxStalenessSeconds: 120 - description: "Single readPreferenceTags is parsed as array of size one" - uri: "mongodb://example.com/?readPreferenceTags=dc:ny" + uri: "mongodb://example.com/?readPreference=secondary&readPreferenceTags=dc:ny" valid: true warning: false hosts: ~ diff --git a/test/tools/uri_spec_runner.ts b/test/tools/uri_spec_runner.ts index 6ad6e86381..844e5bd470 100644 --- a/test/tools/uri_spec_runner.ts +++ b/test/tools/uri_spec_runner.ts @@ -349,6 +349,7 @@ export function executeUriValidationTest( case 'maxConnecting': case 'maxPoolSize': case 'minPoolSize': + case 'timeoutMS': case 'connectTimeoutMS': case 'heartbeatFrequencyMS': case 'localThresholdMS': diff --git a/test/unit/mongo_client.test.js b/test/unit/mongo_client.test.ts similarity index 97% rename from test/unit/mongo_client.test.js rename to test/unit/mongo_client.test.ts index e3b3beb92b..79663a2603 100644 --- a/test/unit/mongo_client.test.js +++ b/test/unit/mongo_client.test.ts @@ -1,28 +1,28 @@ -('use strict'); - +import { expect } from 'chai'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as sinon from 'sinon'; +import { Writable } from 'stream'; import { inspect } from 'util'; -const os = require('os'); -const fs = require('fs'); -const { expect } = require('chai'); -const { getSymbolFrom } = require('../tools/utils'); -const { parseOptions, resolveSRVRecord } = require('../mongodb'); -const { ReadConcern } = require('../mongodb'); -const { WriteConcern } = require('../mongodb'); -const { ReadPreference } = require('../mongodb'); -const { MongoCredentials } = require('../mongodb'); -const { +import { + MongoAPIError, MongoClient, + type MongoClientOptions, + MongoCredentials, + MongoInvalidArgumentError, + MongoLoggableComponent, + MongoLogger, MongoParseError, + parseOptions, + ReadConcern, + ReadPreference, + resolveSRVRecord, ServerApiVersion, - MongoAPIError, - MongoInvalidArgumentError -} = require('../mongodb'); -const { MongoLogger } = require('../mongodb'); -// eslint-disable-next-line no-restricted-modules -const { SeverityLevel, MongoLoggableComponent } = require('../../src/mongo_logger'); -const sinon = require('sinon'); -const { Writable } = require('stream'); + SeverityLevel, + WriteConcern +} from '../mongodb'; +import { getSymbolFrom } from '../tools/utils'; describe('MongoClient', function () { it('MongoClient should always freeze public options', function () { @@ -144,7 +144,10 @@ describe('MongoClient', function () { }; it('should parse all options from the options object', function () { - const options = parseOptions('mongodb://localhost:27017/', ALL_OPTIONS); + const options = parseOptions( + 'mongodb://localhost:27017/', + ALL_OPTIONS as unknown as MongoClientOptions + ); // Check consolidated options expect(options).has.property('writeConcern'); expect(options.writeConcern).has.property('w', 2); @@ -1205,4 +1208,28 @@ describe('MongoClient', function () { expect.fail('missed exception'); }); }); + + context('timeoutMS', function () { + context('when timeoutMS is negative', function () { + it('throws MongoParseError', function () { + expect(() => new MongoClient('mongodb://host', { timeoutMS: -1 })).to.throw( + MongoParseError + ); + }); + }); + + context('when timeoutMS is positive', function () { + it('sets the value on the MongoClient', function () { + const client = new MongoClient('mongodb://host', { timeoutMS: 1 }); + expect(client.options.timeoutMS).to.equal(1); + }); + }); + + context('when timeoutMS is zero', function () { + it('sets the value on the MongoClient', function () { + const client = new MongoClient('mongodb://host', { timeoutMS: 0 }); + expect(client.options.timeoutMS).to.equal(0); + }); + }); + }); }); diff --git a/test/unit/sessions.test.js b/test/unit/sessions.test.ts similarity index 90% rename from test/unit/sessions.test.js rename to test/unit/sessions.test.ts index ba58fef41e..f889ed9c6a 100644 --- a/test/unit/sessions.test.js +++ b/test/unit/sessions.test.ts @@ -1,21 +1,21 @@ -'use strict'; +import { expect } from 'chai'; +import * as sinon from 'sinon'; -const mock = require('../tools/mongodb-mock/index'); -const { expect } = require('chai'); -const { genClusterTime } = require('../tools/common'); -const { - ServerSessionPool, - ServerSession, - ClientSession, +import { applySession, - BSON -} = require('../mongodb'); -const { now, isHello } = require('../mongodb'); -const { getSymbolFrom } = require('../tools/utils'); -const { Long } = require('../mongodb'); -const { MongoRuntimeError } = require('../mongodb'); -const sinon = require('sinon'); -const { MongoClient } = require('../mongodb'); + BSON, + ClientSession, + isHello, + Long, + MongoClient, + MongoRuntimeError, + now, + ServerSession, + ServerSessionPool +} from '../mongodb'; +import { genClusterTime } from '../tools/common'; +import * as mock from '../tools/mongodb-mock/index'; +import { getSymbolFrom } from '../tools/utils'; describe('Sessions - unit', function () { let client; @@ -272,6 +272,60 @@ describe('Sessions - unit', function () { const txnNumberIncrementSymbol = getSymbolFrom(session, 'txnNumberIncrement'); expect(session).to.have.property(txnNumberIncrementSymbol, 0); }); + + describe('defaultTimeoutMS', function () { + let client: MongoClient; + let session: ClientSession; + let server; + + beforeEach(async () => { + server = await mock.createServer(); + }); + + afterEach(async () => { + await mock.cleanup(); + }); + + context('when client has defined timeoutMS', function () { + beforeEach(async () => { + client = new MongoClient(`mongodb://${server.hostAddress()}`, { timeoutMS: 100 }); + }); + + context('when defaultTimeoutMS is defined', function () { + it(`overrides client's timeoutMS value`, function () { + session = new ClientSession(client, serverSessionPool, { defaultTimeoutMS: 200 }); + expect(session).to.have.property('timeoutMS', 200); + }); + }); + + context('when defaultTimeoutMS is not defined', function () { + it(`inherits client's timeoutMS value`, function () { + session = new ClientSession(client, serverSessionPool, {}); + expect(session).to.have.property('timeoutMS', 100); + }); + }); + }); + + context('when client has not defined timeoutMS', function () { + beforeEach(async () => { + client = new MongoClient(`mongodb://${server.hostAddress()}`, {}); + }); + + context('when defaultTimeoutMS is defined', function () { + it(`sets timeoutMS to defaultTimeoutMS`, function () { + session = new ClientSession(client, serverSessionPool, { defaultTimeoutMS: 200 }); + expect(session).to.have.property('timeoutMS', 200); + }); + }); + + context('when defaultTimeoutMS is not defined', function () { + it(`leaves timeoutMS as undefined`, function () { + session = new ClientSession(client, serverSessionPool, {}); + expect(session.timeoutMS).to.be.undefined; + }); + }); + }); + }); }); describe('get serverSession()', () => { @@ -442,7 +496,7 @@ describe('Sessions - unit', function () { beforeEach(async () => { server = await mock.createServer(); server.setMessageHandler(request => { - var doc = request.document; + const doc = request.document; if (isHello(doc)) { request.reply(Object.assign({}, mock.HELLO, { logicalSessionTimeoutMinutes: 10 })); }