Skip to content

Commit

Permalink
chore(NODE-6493): CSOT clean ups and sync runCursorCommand test (#4309)
Browse files Browse the repository at this point in the history
  • Loading branch information
nbbeeken authored and dariakp committed Nov 6, 2024
1 parent 7d548a8 commit d29210e
Show file tree
Hide file tree
Showing 25 changed files with 48 additions and 137 deletions.
2 changes: 1 addition & 1 deletion etc/notes/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ This class should **never** be directly instantiated.

### `MongoOperationTimeoutError`

- TODO(NODE-5688): Add MongoOperationTimeoutError documentation
- TODO(NODE-6491): Add MongoOperationTimeoutError documentation

### MongoUnexpectedServerResponseError

Expand Down
2 changes: 2 additions & 0 deletions src/change_stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,8 @@ export type ChangeStreamEvents<
/**
* @remarks Note that the `close` event is currently emitted whenever the internal `ChangeStreamCursor`
* instance is closed, which can occur multiple times for a given `ChangeStream` instance.
*
* TODO(NODE-6434): address this issue in NODE-6434
*/
close(): void;
};
Expand Down
4 changes: 2 additions & 2 deletions src/connection_string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1092,7 +1092,7 @@ export const OPTIONS = {
type: 'string'
},
socketTimeoutMS: {
deprecated: 'Please use timeoutMS instead',
// TODO(NODE-6491): deprecated: 'Please use timeoutMS instead',
default: 0,
type: 'uint'
},
Expand Down Expand Up @@ -1163,7 +1163,7 @@ export const OPTIONS = {
}
},
waitQueueTimeoutMS: {
deprecated: 'Please use timeoutMS instead',
// TODO(NODE-6491): deprecated: 'Please use timeoutMS instead',
default: 0,
type: 'uint'
},
Expand Down
2 changes: 0 additions & 2 deletions src/cursor/abstract_cursor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ export interface AbstractCursorOptions extends BSONSerializeOptions {
/**
* When applicable `maxTimeMS` controls the amount of time the initial command
* that constructs a cursor should take. (ex. find, aggregate, listCollections)
* @deprecated Will be removed in the next major version. Please use timeoutMS instead.
*/
maxTimeMS?: number;
/**
Expand Down Expand Up @@ -775,7 +774,6 @@ export abstract class AbstractCursor<
* Set a maxTimeMS on the cursor query, allowing for hard timeout limits on queries (Only supported on MongoDB 2.6 or higher)
*
* @param value - Number of milliseconds to wait before aborting the query.
* @deprecated Will be removed in the next major version. Please use the timeoutMS option instead.
*/
maxTimeMS(value: number): this {
this.throwIfInitialized();
Expand Down
2 changes: 0 additions & 2 deletions src/cursor/run_command_cursor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ export class RunCommandCursor extends AbstractCursor {
/**
* Controls the `getMore.maxTimeMS` field. Only valid when cursor is tailable await
* @param maxTimeMS - the number of milliseconds to wait for new data
* @deprecated Will be removed in the next major version. Please use timeoutMS instead.
*/
public setMaxTimeMS(maxTimeMS: number): this {
this.getMoreOptions.maxAwaitTimeMS = maxTimeMS;
Expand Down Expand Up @@ -118,7 +117,6 @@ export class RunCommandCursor extends AbstractCursor {

/**
* Unsupported for RunCommandCursor: maxTimeMS must be configured directly on command document
* @deprecated Will be removed in the next major version.
*/
public override maxTimeMS(_: number): never {
throw new MongoAPIError(
Expand Down
3 changes: 0 additions & 3 deletions src/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,6 @@ function isAggregateError(e: unknown): e is Error & { errors: Error[] } {
* mongodb-client-encryption has a dependency on this error, it uses the constructor with a string argument
*/
export class MongoError extends Error {
get [Symbol.toStringTag]() {
return this.name;
}
/** @internal */
[kErrorLabels]: Set<string>;
/**
Expand Down
10 changes: 2 additions & 8 deletions src/mongo_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,7 @@ export interface MongoClientOptions extends BSONSerializeOptions, SupportedNodeC
tlsInsecure?: boolean;
/** The time in milliseconds to attempt a connection before timing out. */
connectTimeoutMS?: number;
/**
* The time in milliseconds to attempt a send or receive on a socket before the attempt times out.
* @deprecated Will be removed in the next major version. Please use timeoutMS instead
*/
/** The time in milliseconds to attempt a send or receive on a socket before the attempt times out. */
socketTimeoutMS?: number;
/** An array or comma-delimited string of compressors to enable network compression for communication between this client and a mongod/mongos instance. */
compressors?: CompressorName[] | string;
Expand All @@ -182,10 +179,7 @@ export interface MongoClientOptions extends BSONSerializeOptions, SupportedNodeC
maxConnecting?: number;
/** The maximum number of milliseconds that a connection can remain idle in the pool before being removed and closed. */
maxIdleTimeMS?: number;
/**
* The maximum time in milliseconds that a thread can wait for a connection to become available.
* @deprecated Will be removed in the next major version. Please use timeoutMS instead
*/
/** The maximum time in milliseconds that a thread can wait for a connection to become available. */
waitQueueTimeoutMS?: number;
/** Specify a read concern for the collection (only MongoDB 3.2 or higher supported) */
readConcern?: ReadConcernLike;
Expand Down
1 change: 0 additions & 1 deletion src/operations/aggregate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ export interface AggregateOptions extends Omit<CommandOperationOptions, 'explain
cursor?: Document;
/**
* Specifies a cumulative time limit in milliseconds for processing operations on the cursor. MongoDB interrupts the operation at the earliest following interrupt point.
* @deprecated Will be removed in the next major version. Please use timeoutMS instead.
*/
maxTimeMS?: number;
/** The maximum amount of time for the server to wait on new documents to satisfy a tailable cursor query. */
Expand Down
1 change: 0 additions & 1 deletion src/operations/command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ export interface CommandOperationOptions
collation?: CollationOptions;
/**
* maxTimeMS is a server-side time limit in milliseconds for processing an operation.
* @deprecated Will be removed in the next major version. Please use timeoutMS instead.
*/
maxTimeMS?: number;
/**
Expand Down
1 change: 0 additions & 1 deletion src/operations/count.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ export interface CountOptions extends CommandOperationOptions {
limit?: number;
/**
* Number of milliseconds to wait before aborting the query.
* @deprecated Will be removed in the next major version. Please use timeoutMS instead.
*/
maxTimeMS?: number;
/** An index name hint for the query. */
Expand Down
1 change: 0 additions & 1 deletion src/operations/estimated_document_count.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ export interface EstimatedDocumentCountOptions extends CommandOperationOptions {
* The maximum amount of time to allow the operation to run.
*
* This option is sent only if the caller explicitly provides a value. The default is to not send a value.
* @deprecated Will be removed in the next major version. Please use timeoutMS instead.
*/
maxTimeMS?: number;
}
Expand Down
1 change: 0 additions & 1 deletion src/sdam/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,6 @@ export class Server extends TypedEventEmitter<ServerEvents> {
operationError.code === MONGODB_ERROR_CODES.Reauthenticate
) {
await this.pool.reauthenticate(conn);
// TODO(NODE-5682): Implement CSOT support for socket read/write at the connection layer
try {
const res = await conn.command(ns, cmd, finalOptions, responseType);
throwIfWriteConcernError(res);
Expand Down
2 changes: 1 addition & 1 deletion src/sdam/topology.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ export interface SelectServerOptions {
previousServer?: ServerDescription;
/**
* @internal
* TODO(NODE-5685): Make this required
* TODO(NODE-6496): Make this required by making ChangeStream use LegacyTimeoutContext
* */
timeoutContext?: TimeoutContext;
}
Expand Down
15 changes: 0 additions & 15 deletions src/timeout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,6 @@ type Reject = Parameters<ConstructorParameters<typeof Promise<never>>[0]>[1];
* if interacted with exclusively through its public API
* */
export class Timeout extends Promise<never> {
get [Symbol.toStringTag](): 'MongoDBTimeout' {
return 'MongoDBTimeout';
}

private id?: NodeJS.Timeout;

public readonly start: number;
Expand Down Expand Up @@ -112,17 +108,6 @@ export class Timeout extends Promise<never> {
return new Timeout(undefined, { duration, unref });
}

static is(timeout: unknown): timeout is Timeout {
return (
typeof timeout === 'object' &&
timeout != null &&
Symbol.toStringTag in timeout &&
timeout[Symbol.toStringTag] === 'MongoDBTimeout' &&
'then' in timeout &&
typeof timeout.then === 'function'
);
}

static override reject(rejection?: Error): Timeout {
return new Timeout(undefined, { duration: 0, unref: true, rejection });
}
Expand Down
5 changes: 1 addition & 4 deletions src/transactions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,7 @@ export interface TransactionOptions extends Omit<CommandOperationOptions, 'timeo
writeConcern?: WriteConcern;
/** A default read preference for commands in this transaction */
readPreference?: ReadPreferenceLike;
/**
* Specifies the maximum amount of time to allow a commit action on a transaction to run in milliseconds
* @deprecated This option is deprecated in favor of `timeoutMS` or `defaultTimeoutMS`.
*/
/** Specifies the maximum amount of time to allow a commit action on a transaction to run in milliseconds */
maxCommitTimeMS?: number;
}

Expand Down
8 changes: 3 additions & 5 deletions src/write_concern.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ export interface WriteConcernOptions {
export interface WriteConcernSettings {
/** The write concern */
w?: W;
/** The write concern timeout
* @deprecated Will be removed in the next major version. Please use timeoutMS */
/**
* The write concern timeout.
*/
wtimeoutMS?: number;
/** The journal write concern */
journal?: boolean;
Expand All @@ -29,8 +30,6 @@ export interface WriteConcernSettings {
j?: boolean;
/**
* The write concern timeout.
* @deprecated
* Will be removed in the next major version. Please use the wtimeoutMS option.
*/
wtimeout?: number;
/**
Expand Down Expand Up @@ -69,7 +68,6 @@ export class WriteConcern {
readonly journal?: boolean;
/**
* Specify a time limit to prevent write operations from blocking indefinitely.
* @deprecated Will be removed in the next major version. Please use timeoutMS
*/
readonly wtimeoutMS?: number;
/**
Expand Down
4 changes: 1 addition & 3 deletions test/benchmarks/driverBench/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@ function loadSpecString(filePath) {
}

function makeClient() {
this.client = new MongoClient(process.env.MONGODB_URI || 'mongodb://127.0.0.1:27017', {
timeoutMS: 0
});
this.client = new MongoClient(process.env.MONGODB_URI || 'mongodb://127.0.0.1:27017');
}

function connectClient() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,7 @@ describe('CSOT spec prose tests', function () {
});
});

/** TODO(DRIVERS-2884): Drivers should not interrupt creating connections with a client-side timeout */
context.skip('4. Background Connection Pooling', () => {
/**
* The tests in this section MUST only be run if the server version is 4.4 or higher and the URI has authentication
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,11 @@ import { runUnifiedSuite } from '../../tools/unified-spec-runner/runner';
const skippedSpecs = {};

const skippedTests = {
'timeoutMS can be configured on a MongoClient - createChangeStream on client': 'TODO(NODE-6305)',
'timeoutMS applies to whole operation, not individual attempts - createChangeStream on client':
'TODO(NODE-6305)',
'Tailable cursor iteration timeoutMS is refreshed for getMore - failure': 'TODO(NODE-6305)',
'Tailable cursor iteration timeoutMS is refreshed for getMore - failure': 'TODO(DRIVERS-2965)',
'Tailable cursor awaitData iteration timeoutMS is refreshed for getMore - failure':
'TODO(NODE-6305)',
'command is not sent if RTT is greater than timeoutMS': 'TODO(DRIVERS-2965)',
'Non=tailable cursor iteration timeoutMS is refreshed for getMore if timeoutMode is iteration - failure':
'TODO(DRIVERS-2965)',
'Non-tailable cursor lifetime remaining timeoutMS applied to getMore if timeoutMode is unset':
'command is not sent if RTT is greater than timeoutMS': 'TODO(DRIVERS-2965)',
'Non-tailable cursor iteration timeoutMS is refreshed for getMore if timeoutMode is iteration - failure':
'TODO(DRIVERS-2965)',
'maxTimeMS value in the command is less than timeoutMS':
'TODO(DRIVERS-2970): see modified test in unified-csot-node-specs',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@
},
"collection": "collection",
"maxTimeMS": {
"$$exists": true
"$$exists": false
}
}
}
Expand All @@ -210,7 +210,7 @@
]
},
{
"description": "Non=tailable cursor iteration timeoutMS is refreshed for getMore if timeoutMode is iteration - failure",
"description": "Non-tailable cursor iteration timeoutMS is refreshed for getMore if timeoutMode is iteration - failure",
"runOnRequirements": [
{
"serverless": "forbid"
Expand Down
11 changes: 6 additions & 5 deletions test/spec/client-side-operations-timeout/runCursorCommand.yml
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ tests:
runOnRequirements:
- serverless: forbid
operations:
# Block find/getMore for 15ms.
# Block find/getMore for 60ms.
- name: failPoint
object: testRunner
arguments:
Expand All @@ -83,8 +83,9 @@ tests:
blockConnection: true
blockTimeMS: 60
# Run a find with timeoutMS less than double our failPoint blockTimeMS and
# batchSize less than the total document count will cause a find and a getMore to be sent.
# Both will block for 60ms so together they will go over the timeout.
# batchSize less than the total document count will cause a find and a
# getMore to be sent. Both will block for 60ms so together they will go
# over the timeout.
- name: runCursorCommand
object: *db
arguments:
Expand All @@ -106,12 +107,12 @@ tests:
command:
getMore: { $$type: [int, long] }
collection: *collection
maxTimeMS: { $$exists: true }
maxTimeMS: { $$exists: false }

# If timeoutMode=ITERATION, timeoutMS applies separately to the initial find and the getMore on the cursor. Neither
# command should have a maxTimeMS field. This is a failure test. The "find" inherits timeoutMS=100 and "getMore"
# commands are blocked for 60ms, causing iteration to fail with a timeout error.
- description: Non=tailable cursor iteration timeoutMS is refreshed for getMore if timeoutMode is iteration - failure
- description: Non-tailable cursor iteration timeoutMS is refreshed for getMore if timeoutMode is iteration - failure
runOnRequirements:
- serverless: forbid
operations:
Expand Down
22 changes: 11 additions & 11 deletions test/types/mongodb.test-d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,38 +20,38 @@ declare const options: MongoDBDriver.MongoClientOptions;
expectDeprecated(options.w);
expectDeprecated(options.journal);
expectDeprecated(options.wtimeoutMS);
expectDeprecated(options.socketTimeoutMS);
expectDeprecated(options.waitQueueTimeoutMS);
// TODO(NODE-6491): expectDeprecated(options.socketTimeoutMS);
// TODO(NODE-6491): expectDeprecated(options.waitQueueTimeoutMS);
expectNotDeprecated(options.writeConcern);
expectNotDeprecated(options.serverSelectionTimeoutMS);
expectNotDeprecated(options.connectTimeoutMS);

expectType<WriteConcernSettings | WriteConcern | undefined>(options.writeConcern);

declare const estimatedDocumentCountOptions: MongoDBDriver.EstimatedDocumentCountOptions;
expectDeprecated(estimatedDocumentCountOptions.maxTimeMS);
// TODO(NODE-6491): expectDeprecated(estimatedDocumentCountOptions.maxTimeMS);

declare const countOptions: MongoDBDriver.CountOptions;
expectDeprecated(countOptions.maxTimeMS);
// TODO(NODE-6491): expectDeprecated(countOptions.maxTimeMS);

declare const commandOptions: MongoDBDriver.CommandOperationOptions;
expectDeprecated(commandOptions.maxTimeMS);
// TODO(NODE-6491): expectDeprecated(commandOptions.maxTimeMS);

declare const aggregateOptions: MongoDBDriver.AggregateOptions;
expectDeprecated(aggregateOptions.maxTimeMS);
// TODO(NODE-6491): expectDeprecated(aggregateOptions.maxTimeMS);

declare const runCommandCursor: MongoDBDriver.RunCommandCursor;
expectDeprecated(runCommandCursor.setMaxTimeMS);
expectDeprecated(runCommandCursor.maxTimeMS);
// TODO(NODE-6491): expectDeprecated(runCommandCursor.setMaxTimeMS);
// TODO(NODE-6491): expectDeprecated(runCommandCursor.maxTimeMS);

declare const cursorOptions: MongoDBDriver.AbstractCursorOptions;
expectDeprecated(cursorOptions.maxTimeMS);
// TODO(NODE-6491): expectDeprecated(cursorOptions.maxTimeMS);

declare const abstractCursor: MongoDBDriver.AbstractCursor;
expectDeprecated(abstractCursor.maxTimeMS);
// TODO(NODE-6491): expectDeprecated(abstractCursor.maxTimeMS);

declare const txnOptions: MongoDBDriver.TransactionOptions;
expectDeprecated(txnOptions.maxCommitTimeMS);
// TODO(NODE-6491): expectDeprecated(txnOptions.maxCommitTimeMS);

interface TSchema extends Document {
name: string;
Expand Down
2 changes: 1 addition & 1 deletion test/types/write_concern.test-d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@ expectNotAssignable<ListIndexesOptions>({ writeConcern: { w: 0 } });
expectNotAssignable<ChangeStreamOptions>({ writeConcern: { w: 0 } });

declare const wc: WriteConcern;
expectDeprecated(wc.wtimeoutMS);
// TODO(NODE-6491): expectDeprecated(wc.wtimeoutMS);
expectDeprecated(wc.wtimeout);
Loading

0 comments on commit d29210e

Please sign in to comment.