Skip to content

docs(NODE-6516): add documentation for MongoOperationTimeoutError and fix internal references #4315

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion etc/notes/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,9 @@ This class should **never** be directly instantiated.

### `MongoOperationTimeoutError`

- TODO(NODE-6491): Add MongoOperationTimeoutError documentation
The `MongoOperationTimeoutError` class represents an error that occurs when an operation could not be completed within the specified `timeoutMS`.
It is generated by the driver in support of the "client side operation timeout" feature and inherits from `MongoDriverError`.
When `timeoutMS` is enabled `MongoServerErrors` relating to `MaxTimeExpired` errors will be converted to `MongoOperationTimeoutError`.

### MongoUnexpectedServerResponseError

Expand Down
2 changes: 1 addition & 1 deletion src/client-side-encryption/auto_encrypter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ export class AutoEncrypter {
if (net.getDefaultAutoSelectFamily) {
// AutoEncrypter is made inside of MongoClient constructor while options are being parsed,
// we do not have access to the options that are in progress.
// TODO(NODE-NODE-6449): AutoEncrypter does not use client options for autoSelectFamily
// TODO(NODE-6449): AutoEncrypter does not use client options for autoSelectFamily
Object.assign(clientOptions, autoSelectSocketOptions(this._client.s?.options ?? {}));
}

Expand Down
4 changes: 2 additions & 2 deletions src/collection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -687,8 +687,8 @@ export class Collection<TSchema extends Document = Document> {
);
return true;
} catch (error) {
if (error instanceof MongoOperationTimeoutError) throw error; // TODO: Check the spec for index management behaviour/file a drivers ticket for this
// Seems like we should throw all errors
// TODO(NODE-6517): Driver should only filter for namespace not found error. Other errors should be thrown.
if (error instanceof MongoOperationTimeoutError) throw error;
return false;
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -861,7 +861,9 @@ export class MongoUnexpectedServerResponseError extends MongoRuntimeError {
* @public
* @category Error
*
* This error is thrown when an operation could not be completed within the specified `timeoutMS`.
* The `MongoOperationTimeoutError` class represents an error that occurs when an operation could not be completed within the specified `timeoutMS`.
* It is generated by the driver in support of the "client side operation timeout" feature so inherits from `MongoDriverError`.
* When `timeoutMS` is enabled `MongoServerError`s relating to `MaxTimeExpired` errors will be converted to `MongoOperationTimeoutError`
*
* @example
* ```ts
Expand Down
5 changes: 4 additions & 1 deletion src/gridfs/download.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ export interface GridFSBucketReadStreamOptions {
* to be returned by the stream. `end` is non-inclusive
*/
end?: number;
/** @public */
/**
* @experimental
* Specifies the time an operation will run until it throws a timeout error
*/
timeoutMS?: number;
}

Expand Down
5 changes: 4 additions & 1 deletion src/gridfs/upload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ 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[];
/** @public */
/**
* @experimental
* Specifies the time an operation will run until it throws a timeout error
*/
timeoutMS?: number;
}

Expand Down
3 changes: 1 addition & 2 deletions src/sdam/topology.ts
Original file line number Diff line number Diff line change
Expand Up @@ -460,11 +460,10 @@ export class Topology extends TypedEventEmitter<TopologyEvents> {
}
}

// TODO(NODE-6223): auto connect cannot use timeoutMS
// const timeoutMS = this.client.s.options.timeoutMS;
const serverSelectionTimeoutMS = this.client.s.options.serverSelectionTimeoutMS;
const readPreference = options.readPreference ?? ReadPreference.primary;
const timeoutContext = TimeoutContext.create({
// TODO(NODE-6448): auto-connect ignores timeoutMS; potential future feature
timeoutMS: undefined,
serverSelectionTimeoutMS,
waitQueueTimeoutMS: this.client.s.options.waitQueueTimeoutMS
Expand Down
1 change: 1 addition & 0 deletions src/sessions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export interface ClientSessionOptions {
defaultTransactionOptions?: TransactionOptions;
/**
* @public
* @experimental
* An overriding timeoutMS value to use for a client-side timeout.
* If not provided the session uses the timeoutMS specified on the MongoClient.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import {
import { type FailPoint, makeMultiBatchWrite, measureDuration } from '../../tools/utils';
import { filterForCommands } from '../shared';

// TODO(NODE-5824): Implement CSOT prose tests
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a comment that says
'TODO(DRIVERS-2965): see modified test in unified-csot-node-specs', // Skipping for both tailable awaitData and tailable non-awaitData cursors
in client_side_operations_timeout.spec.test.ts - can we remove or update it in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

describe('CSOT spec prose tests', function () {
let internalClient: MongoClient;
let client: MongoClient;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const skippedTests = {
'maxTimeMS value in the command is less than timeoutMS':
'TODO(DRIVERS-2970): see modified test in unified-csot-node-specs',
'timeoutMS is refreshed for getMore - failure':
'TODO(DRIVERS-2965): see modified test in unified-csot-node-specs', // Skipping for both tailable awaitData and tailable non-awaitData cursors
'TODO(DRIVERS-2965): see modified test in unified-csot-node-specs',
'timeoutMS applies to full resume attempt in a next call': 'TODO(DRIVERS-3006)',
'timeoutMS is refreshed for getMore if maxAwaitTimeMS is set': 'TODO(DRIVERS-3018)'
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import {
import { measureDuration, sleep } from '../../tools/utils';
import { createTimerSandbox } from '../../unit/timer_sandbox';

// TODO(NODE-5824): Implement CSOT prose tests
describe('CSOT spec unit tests', function () {
let client: MongoClient;

Expand Down Expand Up @@ -107,7 +106,7 @@ describe('CSOT spec unit tests', function () {
() => {}
);
}).skipReason =
'TODO(NODE-5682): Add CSOT support for socket read/write at the connection layer for CRUD APIs';
'TODO(NODE-6518): Add CSOT support for socket read/write at the connection layer for CRUD APIs';

describe('Client side encryption', function () {
describe('KMS requests', function () {
Expand Down
9 changes: 7 additions & 2 deletions test/unit/operations/get_more.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,12 @@ describe('GetMoreOperation', function () {
new ServerDescription('a:1'),
{} as any
);
const opts = { ...options, documentsReturnedIn: 'nextBatch', returnFieldSelector: null };
const opts = {
...options,
documentsReturnedIn: 'nextBatch',
returnFieldSelector: null,
timeoutContext: undefined
};
const operation = new GetMoreOperation(namespace, cursorId, server, opts);
const stub = sinon.stub(server, 'command').resolves({});

Expand All @@ -69,7 +74,7 @@ describe('GetMoreOperation', function () {
const call = stub.getCall(0);
expect(call.args[0]).to.equal(namespace);
expect(call.args[1]).to.deep.equal(expectedGetMoreCommand);
expect(call.args[2]).to.containSubset(opts);
expect(call.args[2]).to.deep.equal(opts);
});
});

Expand Down