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-6446): deprecate legacy timeout options #4279

Merged
merged 13 commits into from
Oct 25, 2024
2 changes: 2 additions & 0 deletions src/connection_string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1092,6 +1092,7 @@ export const OPTIONS = {
type: 'string'
},
socketTimeoutMS: {
deprecated: 'Please use timeoutMS instead',
default: 0,
type: 'uint'
},
Expand Down Expand Up @@ -1162,6 +1163,7 @@ export const OPTIONS = {
}
},
waitQueueTimeoutMS: {
deprecated: 'Please use timeoutMS instead',
default: 0,
type: 'uint'
},
Expand Down
4 changes: 4 additions & 0 deletions src/cursor/abstract_cursor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ 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 @@ -721,6 +723,8 @@ 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
14 changes: 8 additions & 6 deletions src/cursor/aggregation_cursor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,14 @@ export class AggregationCursor<TSchema = any> extends ExplainableCursor<TSchema>
...this.cursorOptions,
session
};
try {
validateExplainTimeoutOptions(options, Explain.fromOptions(options));
} catch {
throw new MongoAPIError(
'timeoutMS cannot be used with explain when explain is specified in aggregateOptions'
);
if (options.explain) {
try {
validateExplainTimeoutOptions(options, Explain.fromOptions(options));
} catch {
throw new MongoAPIError(
'timeoutMS cannot be used with explain when explain is specified in aggregateOptions'
);
}
}

const aggregateOperation = new AggregateOperation(this.namespace, this.pipeline, options);
Expand Down
14 changes: 8 additions & 6 deletions src/cursor/find_cursor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,14 @@ export class FindCursor<TSchema = any> extends ExplainableCursor<TSchema> {
session
};

try {
validateExplainTimeoutOptions(options, Explain.fromOptions(options));
} catch {
throw new MongoAPIError(
'timeoutMS cannot be used with explain when explain is specified in findOptions'
);
if (options.explain) {
try {
validateExplainTimeoutOptions(options, Explain.fromOptions(options));
} catch {
throw new MongoAPIError(
'timeoutMS cannot be used with explain when explain is specified in findOptions'
);
}
}

const findOperation = new FindOperation(this.namespace, this.cursorFilter, options);
Expand Down
8 changes: 6 additions & 2 deletions src/cursor/run_command_cursor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ 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 All @@ -56,7 +58,7 @@ export class RunCommandCursor extends AbstractCursor {

/**
* Controls the `getMore.batchSize` field
* @param maxTimeMS - the number documents to return in the `nextBatch`
* @param batchSize - the number documents to return in the `nextBatch`
*/
public setBatchSize(batchSize: number): this {
this.getMoreOptions.batchSize = batchSize;
Expand All @@ -82,7 +84,9 @@ export class RunCommandCursor extends AbstractCursor {
);
}

/** Unsupported for RunCommandCursor: maxTimeMS must be configured directly on command document */
/** 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(
'maxTimeMS must be configured on the command document directly, to configure getMore.maxTimeMS use cursor.setMaxTimeMS()'
Expand Down
8 changes: 6 additions & 2 deletions src/mongo_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,9 @@ 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. */
/** 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 */
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 @@ -176,7 +178,9 @@ 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. */
/** 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 */
waitQueueTimeoutMS?: number;
/** Specify a read concern for the collection (only MongoDB 3.2 or higher supported) */
readConcern?: ReadConcernLike;
Expand Down
4 changes: 3 additions & 1 deletion src/operations/aggregate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ export interface AggregateOptions extends Omit<CommandOperationOptions, 'explain
bypassDocumentValidation?: boolean;
/** Return the query as cursor, on 2.6 \> it returns as a real cursor on pre 2.6 it returns as an emulated cursor. */
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. */
/** 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. */
maxAwaitTimeMS?: number;
Expand Down
5 changes: 4 additions & 1 deletion src/operations/command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ export interface CommandOperationOptions
readConcern?: ReadConcernLike;
/** Collation */
collation?: CollationOptions;
/**
* @deprecated
* Will be removed in the next major version. Please use timeoutMS instead. */
maxTimeMS?: number;
/**
* Comment to apply to the operation.
Expand Down Expand Up @@ -97,7 +100,7 @@ export abstract class CommandOperation<T> extends AbstractOperation<T> {

if (this.hasAspect(Aspect.EXPLAINABLE)) {
this.explain = Explain.fromOptions(options);
validateExplainTimeoutOptions(this.options, this.explain);
if (this.explain) validateExplainTimeoutOptions(this.options, this.explain);
} else if (options?.explain != null) {
throw new MongoInvalidArgumentError(`Option "explain" is not supported on this command`);
}
Expand Down
3 changes: 2 additions & 1 deletion src/operations/count.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ export interface CountOptions extends CommandOperationOptions {
skip?: number;
/** The maximum amounts to count before aborting. */
limit?: number;
/** Number of milliseconds to wait before aborting the query. */
/** 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. */
hint?: string | Document;
Expand Down
2 changes: 2 additions & 0 deletions src/operations/estimated_document_count.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ 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
59 changes: 35 additions & 24 deletions src/sessions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -487,13 +487,31 @@ export class ClientSession
maxTimeMS?: number;
} = { commitTransaction: 1 };

const timeoutMS =
typeof options?.timeoutMS === 'number'
? options.timeoutMS
: typeof this.timeoutMS === 'number'
? this.timeoutMS
: null;

const wc = this.transaction.options.writeConcern ?? this.clientOptions?.writeConcern;
if (wc != null) {
WriteConcern.apply(command, { wtimeoutMS: 10000, w: 'majority', ...wc });
if (timeoutMS == null && this.timeoutContext == null) {
WriteConcern.apply(command, { wtimeoutMS: 10000, w: 'majority', ...wc });
} else {
const wcKeys = Object.keys(wc);
if (wcKeys.length > 2 || (!wcKeys.includes('wtimeoutMS') && !wcKeys.includes('wTimeoutMS')))
// if the write concern was specified with wTimeoutMS, then we set both wtimeoutMS and wTimeoutMS, guaranteeing at least two keys, so if we have more than two keys, then we can automatically assume that we should add the write concern to the command. If it has 2 or fewer keys, we need to check that those keys aren't the wtimeoutMS or wTimeoutMS options before we add the write concern to the command
WriteConcern.apply(command, { ...wc, wtimeoutMS: undefined });
}
}

if (this.transaction.state === TxnState.TRANSACTION_COMMITTED || this.commitAttempted) {
WriteConcern.apply(command, { wtimeoutMS: 10000, ...wc, w: 'majority' });
if (timeoutMS == null && this.timeoutContext == null) {
WriteConcern.apply(command, { wtimeoutMS: 10000, ...wc, w: 'majority' });
} else {
WriteConcern.apply(command, { w: 'majority', ...wc, wtimeoutMS: undefined });
}
}

if (typeof this.transaction.options.maxTimeMS === 'number') {
Expand All @@ -510,13 +528,6 @@ export class ClientSession
bypassPinningCheck: true
});

const timeoutMS =
typeof options?.timeoutMS === 'number'
? options.timeoutMS
: typeof this.timeoutMS === 'number'
? this.timeoutMS
: null;

const timeoutContext =
this.timeoutContext ??
(typeof timeoutMS === 'number'
Expand Down Expand Up @@ -616,21 +627,6 @@ export class ClientSession
recoveryToken?: Document;
} = { abortTransaction: 1 };

const wc = this.transaction.options.writeConcern ?? this.clientOptions?.writeConcern;
if (wc != null) {
WriteConcern.apply(command, { wtimeoutMS: 10000, w: 'majority', ...wc });
}

if (this.transaction.recoveryToken) {
command.recoveryToken = this.transaction.recoveryToken;
}

const operation = new RunAdminCommandOperation(command, {
session: this,
readPreference: ReadPreference.primary,
bypassPinningCheck: true
});

const timeoutMS =
typeof options?.timeoutMS === 'number'
? options.timeoutMS
Expand All @@ -649,6 +645,21 @@ export class ClientSession
})
: null;

