Skip to content

Commit

Permalink
feat(NODE-5678): add options parsing support for timeoutMS and defaul…
Browse files Browse the repository at this point in the history
…tTimeoutMS (#4068)

Co-authored-by: Durran Jordan <[email protected]>
  • Loading branch information
W-A-James and durran authored Apr 10, 2024
1 parent 30cac05 commit ddd1e81
Show file tree
Hide file tree
Showing 19 changed files with 309 additions and 49 deletions.
2 changes: 2 additions & 0 deletions src/collection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
3 changes: 3 additions & 0 deletions src/cursor/abstract_cursor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ export interface AbstractCursorOptions extends BSONSerializeOptions {
*/
awaitData?: boolean;
noCursorTimeout?: boolean;
/** @internal TODO(NODE-5688): make this public */
timeoutMS?: number;
}

/** @internal */
Expand Down Expand Up @@ -184,6 +186,7 @@ export abstract class AbstractCursor<
: ReadPreference.primary,
...pluckBSONSerializeOptions(options)
};
this[kOptions].timeoutMS = options.timeoutMS;

const readConcern = ReadConcern.fromOptions(options);
if (readConcern) {
Expand Down
5 changes: 4 additions & 1 deletion src/db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ const DB_OPTIONS_ALLOW_LIST = [
'enableUtf8Validation',
'promoteValues',
'compression',
'retryWrites'
'retryWrites',
'timeoutMS'
];

/** @internal */
Expand Down Expand Up @@ -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;
}

/**
Expand Down
2 changes: 2 additions & 0 deletions src/gridfs/download.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
2 changes: 2 additions & 0 deletions src/gridfs/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
2 changes: 2 additions & 0 deletions src/gridfs/upload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
4 changes: 3 additions & 1 deletion src/mongo_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
5 changes: 5 additions & 0 deletions src/operations/execute_operation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
3 changes: 3 additions & 0 deletions src/operations/operation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
6 changes: 6 additions & 0 deletions src/sessions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -131,6 +134,8 @@ export class ClientSession extends TypedEventEmitter<ClientSessionEvents> {
[kPinnedConnection]?: Connection;
/** @internal */
[kTxnNumberIncrement]: number;
/** @internal */
timeoutMS?: number;

/**
* Create a client session.
Expand Down Expand Up @@ -173,6 +178,7 @@ export class ClientSession extends TypedEventEmitter<ClientSessionEvents> {
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;
Expand Down
97 changes: 95 additions & 2 deletions test/integration/client-side-operations-timeout/node_csot.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
});
});
});
34 changes: 32 additions & 2 deletions test/spec/uri-options/connection-options.json
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -16,7 +16,8 @@
"replicaSet": "uri-options-spec",
"retryWrites": true,
"serverSelectionTimeoutMS": 15000,
"socketTimeoutMS": 7500
"socketTimeoutMS": 7500,
"timeoutMS": 100
}
},
{
Expand Down Expand Up @@ -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": {}
}
]
}
30 changes: 28 additions & 2 deletions test/spec/uri-options/connection-options.yml
Original file line number Diff line number Diff line change
@@ -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: ~
Expand All @@ -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"
Expand Down Expand Up @@ -64,7 +65,7 @@ tests:
hosts: ~
auth: ~
options: {}
-
-
description: "Invalid retryWrites causes a warning"
uri: "mongodb://example.com/?retryWrites=invalid"
valid: true
Expand Down Expand Up @@ -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: {}
1 change: 0 additions & 1 deletion test/spec/uri-options/connection-pool-options.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,3 @@ tests:
hosts: ~
auth: ~
options: {}

2 changes: 1 addition & 1 deletion test/spec/uri-options/read-preference-options.json
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion test/spec/uri-options/read-preference-options.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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: ~
Expand Down
1 change: 1 addition & 0 deletions test/tools/uri_spec_runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ export function executeUriValidationTest(
case 'maxConnecting':
case 'maxPoolSize':
case 'minPoolSize':
case 'timeoutMS':
case 'connectTimeoutMS':
case 'heartbeatFrequencyMS':
case 'localThresholdMS':
Expand Down
Loading

0 comments on commit ddd1e81

Please sign in to comment.