From fb86c85951bca7ea7f7ba03b126ecdeec3e26941 Mon Sep 17 00:00:00 2001 From: Warren James Date: Wed, 3 Apr 2024 15:40:38 -0400 Subject: [PATCH 01/25] convert to typescript --- test/unit/{mongo_client.test.js => mongo_client.test.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/unit/{mongo_client.test.js => mongo_client.test.ts} (100%) diff --git a/test/unit/mongo_client.test.js b/test/unit/mongo_client.test.ts similarity index 100% rename from test/unit/mongo_client.test.js rename to test/unit/mongo_client.test.ts From 042e9aa385da9d0ee8ea668021e2827185134c60 Mon Sep 17 00:00:00 2001 From: Warren James Date: Wed, 3 Apr 2024 15:45:05 -0400 Subject: [PATCH 02/25] small fixes --- test/unit/mongo_client.test.ts | 45 ++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/test/unit/mongo_client.test.ts b/test/unit/mongo_client.test.ts index e3b3beb92b..f302d48efb 100644 --- a/test/unit/mongo_client.test.ts +++ 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); From 2ac89dc28f61b8a89f3b697ba5a65a3929e91368 Mon Sep 17 00:00:00 2001 From: Warren James Date: Wed, 3 Apr 2024 16:06:15 -0400 Subject: [PATCH 03/25] add option parsing tests --- test/unit/connection_string.test.ts | 104 ++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/test/unit/connection_string.test.ts b/test/unit/connection_string.test.ts index 2a38fc491a..617ca41c8c 100644 --- a/test/unit/connection_string.test.ts +++ b/test/unit/connection_string.test.ts @@ -14,8 +14,10 @@ import { DOCUMENT_DB_MSG, FEATURE_FLAGS, type Log, + Long, MongoAPIError, MongoClient, + type MongoClientOptions, MongoCredentials, MongoDriverError, MongoInvalidArgumentError, @@ -922,6 +924,108 @@ describe('Connection String', function () { }); }); + describe('when timeoutMS is provided as a client option', function () { + const connString = 'mongodb://host'; + const types = ['number', 'bigint', 'Long']; + const values = [-100, 0, 100]; + + for (const type of types) { + describe(`when timeoutMS provided as a ${type}`, function () { + for (const v of values) { + const timeoutMS = + type === 'number' ? v : type === 'bigint' ? BigInt(v) : Long.fromNumber(v); + describe(`when timeoutMS is ${ + v < 0 ? 'negative' : v > 0 ? 'positive' : 'zero' + }`, function () { + if (v >= 0) { + it('sets the value correctly as a number', function () { + const options = parseOptions(connString, { timeoutMS }); + expect(options.timeoutMS).to.equal(v); + }); + } else { + it('throws MongoParseError', function () { + expect(() => parseOptions(connString, { timeoutMS })).to.throw(MongoParseError); + }); + } + }); + } + }); + } + }); + + describe('when defaultTimeoutMS is provided as a client option', function () { + const connString = 'mongodb://host'; + const types = ['number', 'bigint', 'Long']; + const values = [-100, 0, 100]; + + for (const type of types) { + describe(`when timeoutMS provided as a ${type}`, function () { + for (const v of values) { + const defaultTimeoutMS = + type === 'number' ? v : type === 'bigint' ? BigInt(v) : Long.fromNumber(v); + describe(`when timeoutMS is ${ + v < 0 ? 'negative' : v > 0 ? 'positive' : 'zero' + }`, function () { + if (v >= 0) { + it('sets the value correctly as a number', function () { + const options = parseOptions(connString, { defaultTimeoutMS }); + expect(options.defaultTimeoutMS).to.equal(v); + }); + } else { + it('throws MongoParseError', function () { + expect(() => parseOptions(connString, { defaultTimeoutMS })).to.throw( + MongoParseError + ); + }); + } + }); + } + }); + } + }); + + describe('when timeoutMS is provided in the connection string', function () { + const values = [-100, 0, 100]; + for (const v of values) { + const connString = `mongodb://host?timeoutMS=${v}`; + describe(`when timeoutMS is ${ + v < 0 ? 'negative' : v > 0 ? 'positive' : 'zero' + }`, function () { + if (v >= 0) { + it('sets the value correctly as a number', function () { + const options = parseOptions(connString); + expect(options.timeoutMS).to.equal(v); + }); + } else { + it('throws MongoParseError', function () { + expect(() => parseOptions(connString)).to.throw(MongoParseError); + }); + } + }); + } + }); + + describe('when defaultTimeoutMS is provided in the connection string', function () { + const values = [-100, 0, 100]; + for (const v of values) { + const connString = `mongodb://host?defaultTimeoutMS=${v}`; + describe(`when defaultTimeoutMS is ${ + v < 0 ? 'negative' : v > 0 ? 'positive' : 'zero' + }`, function () { + if (v >= 0) { + it('sets the value correctly as a number', function () { + const options = parseOptions(connString); + expect(options.defaultTimeoutMS).to.equal(v); + }); + } else { + it('throws MongoParseError', function () { + expect(() => parseOptions(connString)).to.throw(MongoParseError); + }); + } + }); + } + }); + describe('non-genuine hosts', () => { beforeEach(() => { process.env.MONGODB_LOG_CLIENT = 'info'; From 06622697ff726398f9e934850d3c0e7597f8293a Mon Sep 17 00:00:00 2001 From: Warren James Date: Wed, 3 Apr 2024 16:18:40 -0400 Subject: [PATCH 04/25] add client tests --- test/unit/mongo_client.test.ts | 48 ++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/test/unit/mongo_client.test.ts b/test/unit/mongo_client.test.ts index f302d48efb..8e56b897ff 100644 --- a/test/unit/mongo_client.test.ts +++ b/test/unit/mongo_client.test.ts @@ -1208,4 +1208,52 @@ 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); + }); + }); + }); + + context('defaultTimeoutMS', function () { + context('when defaultTimeoutMS is negative', function () { + it('throws MongoParseError', function () { + expect(() => new MongoClient('mongodb://host', { defaultTimeoutMS: -1 })).to.throw( + MongoParseError + ); + }); + }); + + context('when defaultTimeoutMS is positive', function () { + it('sets the value on the MongoClient', function () { + const client = new MongoClient('mongodb://host', { defaultTimeoutMS: 1 }); + expect(client.options.defaultTimeoutMS).to.equal(1); + }); + }); + + context('when defaultTimeoutMS is zero', function () { + it('sets the value on the MongoClient', function () { + const client = new MongoClient('mongodb://host', { defaultTimeoutMS: 0 }); + expect(client.options.defaultTimeoutMS).to.equal(0); + }); + }); + }); }); From cd562ca73aa1b2f9d3b4cfc222065a8e254b4aac Mon Sep 17 00:00:00 2001 From: Warren James Date: Wed, 3 Apr 2024 16:20:26 -0400 Subject: [PATCH 05/25] add client and connection string support for defaultTimeoutMS --- src/connection_string.ts | 3 +++ src/mongo_client.ts | 8 ++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/connection_string.ts b/src/connection_string.ts index c2abf08aaa..c22f54444b 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -1108,6 +1108,9 @@ export const OPTIONS = { timeoutMS: { type: 'uint' }, + defaultTimeoutMS: { + type: 'uint' + }, tls: { type: 'boolean' }, diff --git a/src/mongo_client.ts b/src/mongo_client.ts index 98a9f151ac..61dfeb35ee 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -2,7 +2,7 @@ import { promises as fs } from 'fs'; import type { TcpNetConnectOpts } from 'net'; import type { ConnectionOptions as TLSConnectionOptions, TLSSocketOptions } from 'tls'; -import { type BSONSerializeOptions, type Document, resolveBSONOptions } from './bson'; +import { type BSONSerializeOptions, type Document, type Long, resolveBSONOptions } from './bson'; import { ChangeStream, type ChangeStreamDocument, type ChangeStreamOptions } from './change_stream'; import type { AutoEncrypter, AutoEncryptionOptions } from './client-side-encryption/auto_encrypter'; import { @@ -121,7 +121,9 @@ export interface MongoClientOptions extends BSONSerializeOptions, SupportedNodeC /** 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. */ - timeoutMS?: number; + timeoutMS?: number | bigint | Long; + /** @internal This option is in development and currently has no behaviour. */ + defaultTimeoutMS?: number | bigint | Long; /** Enables or disables TLS/SSL for the connection. */ tls?: boolean; /** A boolean to enable or disables TLS/SSL for the connection. (The ssl option is equivalent to the tls option.) */ @@ -803,6 +805,8 @@ export interface MongoOptions | 'socketTimeoutMS' | 'srvMaxHosts' | 'srvServiceName' + | 'timeoutMS' + | 'defaultTimeoutMS' | 'tlsAllowInvalidCertificates' | 'tlsAllowInvalidHostnames' | 'tlsInsecure' From 79230291b5d120bf8aa1a649d13f9914a90fc2cf Mon Sep 17 00:00:00 2001 From: Warren James Date: Wed, 3 Apr 2024 16:50:24 -0400 Subject: [PATCH 06/25] lint fix --- test/unit/connection_string.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/unit/connection_string.test.ts b/test/unit/connection_string.test.ts index 617ca41c8c..4d46f0802a 100644 --- a/test/unit/connection_string.test.ts +++ b/test/unit/connection_string.test.ts @@ -17,7 +17,6 @@ import { Long, MongoAPIError, MongoClient, - type MongoClientOptions, MongoCredentials, MongoDriverError, MongoInvalidArgumentError, From 07b57a0d34ce3cebd802f1e2ec08b7b8a2bb3e42 Mon Sep 17 00:00:00 2001 From: Warren James Date: Wed, 3 Apr 2024 16:51:11 -0400 Subject: [PATCH 07/25] Add support for timeoutMS and defaultTimeoutMS to db --- src/db.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/db.ts b/src/db.ts index 1dda4834c2..c52d979eef 100644 --- a/src/db.ts +++ b/src/db.ts @@ -7,7 +7,7 @@ import { AggregationCursor } from './cursor/aggregation_cursor'; import { ListCollectionsCursor } from './cursor/list_collections_cursor'; import { RunCommandCursor, type RunCursorCommandOptions } from './cursor/run_command_cursor'; import { MongoInvalidArgumentError } from './error'; -import type { MongoClient, PkFactory } from './mongo_client'; +import type { MongoClient, MongoClientOptions, PkFactory } from './mongo_client'; import type { TODO_NODE_3286 } from './mongo_types'; import type { AggregateOptions } from './operations/aggregate'; import { CollectionsOperation } from './operations/collections'; @@ -66,7 +66,9 @@ const DB_OPTIONS_ALLOW_LIST = [ 'enableUtf8Validation', 'promoteValues', 'compression', - 'retryWrites' + 'retryWrites', + 'timeoutMS', + 'defaultTimeoutMS' ]; /** @internal */ @@ -81,7 +83,10 @@ export interface DbPrivate { } /** @public */ -export interface DbOptions extends BSONSerializeOptions, WriteConcernOptions { +export interface DbOptions + extends BSONSerializeOptions, + WriteConcernOptions, + Pick { /** If the database authentication is dependent on another databaseName. */ authSource?: string; /** Force server to assign _id values instead of driver. */ From 4c8219e8f98cd11238dac22b68df9f893e97509f Mon Sep 17 00:00:00 2001 From: Warren James Date: Wed, 3 Apr 2024 16:55:36 -0400 Subject: [PATCH 08/25] Add support for timeoutMS and defaultTimeoutMS to collection --- src/collection.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/collection.ts b/src/collection.ts index 8fcdf90bff..b481129393 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -10,7 +10,7 @@ import { ListSearchIndexesCursor, type ListSearchIndexesOptions } from './cursor/list_search_indexes_cursor'; -import type { Db } from './db'; +import type { Db, DbOptions } from './db'; import { MongoInvalidArgumentError } from './error'; import type { MongoClient, PkFactory } from './mongo_client'; import type { @@ -101,7 +101,10 @@ export interface ModifyResult { } /** @public */ -export interface CollectionOptions extends BSONSerializeOptions, WriteConcernOptions { +export interface CollectionOptions + extends BSONSerializeOptions, + WriteConcernOptions, + Pick { /** Specify a read concern for the collection. (only MongoDB 3.2 or higher supported) */ readConcern?: ReadConcernLike; /** The preferred read preference (ReadPreference.PRIMARY, ReadPreference.PRIMARY_PREFERRED, ReadPreference.SECONDARY, ReadPreference.SECONDARY_PREFERRED, ReadPreference.NEAREST). */ From e3a1548703e947a9bf82ef9e1386b99c55f55f0f Mon Sep 17 00:00:00 2001 From: Warren James Date: Wed, 3 Apr 2024 16:56:17 -0400 Subject: [PATCH 09/25] Add support for timeoutMS and defaultTimeoutMS to gridfs --- src/gridfs/download.ts | 4 +++- src/gridfs/index.ts | 5 ++++- src/gridfs/upload.ts | 6 ++++-- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/gridfs/download.ts b/src/gridfs/download.ts index 08659dfabf..8e8ca673f6 100644 --- a/src/gridfs/download.ts +++ b/src/gridfs/download.ts @@ -13,10 +13,12 @@ import type { FindOptions } from '../operations/find'; import type { ReadPreference } from '../read_preference'; import type { Sort } from '../sort'; import type { Callback } from '../utils'; +import { type GridFSBucketOptions } from '.'; import type { GridFSChunk } from './upload'; /** @public */ -export interface GridFSBucketReadStreamOptions { +export interface GridFSBucketReadStreamOptions + extends Pick { sort?: Sort; skip?: number; /** diff --git a/src/gridfs/index.ts b/src/gridfs/index.ts index c7611c852b..95cf43db55 100644 --- a/src/gridfs/index.ts +++ b/src/gridfs/index.ts @@ -3,6 +3,7 @@ import type { Collection } from '../collection'; import type { FindCursor } from '../cursor/find_cursor'; import type { Db } from '../db'; import { MongoRuntimeError } from '../error'; +import { type MongoClientOptions } from '../mongo_client'; import { type Filter, TypedEventEmitter } from '../mongo_types'; import type { ReadPreference } from '../read_preference'; import type { Sort } from '../sort'; @@ -29,7 +30,9 @@ const DEFAULT_GRIDFS_BUCKET_OPTIONS: { }; /** @public */ -export interface GridFSBucketOptions extends WriteConcernOptions { +export interface GridFSBucketOptions + extends WriteConcernOptions, + Pick { /** The 'files' and 'chunks' collections will be prefixed with the bucket name followed by a dot. */ bucketName?: string; /** Number of bytes stored in each chunk. Defaults to 255KB */ diff --git a/src/gridfs/upload.ts b/src/gridfs/upload.ts index 80024d7d32..9c013bfd0b 100644 --- a/src/gridfs/upload.ts +++ b/src/gridfs/upload.ts @@ -8,7 +8,7 @@ import { type Callback, squashError } from '../utils'; import type { WriteConcernOptions } from '../write_concern'; import { WriteConcern } from './../write_concern'; import type { GridFSFile } from './download'; -import type { GridFSBucket } from './index'; +import type { GridFSBucket, GridFSBucketOptions } from './index'; /** @public */ export interface GridFSChunk { @@ -19,7 +19,9 @@ export interface GridFSChunk { } /** @public */ -export interface GridFSBucketWriteStreamOptions extends WriteConcernOptions { +export interface GridFSBucketWriteStreamOptions + extends WriteConcernOptions, + Pick { /** Overwrite this bucket's chunkSizeBytes for this file */ chunkSizeBytes?: number; /** Custom file id for the GridFS file. */ From 504eba729b8b0873e4f79c310cb3ab54fe63433c Mon Sep 17 00:00:00 2001 From: Warren James Date: Wed, 3 Apr 2024 16:57:20 -0400 Subject: [PATCH 10/25] Add support for timeoutMS and defaultTimeoutMS to operations --- src/operations/operation.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/operations/operation.ts b/src/operations/operation.ts index 7f564cbc27..fb31665097 100644 --- a/src/operations/operation.ts +++ b/src/operations/operation.ts @@ -1,4 +1,5 @@ import { type BSONSerializeOptions, type Document, resolveBSONOptions } from '../bson'; +import { type MongoClientOptions } from '../mongo_client'; import { ReadPreference, type ReadPreferenceLike } from '../read_preference'; import type { Server } from '../sdam/server'; import type { ClientSession } from '../sessions'; @@ -23,7 +24,9 @@ export interface OperationConstructor extends Function { } /** @public */ -export interface OperationOptions extends BSONSerializeOptions { +export interface OperationOptions + extends BSONSerializeOptions, + Pick { /** Specify ClientSession for this command */ session?: ClientSession; willRetryWrite?: boolean; From dc1aaf4e98db8b3ebdbdcd0bfa82c3821838ec67 Mon Sep 17 00:00:00 2001 From: Warren James Date: Wed, 3 Apr 2024 16:57:44 -0400 Subject: [PATCH 11/25] Add support for timeoutMS and defaultTimeoutMS to sessions --- src/sessions.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/sessions.ts b/src/sessions.ts index 8f2892f0c0..6c4e82b203 100644 --- a/src/sessions.ts +++ b/src/sessions.ts @@ -23,7 +23,7 @@ import { MongoTransactionError, MongoWriteConcernError } from './error'; -import type { MongoClient, MongoOptions } from './mongo_client'; +import type { MongoClient, MongoClientOptions, MongoOptions } from './mongo_client'; import { TypedEventEmitter } from './mongo_types'; import { executeOperation } from './operations/execute_operation'; import { RunAdminCommandOperation } from './operations/run_command'; @@ -53,7 +53,8 @@ import { WriteConcern } from './write_concern'; const minWireVersionForShardedTransactions = 8; /** @public */ -export interface ClientSessionOptions { +export interface ClientSessionOptions + extends Pick { /** Whether causal consistency should be enabled on this session */ causalConsistency?: boolean; /** Whether all read operations should be read from the same snapshot for this session (NOTE: not compatible with `causalConsistency=true`) */ From 7142a0d1d63659b1de8bd07db2e7056e21d173bd Mon Sep 17 00:00:00 2001 From: Warren James Date: Thu, 4 Apr 2024 16:40:30 -0400 Subject: [PATCH 12/25] pull in uri spec tests --- test/spec/uri-options/connection-options.json | 34 +++++++++++++++++-- test/spec/uri-options/connection-options.yml | 30 ++++++++++++++-- .../uri-options/connection-pool-options.yml | 1 - .../uri-options/read-preference-options.json | 2 +- .../uri-options/read-preference-options.yml | 2 +- 5 files changed, 62 insertions(+), 7 deletions(-) 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: ~ From 75acc7b508f27038187855c1173e61169128b6bc Mon Sep 17 00:00:00 2001 From: Warren James Date: Thu, 4 Apr 2024 16:41:04 -0400 Subject: [PATCH 13/25] support timeoutMS option on uri-spec-runner --- test/tools/uri_spec_runner.ts | 1 + 1 file changed, 1 insertion(+) 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': From 524ccf9580ba4ef7b70ffa5c29ece65bc033c7a3 Mon Sep 17 00:00:00 2001 From: Warren James Date: Thu, 4 Apr 2024 16:41:38 -0400 Subject: [PATCH 14/25] remove defaultTimeoutMS option --- test/unit/connection_string.test.ts | 52 ----------------------------- test/unit/mongo_client.test.ts | 24 ------------- 2 files changed, 76 deletions(-) diff --git a/test/unit/connection_string.test.ts b/test/unit/connection_string.test.ts index 4d46f0802a..ab4b827d7a 100644 --- a/test/unit/connection_string.test.ts +++ b/test/unit/connection_string.test.ts @@ -952,37 +952,6 @@ describe('Connection String', function () { } }); - describe('when defaultTimeoutMS is provided as a client option', function () { - const connString = 'mongodb://host'; - const types = ['number', 'bigint', 'Long']; - const values = [-100, 0, 100]; - - for (const type of types) { - describe(`when timeoutMS provided as a ${type}`, function () { - for (const v of values) { - const defaultTimeoutMS = - type === 'number' ? v : type === 'bigint' ? BigInt(v) : Long.fromNumber(v); - describe(`when timeoutMS is ${ - v < 0 ? 'negative' : v > 0 ? 'positive' : 'zero' - }`, function () { - if (v >= 0) { - it('sets the value correctly as a number', function () { - const options = parseOptions(connString, { defaultTimeoutMS }); - expect(options.defaultTimeoutMS).to.equal(v); - }); - } else { - it('throws MongoParseError', function () { - expect(() => parseOptions(connString, { defaultTimeoutMS })).to.throw( - MongoParseError - ); - }); - } - }); - } - }); - } - }); - describe('when timeoutMS is provided in the connection string', function () { const values = [-100, 0, 100]; for (const v of values) { @@ -1004,27 +973,6 @@ describe('Connection String', function () { } }); - describe('when defaultTimeoutMS is provided in the connection string', function () { - const values = [-100, 0, 100]; - for (const v of values) { - const connString = `mongodb://host?defaultTimeoutMS=${v}`; - describe(`when defaultTimeoutMS is ${ - v < 0 ? 'negative' : v > 0 ? 'positive' : 'zero' - }`, function () { - if (v >= 0) { - it('sets the value correctly as a number', function () { - const options = parseOptions(connString); - expect(options.defaultTimeoutMS).to.equal(v); - }); - } else { - it('throws MongoParseError', function () { - expect(() => parseOptions(connString)).to.throw(MongoParseError); - }); - } - }); - } - }); - describe('non-genuine hosts', () => { beforeEach(() => { process.env.MONGODB_LOG_CLIENT = 'info'; diff --git a/test/unit/mongo_client.test.ts b/test/unit/mongo_client.test.ts index 8e56b897ff..79663a2603 100644 --- a/test/unit/mongo_client.test.ts +++ b/test/unit/mongo_client.test.ts @@ -1232,28 +1232,4 @@ describe('MongoClient', function () { }); }); }); - - context('defaultTimeoutMS', function () { - context('when defaultTimeoutMS is negative', function () { - it('throws MongoParseError', function () { - expect(() => new MongoClient('mongodb://host', { defaultTimeoutMS: -1 })).to.throw( - MongoParseError - ); - }); - }); - - context('when defaultTimeoutMS is positive', function () { - it('sets the value on the MongoClient', function () { - const client = new MongoClient('mongodb://host', { defaultTimeoutMS: 1 }); - expect(client.options.defaultTimeoutMS).to.equal(1); - }); - }); - - context('when defaultTimeoutMS is zero', function () { - it('sets the value on the MongoClient', function () { - const client = new MongoClient('mongodb://host', { defaultTimeoutMS: 0 }); - expect(client.options.defaultTimeoutMS).to.equal(0); - }); - }); - }); }); From 51724dbc520d8eed9d33ea103da260fe5bee79c9 Mon Sep 17 00:00:00 2001 From: Warren James Date: Thu, 4 Apr 2024 17:01:23 -0400 Subject: [PATCH 15/25] fix option implementation --- src/collection.ts | 9 ++++----- src/connection_string.ts | 3 --- src/db.ts | 12 +++++------- src/gridfs/download.ts | 6 +++--- src/gridfs/index.ts | 7 +++---- src/gridfs/upload.ts | 8 ++++---- src/mongo_client.ts | 10 ++++------ src/operations/operation.ts | 8 ++++---- src/sessions.ts | 12 +++++++++--- 9 files changed, 36 insertions(+), 39 deletions(-) diff --git a/src/collection.ts b/src/collection.ts index b481129393..180e9822e8 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -10,7 +10,7 @@ import { ListSearchIndexesCursor, type ListSearchIndexesOptions } from './cursor/list_search_indexes_cursor'; -import type { Db, DbOptions } from './db'; +import type { Db } from './db'; import { MongoInvalidArgumentError } from './error'; import type { MongoClient, PkFactory } from './mongo_client'; import type { @@ -101,14 +101,13 @@ export interface ModifyResult { } /** @public */ -export interface CollectionOptions - extends BSONSerializeOptions, - WriteConcernOptions, - Pick { +export interface CollectionOptions extends BSONSerializeOptions, WriteConcernOptions { /** Specify a read concern for the collection. (only MongoDB 3.2 or higher supported) */ readConcern?: ReadConcernLike; /** The preferred read preference (ReadPreference.PRIMARY, ReadPreference.PRIMARY_PREFERRED, ReadPreference.SECONDARY, ReadPreference.SECONDARY_PREFERRED, ReadPreference.NEAREST). */ readPreference?: ReadPreferenceLike; + /** @internal */ + timeoutMS?: number; } /** @internal */ diff --git a/src/connection_string.ts b/src/connection_string.ts index c22f54444b..c2abf08aaa 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -1108,9 +1108,6 @@ export const OPTIONS = { timeoutMS: { type: 'uint' }, - defaultTimeoutMS: { - type: 'uint' - }, tls: { type: 'boolean' }, diff --git a/src/db.ts b/src/db.ts index c52d979eef..a3269a58e0 100644 --- a/src/db.ts +++ b/src/db.ts @@ -7,7 +7,7 @@ import { AggregationCursor } from './cursor/aggregation_cursor'; import { ListCollectionsCursor } from './cursor/list_collections_cursor'; import { RunCommandCursor, type RunCursorCommandOptions } from './cursor/run_command_cursor'; import { MongoInvalidArgumentError } from './error'; -import type { MongoClient, MongoClientOptions, PkFactory } from './mongo_client'; +import type { MongoClient, PkFactory } from './mongo_client'; import type { TODO_NODE_3286 } from './mongo_types'; import type { AggregateOptions } from './operations/aggregate'; import { CollectionsOperation } from './operations/collections'; @@ -67,8 +67,7 @@ const DB_OPTIONS_ALLOW_LIST = [ 'promoteValues', 'compression', 'retryWrites', - 'timeoutMS', - 'defaultTimeoutMS' + 'timeoutMS' ]; /** @internal */ @@ -83,10 +82,7 @@ export interface DbPrivate { } /** @public */ -export interface DbOptions - extends BSONSerializeOptions, - WriteConcernOptions, - Pick { +export interface DbOptions extends BSONSerializeOptions, WriteConcernOptions { /** If the database authentication is dependent on another databaseName. */ authSource?: string; /** Force server to assign _id values instead of driver. */ @@ -99,6 +95,8 @@ export interface DbOptions readConcern?: ReadConcern; /** Should retry failed writes */ retryWrites?: boolean; + /** @internal */ + timeoutMS?: number; } /** diff --git a/src/gridfs/download.ts b/src/gridfs/download.ts index 8e8ca673f6..19c1e53b89 100644 --- a/src/gridfs/download.ts +++ b/src/gridfs/download.ts @@ -13,12 +13,10 @@ import type { FindOptions } from '../operations/find'; import type { ReadPreference } from '../read_preference'; import type { Sort } from '../sort'; import type { Callback } from '../utils'; -import { type GridFSBucketOptions } from '.'; import type { GridFSChunk } from './upload'; /** @public */ -export interface GridFSBucketReadStreamOptions - extends Pick { +export interface GridFSBucketReadStreamOptions { sort?: Sort; skip?: number; /** @@ -30,6 +28,8 @@ export interface GridFSBucketReadStreamOptions * to be returned by the stream. `end` is non-inclusive */ end?: number; + /** @internal */ + timeoutMS?: number; } /** @public */ diff --git a/src/gridfs/index.ts b/src/gridfs/index.ts index 95cf43db55..f89428f5e2 100644 --- a/src/gridfs/index.ts +++ b/src/gridfs/index.ts @@ -3,7 +3,6 @@ import type { Collection } from '../collection'; import type { FindCursor } from '../cursor/find_cursor'; import type { Db } from '../db'; import { MongoRuntimeError } from '../error'; -import { type MongoClientOptions } from '../mongo_client'; import { type Filter, TypedEventEmitter } from '../mongo_types'; import type { ReadPreference } from '../read_preference'; import type { Sort } from '../sort'; @@ -30,15 +29,15 @@ const DEFAULT_GRIDFS_BUCKET_OPTIONS: { }; /** @public */ -export interface GridFSBucketOptions - extends WriteConcernOptions, - Pick { +export interface GridFSBucketOptions extends WriteConcernOptions { /** The 'files' and 'chunks' collections will be prefixed with the bucket name followed by a dot. */ bucketName?: string; /** Number of bytes stored in each chunk. Defaults to 255KB */ chunkSizeBytes?: number; /** Read preference to be passed to read operations */ readPreference?: ReadPreference; + /** @internal */ + timeoutMS?: number; } /** @internal */ diff --git a/src/gridfs/upload.ts b/src/gridfs/upload.ts index 9c013bfd0b..3f5559301f 100644 --- a/src/gridfs/upload.ts +++ b/src/gridfs/upload.ts @@ -8,7 +8,7 @@ import { type Callback, squashError } from '../utils'; import type { WriteConcernOptions } from '../write_concern'; import { WriteConcern } from './../write_concern'; import type { GridFSFile } from './download'; -import type { GridFSBucket, GridFSBucketOptions } from './index'; +import type { GridFSBucket } from './index'; /** @public */ export interface GridFSChunk { @@ -19,9 +19,7 @@ export interface GridFSChunk { } /** @public */ -export interface GridFSBucketWriteStreamOptions - extends WriteConcernOptions, - Pick { +export interface GridFSBucketWriteStreamOptions extends WriteConcernOptions { /** Overwrite this bucket's chunkSizeBytes for this file */ chunkSizeBytes?: number; /** Custom file id for the GridFS file. */ @@ -38,6 +36,8 @@ export interface GridFSBucketWriteStreamOptions * @deprecated Will be removed in the next major version. Add an aliases field to the metadata document instead. */ aliases?: string[]; + /** @internal */ + timeoutMS?: number; } /** diff --git a/src/mongo_client.ts b/src/mongo_client.ts index 61dfeb35ee..df64543f33 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -2,7 +2,7 @@ import { promises as fs } from 'fs'; import type { TcpNetConnectOpts } from 'net'; import type { ConnectionOptions as TLSConnectionOptions, TLSSocketOptions } from 'tls'; -import { type BSONSerializeOptions, type Document, type Long, resolveBSONOptions } from './bson'; +import { type BSONSerializeOptions, type Document, resolveBSONOptions } from './bson'; import { ChangeStream, type ChangeStreamDocument, type ChangeStreamOptions } from './change_stream'; import type { AutoEncrypter, AutoEncryptionOptions } from './client-side-encryption/auto_encrypter'; import { @@ -121,9 +121,7 @@ export interface MongoClientOptions extends BSONSerializeOptions, SupportedNodeC /** 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. */ - timeoutMS?: number | bigint | Long; - /** @internal This option is in development and currently has no behaviour. */ - defaultTimeoutMS?: number | bigint | Long; + timeoutMS?: number; /** Enables or disables TLS/SSL for the connection. */ tls?: boolean; /** A boolean to enable or disables TLS/SSL for the connection. (The ssl option is equivalent to the tls option.) */ @@ -805,8 +803,6 @@ export interface MongoOptions | 'socketTimeoutMS' | 'srvMaxHosts' | 'srvServiceName' - | 'timeoutMS' - | 'defaultTimeoutMS' | 'tlsAllowInvalidCertificates' | 'tlsAllowInvalidHostnames' | 'tlsInsecure' @@ -901,4 +897,6 @@ export interface MongoOptions * TODO: NODE-5671 - remove internal flag */ mongodbLogPath?: 'stderr' | 'stdout' | MongoDBLogWritable; + /** @internal */ + timeoutMS?: number; } diff --git a/src/operations/operation.ts b/src/operations/operation.ts index fb31665097..8e4a22b547 100644 --- a/src/operations/operation.ts +++ b/src/operations/operation.ts @@ -1,5 +1,4 @@ import { type BSONSerializeOptions, type Document, resolveBSONOptions } from '../bson'; -import { type MongoClientOptions } from '../mongo_client'; import { ReadPreference, type ReadPreferenceLike } from '../read_preference'; import type { Server } from '../sdam/server'; import type { ClientSession } from '../sessions'; @@ -24,9 +23,7 @@ export interface OperationConstructor extends Function { } /** @public */ -export interface OperationOptions - extends BSONSerializeOptions, - Pick { +export interface OperationOptions extends BSONSerializeOptions { /** Specify ClientSession for this command */ session?: ClientSession; willRetryWrite?: boolean; @@ -37,6 +34,9 @@ export interface OperationOptions /** @internal Hints to `executeOperation` that this operation should not unpin on an ended transaction */ bypassPinningCheck?: boolean; omitReadPreference?: boolean; + + /** @internal */ + timeoutMS?: number; } /** @internal */ diff --git a/src/sessions.ts b/src/sessions.ts index 6c4e82b203..76951fcf84 100644 --- a/src/sessions.ts +++ b/src/sessions.ts @@ -23,7 +23,7 @@ import { MongoTransactionError, MongoWriteConcernError } from './error'; -import type { MongoClient, MongoClientOptions, MongoOptions } from './mongo_client'; +import type { MongoClient, MongoOptions } from './mongo_client'; import { TypedEventEmitter } from './mongo_types'; import { executeOperation } from './operations/execute_operation'; import { RunAdminCommandOperation } from './operations/run_command'; @@ -53,14 +53,18 @@ import { WriteConcern } from './write_concern'; const minWireVersionForShardedTransactions = 8; /** @public */ -export interface ClientSessionOptions - extends Pick { +export interface ClientSessionOptions { /** Whether causal consistency should be enabled on this session */ causalConsistency?: boolean; /** Whether all read operations should be read from the same snapshot for this session (NOTE: not compatible with `causalConsistency=true`) */ snapshot?: boolean; /** The default TransactionOptions to use for transactions started on this session. */ defaultTransactionOptions?: TransactionOptions; + /** @internal */ + timeoutMS?: number; + /** @internal + * The value of timeoutMS used for CSOT. Used to override client timeoutMS */ + defaultTimeoutMS?: number; /** @internal */ owner?: symbol | AbstractCursor; @@ -169,6 +173,8 @@ export class ClientSession extends TypedEventEmitter { } } + options.timeoutMS ??= options.defaultTimeoutMS ?? client.options.timeoutMS; + this.client = client; this.sessionPool = sessionPool; this.hasEnded = false; From eb3757501c0ebd2b078c79c257b02f496371da11 Mon Sep 17 00:00:00 2001 From: Warren James Date: Thu, 4 Apr 2024 17:34:59 -0400 Subject: [PATCH 16/25] access timeoutMS optionally --- src/sessions.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/sessions.ts b/src/sessions.ts index 76951fcf84..79b03da2d3 100644 --- a/src/sessions.ts +++ b/src/sessions.ts @@ -135,6 +135,8 @@ export class ClientSession extends TypedEventEmitter { [kPinnedConnection]?: Connection; /** @internal */ [kTxnNumberIncrement]: number; + /** @internal */ + timeoutMS?: number; /** * Create a client session. @@ -173,12 +175,11 @@ export class ClientSession extends TypedEventEmitter { } } - options.timeoutMS ??= options.defaultTimeoutMS ?? client.options.timeoutMS; - this.client = client; 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; From 37c8ae2f590c59d00c9dd5ebe4e21c6f9068caa0 Mon Sep 17 00:00:00 2001 From: Warren James Date: Thu, 4 Apr 2024 17:35:28 -0400 Subject: [PATCH 17/25] migrate file to typescript and add additional tests --- .../{sessions.test.js => sessions.test.ts} | 88 +++++++++++++++---- 1 file changed, 71 insertions(+), 17 deletions(-) rename test/unit/{sessions.test.js => sessions.test.ts} (90%) 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 })); } From b860acca8c4cb59a7802d45da7339eb0b83b4714 Mon Sep 17 00:00:00 2001 From: Warren James Date: Fri, 5 Apr 2024 13:56:48 -0400 Subject: [PATCH 18/25] add inheritance tests --- .../node_csot.test.ts | 96 ++++++++++++++++++- 1 file changed, 94 insertions(+), 2 deletions(-) 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..a187c10a2d 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,96 @@ -/* 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 () => { + if (client) 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(); + }); + + it('overrides the value provided on the db and the session', 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); + }); + }); + }); + }); +}); From fec996cbf83003364e4b5ddc9b0cf156102c34b0 Mon Sep 17 00:00:00 2001 From: Warren James Date: Fri, 5 Apr 2024 14:13:12 -0400 Subject: [PATCH 19/25] remove field from options --- src/sessions.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/sessions.ts b/src/sessions.ts index 79b03da2d3..a5acf2aed6 100644 --- a/src/sessions.ts +++ b/src/sessions.ts @@ -60,8 +60,6 @@ export interface ClientSessionOptions { snapshot?: boolean; /** The default TransactionOptions to use for transactions started on this session. */ defaultTransactionOptions?: TransactionOptions; - /** @internal */ - timeoutMS?: number; /** @internal * The value of timeoutMS used for CSOT. Used to override client timeoutMS */ defaultTimeoutMS?: number; From a7cff11da4f89d6d81e0e99ae4f3ebe036c5374a Mon Sep 17 00:00:00 2001 From: Warren James Date: Fri, 5 Apr 2024 14:46:28 -0400 Subject: [PATCH 20/25] add error logich when timeoutMS is specified on explicit session and at operation level --- src/operations/operation.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/operations/operation.ts b/src/operations/operation.ts index 8e4a22b547..921db13db0 100644 --- a/src/operations/operation.ts +++ b/src/operations/operation.ts @@ -1,4 +1,5 @@ import { type BSONSerializeOptions, type Document, resolveBSONOptions } from '../bson'; +import { MongoInvalidArgumentError } from '../error'; import { ReadPreference, type ReadPreferenceLike } from '../read_preference'; import type { Server } from '../sdam/server'; import type { ClientSession } from '../sessions'; @@ -35,7 +36,7 @@ export interface OperationOptions extends BSONSerializeOptions { bypassPinningCheck?: boolean; omitReadPreference?: boolean; - /** @internal */ + /** @internal TODO(NODE-5688): make this public */ timeoutMS?: number; } @@ -72,6 +73,11 @@ export abstract class AbstractOperation { this.bsonOptions = resolveBSONOptions(options); this[kSession] = options.session != null ? options.session : undefined; + if (options?.session?.explicit && options?.session?.timeoutMS != null && options.timeoutMS) { + throw new MongoInvalidArgumentError( + 'Do not specify timeoutMS on operation if already specified on an explicit session' + ); + } this.options = options; this.bypassPinningCheck = !!options.bypassPinningCheck; From d34ae2f8a1e6c22b11f508dc2e0dcffc8e4232cc Mon Sep 17 00:00:00 2001 From: Warren James Date: Fri, 5 Apr 2024 14:46:37 -0400 Subject: [PATCH 21/25] Add todo comments --- src/collection.ts | 2 +- src/cursor/abstract_cursor.ts | 3 +++ src/db.ts | 2 +- src/gridfs/download.ts | 2 +- src/gridfs/index.ts | 2 +- src/gridfs/upload.ts | 2 +- src/mongo_client.ts | 4 ++-- 7 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/collection.ts b/src/collection.ts index 180e9822e8..3665498995 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -106,7 +106,7 @@ 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 */ + /** @internal TODO(NODE-5688): make this public */ timeoutMS?: number; } 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 a3269a58e0..423979a0d4 100644 --- a/src/db.ts +++ b/src/db.ts @@ -95,7 +95,7 @@ export interface DbOptions extends BSONSerializeOptions, WriteConcernOptions { readConcern?: ReadConcern; /** Should retry failed writes */ retryWrites?: boolean; - /** @internal */ + /** @internal TODO(NODE-5688): make this public */ timeoutMS?: number; } diff --git a/src/gridfs/download.ts b/src/gridfs/download.ts index 19c1e53b89..6b0fc57fc6 100644 --- a/src/gridfs/download.ts +++ b/src/gridfs/download.ts @@ -28,7 +28,7 @@ export interface GridFSBucketReadStreamOptions { * to be returned by the stream. `end` is non-inclusive */ end?: number; - /** @internal */ + /** @internal TODO(NODE-5688): make this public */ timeoutMS?: number; } diff --git a/src/gridfs/index.ts b/src/gridfs/index.ts index f89428f5e2..51c32b7a01 100644 --- a/src/gridfs/index.ts +++ b/src/gridfs/index.ts @@ -36,7 +36,7 @@ export interface GridFSBucketOptions extends WriteConcernOptions { chunkSizeBytes?: number; /** Read preference to be passed to read operations */ readPreference?: ReadPreference; - /** @internal */ + /** @internal TODO(NODE-5688): make this public */ timeoutMS?: number; } diff --git a/src/gridfs/upload.ts b/src/gridfs/upload.ts index 3f5559301f..12440f2670 100644 --- a/src/gridfs/upload.ts +++ b/src/gridfs/upload.ts @@ -36,7 +36,7 @@ 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 */ + /** @internal TODO(NODE-5688): make this public */ timeoutMS?: number; } diff --git a/src/mongo_client.ts b/src/mongo_client.ts index df64543f33..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,6 +897,6 @@ export interface MongoOptions * TODO: NODE-5671 - remove internal flag */ mongodbLogPath?: 'stderr' | 'stdout' | MongoDBLogWritable; - /** @internal */ + /** @internal TODO(NODE-5688): make this public */ timeoutMS?: number; } From d8f97a2093340730b35223266cf1bb3296b65211 Mon Sep 17 00:00:00 2001 From: Warren James Date: Fri, 5 Apr 2024 14:46:45 -0400 Subject: [PATCH 22/25] add csot tests --- .../client-side-operations-timeout/node_csot.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 a187c10a2d..4b1b0558b7 100644 --- a/test/integration/client-side-operations-timeout/node_csot.test.ts +++ b/test/integration/client-side-operations-timeout/node_csot.test.ts @@ -48,7 +48,7 @@ describe('CSOT driver tests', () => { await session.endSession(); }); - it('overrides the value provided on the db and the session', async () => { + it('throws an error', async () => { expect(cursor.cursorOptions).to.have.property('timeoutMS', 500); }); }); From 11f26e18643b3c5f23ff0260a3c17a3cbbad00a6 Mon Sep 17 00:00:00 2001 From: Warren James Date: Fri, 5 Apr 2024 14:57:39 -0400 Subject: [PATCH 23/25] revert duplicated tests --- test/unit/connection_string.test.ts | 51 ----------------------------- 1 file changed, 51 deletions(-) diff --git a/test/unit/connection_string.test.ts b/test/unit/connection_string.test.ts index ab4b827d7a..2a38fc491a 100644 --- a/test/unit/connection_string.test.ts +++ b/test/unit/connection_string.test.ts @@ -14,7 +14,6 @@ import { DOCUMENT_DB_MSG, FEATURE_FLAGS, type Log, - Long, MongoAPIError, MongoClient, MongoCredentials, @@ -923,56 +922,6 @@ describe('Connection String', function () { }); }); - describe('when timeoutMS is provided as a client option', function () { - const connString = 'mongodb://host'; - const types = ['number', 'bigint', 'Long']; - const values = [-100, 0, 100]; - - for (const type of types) { - describe(`when timeoutMS provided as a ${type}`, function () { - for (const v of values) { - const timeoutMS = - type === 'number' ? v : type === 'bigint' ? BigInt(v) : Long.fromNumber(v); - describe(`when timeoutMS is ${ - v < 0 ? 'negative' : v > 0 ? 'positive' : 'zero' - }`, function () { - if (v >= 0) { - it('sets the value correctly as a number', function () { - const options = parseOptions(connString, { timeoutMS }); - expect(options.timeoutMS).to.equal(v); - }); - } else { - it('throws MongoParseError', function () { - expect(() => parseOptions(connString, { timeoutMS })).to.throw(MongoParseError); - }); - } - }); - } - }); - } - }); - - describe('when timeoutMS is provided in the connection string', function () { - const values = [-100, 0, 100]; - for (const v of values) { - const connString = `mongodb://host?timeoutMS=${v}`; - describe(`when timeoutMS is ${ - v < 0 ? 'negative' : v > 0 ? 'positive' : 'zero' - }`, function () { - if (v >= 0) { - it('sets the value correctly as a number', function () { - const options = parseOptions(connString); - expect(options.timeoutMS).to.equal(v); - }); - } else { - it('throws MongoParseError', function () { - expect(() => parseOptions(connString)).to.throw(MongoParseError); - }); - } - }); - } - }); - describe('non-genuine hosts', () => { beforeEach(() => { process.env.MONGODB_LOG_CLIENT = 'info'; From aa2a8bb5f7780110c0bdfeb42b3f2668f0d4d84d Mon Sep 17 00:00:00 2001 From: Warren James Date: Mon, 8 Apr 2024 10:01:44 -0400 Subject: [PATCH 24/25] Apply suggestions from code review Co-authored-by: Durran Jordan --- .../client-side-operations-timeout/node_csot.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) 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 4b1b0558b7..b6a936afbb 100644 --- a/test/integration/client-side-operations-timeout/node_csot.test.ts +++ b/test/integration/client-side-operations-timeout/node_csot.test.ts @@ -26,7 +26,7 @@ describe('CSOT driver tests', () => { }); afterEach(async () => { - if (client) await client.close(); + await client?.close(); }); describe('when timeoutMS is provided on an operation', () => { @@ -44,7 +44,8 @@ describe('CSOT driver tests', () => { }); afterEach(async () => { - await cursor.close(); + await cursor?.close(); + await session?.endSession(); await session.endSession(); }); @@ -63,7 +64,7 @@ describe('CSOT driver tests', () => { }); afterEach(async () => { - await cursor.close(); + await cursor?.close(); }); it('overrides the value provided on the db', async () => { From 5ef3ac1b373d213a6f6fb8ca516b640acb834130 Mon Sep 17 00:00:00 2001 From: Warren James Date: Tue, 9 Apr 2024 17:13:21 -0400 Subject: [PATCH 25/25] move session check logic to executeOperation --- src/operations/execute_operation.ts | 5 +++++ src/operations/operation.ts | 6 ------ 2 files changed, 5 insertions(+), 6 deletions(-) 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 921db13db0..e71baa44a9 100644 --- a/src/operations/operation.ts +++ b/src/operations/operation.ts @@ -1,5 +1,4 @@ import { type BSONSerializeOptions, type Document, resolveBSONOptions } from '../bson'; -import { MongoInvalidArgumentError } from '../error'; import { ReadPreference, type ReadPreferenceLike } from '../read_preference'; import type { Server } from '../sdam/server'; import type { ClientSession } from '../sessions'; @@ -73,11 +72,6 @@ export abstract class AbstractOperation { this.bsonOptions = resolveBSONOptions(options); this[kSession] = options.session != null ? options.session : undefined; - if (options?.session?.explicit && options?.session?.timeoutMS != null && options.timeoutMS) { - throw new MongoInvalidArgumentError( - 'Do not specify timeoutMS on operation if already specified on an explicit session' - ); - } this.options = options; this.bypassPinningCheck = !!options.bypassPinningCheck;