From 9800987da45e922cb727dcf1e6d531c0fc46d2f0 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Thu, 7 Nov 2024 10:56:05 -0700 Subject: [PATCH 1/3] remove dead code --- src/cmap/connection_pool.ts | 2 +- src/cursor/abstract_cursor.ts | 3 --- src/timeout.ts | 6 ------ 3 files changed, 1 insertion(+), 10 deletions(-) diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index 2cd2bcc2c19..83449a91ef9 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -402,7 +402,7 @@ export class ConnectionPool extends TypedEventEmitter { } throw error; } finally { - if (options.timeoutContext.clearConnectionCheckoutTimeout) timeout?.clear(); + timeout?.clear(); } } diff --git a/src/cursor/abstract_cursor.ts b/src/cursor/abstract_cursor.ts index 66bfbed0078..a6f501bd39f 100644 --- a/src/cursor/abstract_cursor.ts +++ b/src/cursor/abstract_cursor.ts @@ -1169,9 +1169,6 @@ export class CursorTimeoutContext extends TimeoutContext { override get clearServerSelectionTimeout(): boolean { return this.timeoutContext.clearServerSelectionTimeout; } - override get clearConnectionCheckoutTimeout(): boolean { - return this.timeoutContext.clearConnectionCheckoutTimeout; - } override get timeoutForSocketWrite(): Timeout | null { return this.timeoutContext.timeoutForSocketWrite; } diff --git a/src/timeout.ts b/src/timeout.ts index 47c27c7b90e..d1105c40ab5 100644 --- a/src/timeout.ts +++ b/src/timeout.ts @@ -171,8 +171,6 @@ export abstract class TimeoutContext { abstract get clearServerSelectionTimeout(): boolean; - abstract get clearConnectionCheckoutTimeout(): boolean; - abstract get timeoutForSocketWrite(): Timeout | null; abstract get timeoutForSocketRead(): Timeout | null; @@ -193,7 +191,6 @@ export class CSOTTimeoutContext extends TimeoutContext { serverSelectionTimeoutMS: number; socketTimeoutMS?: number; - clearConnectionCheckoutTimeout: boolean; clearServerSelectionTimeout: boolean; private _serverSelectionTimeout?: Timeout | null; @@ -212,7 +209,6 @@ export class CSOTTimeoutContext extends TimeoutContext { this.socketTimeoutMS = options.socketTimeoutMS; this.clearServerSelectionTimeout = false; - this.clearConnectionCheckoutTimeout = true; } get maxTimeMS(): number { @@ -331,13 +327,11 @@ export class CSOTTimeoutContext extends TimeoutContext { export class LegacyTimeoutContext extends TimeoutContext { options: LegacyTimeoutContextOptions; clearServerSelectionTimeout: boolean; - clearConnectionCheckoutTimeout: boolean; constructor(options: LegacyTimeoutContextOptions) { super(); this.options = options; this.clearServerSelectionTimeout = true; - this.clearConnectionCheckoutTimeout = true; } csotEnabled(): this is CSOTTimeoutContext { From 76f04c126165862011b60bff6875a1c33094430d Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Thu, 7 Nov 2024 11:16:45 -0700 Subject: [PATCH 2/3] Consolidate logic --- src/cmap/connection.ts | 23 +++++++---------------- src/cursor/abstract_cursor.ts | 8 ++++++-- src/timeout.ts | 23 +++++++++++++++++++++++ 3 files changed, 36 insertions(+), 18 deletions(-) diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index ca7c86a0bad..b92adb50511 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -427,10 +427,7 @@ export class Connection extends TypedEventEmitter { ...options }; - if (!options.omitMaxTimeMS) { - const maxTimeMS = options.timeoutContext?.maxTimeMS; - if (maxTimeMS && maxTimeMS > 0 && Number.isFinite(maxTimeMS)) cmd.maxTimeMS = maxTimeMS; - } + options.timeoutContext?.addMaxTimeMSToCommand(cmd, options); const message = this.supportsOpMsg ? new OpMsgRequest(db, cmd, commandOptions) @@ -446,13 +443,11 @@ export class Connection extends TypedEventEmitter { ): AsyncGenerator { this.throwIfAborted(); - if (options.timeoutContext?.csotEnabled()) { - this.socket.setTimeout(0); - } else if (typeof options.socketTimeoutMS === 'number') { - this.socket.setTimeout(options.socketTimeoutMS); - } else if (this.socketTimeoutMS !== 0) { - this.socket.setTimeout(this.socketTimeoutMS); - } + const timeout = + options.socketTimeoutMS ?? + options?.timeoutContext?.getSocketTimeoutMS() ?? + this.socketTimeoutMS; + this.socket.setTimeout(timeout); try { await this.writeCommand(message, { @@ -487,11 +482,7 @@ export class Connection extends TypedEventEmitter { yield document; this.throwIfAborted(); - if (typeof options.socketTimeoutMS === 'number') { - this.socket.setTimeout(options.socketTimeoutMS); - } else if (this.socketTimeoutMS !== 0) { - this.socket.setTimeout(this.socketTimeoutMS); - } + this.socket.setTimeout(timeout); } } finally { this.socket.setTimeout(0); diff --git a/src/cursor/abstract_cursor.ts b/src/cursor/abstract_cursor.ts index a6f501bd39f..8eccdfcf630 100644 --- a/src/cursor/abstract_cursor.ts +++ b/src/cursor/abstract_cursor.ts @@ -1187,12 +1187,16 @@ export class CursorTimeoutContext extends TimeoutContext { override get maxTimeMS(): number | null { return this.timeoutContext.maxTimeMS; } - get timeoutMS(): number | null { return this.timeoutContext.csotEnabled() ? this.timeoutContext.timeoutMS : null; } - override refreshed(): CursorTimeoutContext { return new CursorTimeoutContext(this.timeoutContext.refreshed(), this.owner); } + override addMaxTimeMSToCommand(command: Document, options: { omitMaxTimeMS?: boolean }): void { + this.timeoutContext.addMaxTimeMSToCommand(command, options); + } + override getSocketTimeoutMS(): number | undefined { + return this.timeoutContext.getSocketTimeoutMS(); + } } diff --git a/src/timeout.ts b/src/timeout.ts index d1105c40ab5..abbc7b554c8 100644 --- a/src/timeout.ts +++ b/src/timeout.ts @@ -1,3 +1,4 @@ +import { type Document } from 'bson'; import { clearTimeout, setTimeout } from 'timers'; import { MongoInvalidArgumentError, MongoOperationTimeoutError, MongoRuntimeError } from './error'; @@ -183,6 +184,10 @@ export abstract class TimeoutContext { /** Returns a new instance of the TimeoutContext, with all timeouts refreshed and restarted. */ abstract refreshed(): TimeoutContext; + + abstract addMaxTimeMSToCommand(command: Document, options: { omitMaxTimeMS?: boolean }): void; + + abstract getSocketTimeoutMS(): number | undefined; } /** @internal */ @@ -321,6 +326,16 @@ export class CSOTTimeoutContext extends TimeoutContext { override refreshed(): CSOTTimeoutContext { return new CSOTTimeoutContext(this); } + + override addMaxTimeMSToCommand(command: Document, options: { omitMaxTimeMS?: boolean }): void { + if (options.omitMaxTimeMS) return; + const maxTimeMS = this.remainingTimeMS - this.minRoundTripTime; + if (maxTimeMS > 0 && Number.isFinite(maxTimeMS)) command.maxTimeMS = maxTimeMS; + } + + override getSocketTimeoutMS(): number | undefined { + return 0; + } } /** @internal */ @@ -373,4 +388,12 @@ export class LegacyTimeoutContext extends TimeoutContext { override refreshed(): LegacyTimeoutContext { return new LegacyTimeoutContext(this.options); } + + override addMaxTimeMSToCommand(_command: Document, _options: { omitMaxTimeMS?: boolean }): void { + // No max timeMS is added to commands in legacy timeout mode. + } + + override getSocketTimeoutMS(): number | undefined { + return this.options.socketTimeoutMS; + } } From 68f5881c5409279807c39edc1b29994e8bced002 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Mon, 11 Nov 2024 09:07:00 -0700 Subject: [PATCH 3/3] fix lint --- src/timeout.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/timeout.ts b/src/timeout.ts index abbc7b554c8..3b1dbcb2346 100644 --- a/src/timeout.ts +++ b/src/timeout.ts @@ -1,6 +1,6 @@ -import { type Document } from 'bson'; import { clearTimeout, setTimeout } from 'timers'; +import { type Document } from './bson'; import { MongoInvalidArgumentError, MongoOperationTimeoutError, MongoRuntimeError } from './error'; import { type ClientSession } from './sessions'; import { csotMin, noop } from './utils';