Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat(NODE-5678): add options parsing support for timeoutMS and defaultTimeoutMS #4068

Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
fb86c85
convert to typescript
W-A-James Apr 3, 2024
042e9aa
small fixes
W-A-James Apr 3, 2024
2ac89dc
add option parsing tests
W-A-James Apr 3, 2024
0662269
add client tests
W-A-James Apr 3, 2024
cd562ca
add client and connection string support for defaultTimeoutMS
W-A-James Apr 3, 2024
7923029
lint fix
W-A-James Apr 3, 2024
07b57a0
Add support for timeoutMS and defaultTimeoutMS to db
W-A-James Apr 3, 2024
4c8219e
Add support for timeoutMS and defaultTimeoutMS to collection
W-A-James Apr 3, 2024
e3a1548
Add support for timeoutMS and defaultTimeoutMS to gridfs
W-A-James Apr 3, 2024
504eba7
Add support for timeoutMS and defaultTimeoutMS to operations
W-A-James Apr 3, 2024
dc1aaf4
Add support for timeoutMS and defaultTimeoutMS to sessions
W-A-James Apr 3, 2024
7142a0d
pull in uri spec tests
W-A-James Apr 4, 2024
75acc7b
support timeoutMS option on uri-spec-runner
W-A-James Apr 4, 2024
524ccf9
remove defaultTimeoutMS option
W-A-James Apr 4, 2024
51724db
fix option implementation
W-A-James Apr 4, 2024
eb37575
access timeoutMS optionally
W-A-James Apr 4, 2024
37c8ae2
migrate file to typescript and add additional tests
W-A-James Apr 4, 2024
b860acc
add inheritance tests
W-A-James Apr 5, 2024
fec996c
remove field from options
W-A-James Apr 5, 2024
a7cff11
add error logich when timeoutMS is specified on explicit session and …
W-A-James Apr 5, 2024
d34ae2f
Add todo comments
W-A-James Apr 5, 2024
d8f97a2
add csot tests
W-A-James Apr 5, 2024
11f26e1
revert duplicated tests
W-A-James Apr 5, 2024
0d87b05
Merge branch 'main' into NODE-5678/add-options-parsing-support-for-ti…
W-A-James Apr 5, 2024
aa2a8bb
Apply suggestions from code review
W-A-James Apr 8, 2024
5ef3ac1
move session check logic to executeOperation
W-A-James Apr 9, 2024
8f23ae3
Merge branch 'main' into NODE-5678/add-options-parsing-support-for-ti…
durran Apr 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 */
timeoutMS?: number;
}

/** @internal */
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 */
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 */
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 */
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 */
timeoutMS?: number;
}

/**
Expand Down
2 changes: 2 additions & 0 deletions src/mongo_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -897,4 +897,6 @@ export interface MongoOptions
* TODO: NODE-5671 - remove internal flag
*/
mongodbLogPath?: 'stderr' | 'stdout' | MongoDBLogWritable;
/** @internal */
timeoutMS?: number;
}
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 */
timeoutMS?: number;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Applies to all operations, including cursor operations

}

/** @internal */
Expand Down
6 changes: 6 additions & 0 deletions src/sessions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ export interface ClientSessionOptions {
snapshot?: boolean;
/** The default TransactionOptions to use for transactions started on this session. */
defaultTransactionOptions?: TransactionOptions;
durran marked this conversation as resolved.
Show resolved Hide resolved
/** @internal
* The value of timeoutMS used for CSOT. Used to override client timeoutMS */
defaultTimeoutMS?: number;

/** @internal */
owner?: symbol | AbstractCursor;
Expand Down Expand Up @@ -130,6 +133,8 @@ export class ClientSession extends TypedEventEmitter<ClientSessionEvents> {
[kPinnedConnection]?: Connection;
/** @internal */
[kTxnNumberIncrement]: number;
/** @internal */
timeoutMS?: number;

/**
* Create a client session.
Expand Down Expand Up @@ -172,6 +177,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
96 changes: 94 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,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();
});
W-A-James marked this conversation as resolved.
Show resolved Hide resolved

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();
W-A-James marked this conversation as resolved.
Show resolved Hide resolved
});

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();
});
W-A-James marked this conversation as resolved.
Show resolved Hide resolved

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
Loading