const wc = this.transaction.options.writeConcern ?? this.clientOptions?.writeConcern;
if (wc != null && timeoutMS == null) {
WriteConcern.apply(command, { wtimeoutMS: 10000, w: 'majority', ...wc });
}

if (this.transaction.recoveryToken) {
command.recoveryToken = this.transaction.recoveryToken;
}

const operation = new RunAdminCommandOperation(command, {
session: this,
readPreference: ReadPreference.primary,
bypassPinningCheck: true
});

try {
await executeOperation(this.client, operation, timeoutContext);
this.unpin();
Expand Down
15 changes: 12 additions & 3 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -516,20 +516,31 @@ export function resolveOptions<T extends CommandOperationOptions>(
): T {
const result: T = Object.assign({}, options, resolveBSONOptions(options, parent));

const timeoutMS = options?.timeoutMS ?? parent?.timeoutMS;
// Users cannot pass a readConcern/writeConcern to operations in a transaction
const session = options?.session;

if (!session?.inTransaction()) {
const readConcern = ReadConcern.fromOptions(options) ?? parent?.readConcern;
if (readConcern) {
result.readConcern = readConcern;
}

const writeConcern = WriteConcern.fromOptions(options) ?? parent?.writeConcern;
let writeConcern = WriteConcern.fromOptions(options) ?? parent?.writeConcern;
if (writeConcern) {
if (timeoutMS != null) {
writeConcern = WriteConcern.fromOptions({
...writeConcern,
wtimeout: undefined,
wtimeoutMS: undefined
});
}
result.writeConcern = writeConcern;
}
}

result.timeoutMS = timeoutMS;

const readPreference = ReadPreference.fromOptions(options) ?? parent?.readPreference;
if (readPreference) {
result.readPreference = readPreference;
Expand All @@ -542,8 +553,6 @@ export function resolveOptions<T extends CommandOperationOptions>(
);
}

result.timeoutMS = options?.timeoutMS ?? parent?.timeoutMS;

return result;
}

Expand Down
9 changes: 6 additions & 3 deletions src/write_concern.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ export interface WriteConcernOptions {
export interface WriteConcernSettings {
/** The write concern */
w?: W;
/** The write concern timeout */
/** The write concern timeout
* @deprecated Will be removed in the next major version. Please use timeoutMS */
wtimeoutMS?: number;
/** The journal write concern */
journal?: boolean;
Expand All @@ -28,7 +29,8 @@ export interface WriteConcernSettings {
j?: boolean;
/**
* The write concern timeout.
* @deprecated Will be removed in the next major version. Please use the wtimeoutMS option.
* @deprecated
* Will be removed in the next major version. Please use the wtimeoutMS option.
*/
wtimeout?: number;
/**
Expand Down Expand Up @@ -65,7 +67,8 @@ export class WriteConcern {
readonly w?: W;
/** Request acknowledgment that the write operation has been written to the on-disk journal */
readonly journal?: boolean;
/** Specify a time limit to prevent write operations from blocking indefinitely */
/** Specify a time limit to prevent write operations from blocking indefinitely
* @deprecated Will be removed in the next major version. Please use timeoutMS */
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use the expectDeprecated helper in tsd to assert none of these get undeprecated and it will help us find them all later. It helps if we refactor the way we declare the options but we preserve these docs and info.

readonly wtimeoutMS?: number;
/**
* Specify a time limit to prevent write operations from blocking indefinitely.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { runUnifiedSuite } from '../../tools/unified-spec-runner/runner';
const skippedSpecs = {
'change-streams': 'TODO(NODE-6035)',
'convenient-transactions': 'TODO(NODE-5687)',
'deprecated-options': 'TODO(NODE-5689)',
'tailable-awaitData': 'TODO(NODE-6035)',
'tailable-non-awaitData': 'TODO(NODE-6035)'
};
Expand Down
24 changes: 24 additions & 0 deletions test/types/mongodb.test-d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,33 @@ declare const options: MongoDBDriver.MongoClientOptions;
expectDeprecated(options.w);
expectDeprecated(options.journal);
expectDeprecated(options.wtimeoutMS);
expectDeprecated(options.socketTimeoutMS);
expectDeprecated(options.waitQueueTimeoutMS);
nbbeeken marked this conversation as resolved.
Show resolved Hide resolved
expectNotDeprecated(options.writeConcern);
expectType<WriteConcernSettings | WriteConcern | undefined>(options.writeConcern);

declare const estimatedDocumentCountOptions: MongoDBDriver.EstimatedDocumentCountOptions;
expectDeprecated(estimatedDocumentCountOptions.maxTimeMS);

declare const countOptions: MongoDBDriver.CountOptions;
expectDeprecated(countOptions.maxTimeMS);

declare const commandOptions: MongoDBDriver.CommandOperationOptions;
expectDeprecated(commandOptions.maxTimeMS);

declare const aggregateOptions: MongoDBDriver.AggregateOptions;
expectDeprecated(aggregateOptions.maxTimeMS);

declare const runCommandCursor: MongoDBDriver.RunCommandCursor;
expectDeprecated(runCommandCursor.setMaxTimeMS);
expectDeprecated(runCommandCursor.maxTimeMS);

declare const cursorOptions: MongoDBDriver.AbstractCursorOptions;
expectDeprecated(cursorOptions.maxTimeMS);

declare const abstractCursor: MongoDBDriver.AbstractCursor;
expectDeprecated(abstractCursor.maxTimeMS);

interface TSchema extends Document {
name: string;
}
Expand Down
Loading