From 494dbd5f5b921abd54eeb660f6d7c14832198933 Mon Sep 17 00:00:00 2001 From: Warren James Date: Fri, 25 Oct 2024 14:18:06 -0400 Subject: [PATCH] feat(NODE-6446): deprecate legacy timeout options (#4279) Co-authored-by: Neal Beeken --- src/connection_string.ts | 2 + src/cursor/abstract_cursor.ts | 2 + src/cursor/aggregation_cursor.ts | 14 +++-- src/cursor/find_cursor.ts | 14 +++-- src/cursor/run_command_cursor.ts | 8 ++- src/mongo_client.ts | 10 +++- src/operations/aggregate.ts | 5 +- src/operations/command.ts | 6 +- src/operations/count.ts | 5 +- src/operations/estimated_document_count.ts | 1 + src/sessions.ts | 59 +++++++++++-------- src/utils.ts | 15 ++++- src/write_concern.ts | 11 +++- ...lient_side_operations_timeout.spec.test.ts | 1 - test/types/mongodb.test-d.ts | 30 ++++++++++ test/types/write_concern.test-d.ts | 9 ++- 16 files changed, 140 insertions(+), 52 deletions(-) diff --git a/src/connection_string.ts b/src/connection_string.ts index f0b497ddf40..3aae2d0a654 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -1092,6 +1092,7 @@ export const OPTIONS = { type: 'string' }, socketTimeoutMS: { + deprecated: 'Please use timeoutMS instead', default: 0, type: 'uint' }, @@ -1162,6 +1163,7 @@ export const OPTIONS = { } }, waitQueueTimeoutMS: { + deprecated: 'Please use timeoutMS instead', default: 0, type: 'uint' }, diff --git a/src/cursor/abstract_cursor.ts b/src/cursor/abstract_cursor.ts index 96d28d05584..dd3c40bfab6 100644 --- a/src/cursor/abstract_cursor.ts +++ b/src/cursor/abstract_cursor.ts @@ -84,6 +84,7 @@ 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; /** @@ -721,6 +722,7 @@ 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(); diff --git a/src/cursor/aggregation_cursor.ts b/src/cursor/aggregation_cursor.ts index db7bd20b5fa..cace0a4b6a2 100644 --- a/src/cursor/aggregation_cursor.ts +++ b/src/cursor/aggregation_cursor.ts @@ -75,12 +75,14 @@ export class AggregationCursor extends ExplainableCursor ...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); diff --git a/src/cursor/find_cursor.ts b/src/cursor/find_cursor.ts index 469c27628a5..28cb373614d 100644 --- a/src/cursor/find_cursor.ts +++ b/src/cursor/find_cursor.ts @@ -75,12 +75,14 @@ export class FindCursor extends ExplainableCursor { 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); diff --git a/src/cursor/run_command_cursor.ts b/src/cursor/run_command_cursor.ts index 90e4a94fd42..15f95042c7f 100644 --- a/src/cursor/run_command_cursor.ts +++ b/src/cursor/run_command_cursor.ts @@ -48,6 +48,7 @@ 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; @@ -56,7 +57,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; @@ -82,7 +83,10 @@ 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()' diff --git a/src/mongo_client.ts b/src/mongo_client.ts index 91dd9d3d38a..e6f64bae217 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -152,7 +152,10 @@ 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; @@ -176,7 +179,10 @@ 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; diff --git a/src/operations/aggregate.ts b/src/operations/aggregate.ts index 0e9fbb0b846..3407e64cebb 100644 --- a/src/operations/aggregate.ts +++ b/src/operations/aggregate.ts @@ -26,7 +26,10 @@ export interface AggregateOptions extends Omit 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; diff --git a/src/operations/command.ts b/src/operations/command.ts index bcd3919017b..13412e7cd70 100644 --- a/src/operations/command.ts +++ b/src/operations/command.ts @@ -40,6 +40,10 @@ export interface CommandOperationOptions readConcern?: ReadConcernLike; /** Collation */ 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; /** * Comment to apply to the operation. @@ -97,7 +101,7 @@ export abstract class CommandOperation extends AbstractOperation { 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`); } diff --git a/src/operations/count.ts b/src/operations/count.ts index 82330a11e76..1f8f96aef27 100644 --- a/src/operations/count.ts +++ b/src/operations/count.ts @@ -13,7 +13,10 @@ 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; diff --git a/src/operations/estimated_document_count.ts b/src/operations/estimated_document_count.ts index 5ab5aa4c305..68df4b002e2 100644 --- a/src/operations/estimated_document_count.ts +++ b/src/operations/estimated_document_count.ts @@ -12,6 +12,7 @@ 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; } diff --git a/src/sessions.ts b/src/sessions.ts index 434abc83ef5..9ada6124d5a 100644 --- a/src/sessions.ts +++ b/src/sessions.ts @@ -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') { @@ -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' @@ -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 @@ -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(); diff --git a/src/utils.ts b/src/utils.ts index c70f682c761..c23161612a8 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -516,20 +516,31 @@ export function resolveOptions( ): 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; @@ -542,8 +553,6 @@ export function resolveOptions( ); } - result.timeoutMS = options?.timeoutMS ?? parent?.timeoutMS; - return result; } diff --git a/src/write_concern.ts b/src/write_concern.ts index 390646a3be0..bf88aa6a74f 100644 --- a/src/write_concern.ts +++ b/src/write_concern.ts @@ -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; @@ -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; /** @@ -65,7 +67,10 @@ 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 + */ readonly wtimeoutMS?: number; /** * Specify a time limit to prevent write operations from blocking indefinitely. diff --git a/test/integration/client-side-operations-timeout/client_side_operations_timeout.spec.test.ts b/test/integration/client-side-operations-timeout/client_side_operations_timeout.spec.test.ts index c519da8039f..6708d7da89f 100644 --- a/test/integration/client-side-operations-timeout/client_side_operations_timeout.spec.test.ts +++ b/test/integration/client-side-operations-timeout/client_side_operations_timeout.spec.test.ts @@ -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)' }; diff --git a/test/types/mongodb.test-d.ts b/test/types/mongodb.test-d.ts index 892235f4747..c87933514f2 100644 --- a/test/types/mongodb.test-d.ts +++ b/test/types/mongodb.test-d.ts @@ -20,9 +20,39 @@ declare const options: MongoDBDriver.MongoClientOptions; expectDeprecated(options.w); expectDeprecated(options.journal); expectDeprecated(options.wtimeoutMS); +expectDeprecated(options.socketTimeoutMS); +expectDeprecated(options.waitQueueTimeoutMS); expectNotDeprecated(options.writeConcern); +expectNotDeprecated(options.serverSelectionTimeoutMS); +expectNotDeprecated(options.connectTimeoutMS); + expectType(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); + +declare const txnOptions: MongoDBDriver.TransactionOptions; +expectDeprecated(txnOptions.maxCommitTimeMS); + interface TSchema extends Document { name: string; } diff --git a/test/types/write_concern.test-d.ts b/test/types/write_concern.test-d.ts index b4249de86c8..fefcaf4fc84 100644 --- a/test/types/write_concern.test-d.ts +++ b/test/types/write_concern.test-d.ts @@ -1,13 +1,18 @@ -import { expectNotAssignable } from 'tsd'; +import { expectDeprecated, expectNotAssignable } from 'tsd'; import type { ChangeStreamOptions, FindOptions, ListCollectionsOptions, - ListIndexesOptions + ListIndexesOptions, + WriteConcern } from '../mongodb'; expectNotAssignable({ writeConcern: { w: 0 } }); expectNotAssignable({ writeConcern: { w: 0 } }); expectNotAssignable({ writeConcern: { w: 0 } }); expectNotAssignable({ writeConcern: { w: 0 } }); + +declare const wc: WriteConcern; +expectDeprecated(wc.wtimeoutMS); +expectDeprecated(wc.wtimeout);