From e1059541d6bba39c005e78435a8aba597d262636 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Tue, 10 Dec 2024 14:16:37 -0500 Subject: [PATCH 1/4] chore: remove all symbol properties --- src/bulk/common.ts | 15 +- src/change_stream.ts | 58 ++--- src/cmap/connection_pool.ts | 244 ++++++++---------- src/encrypter.ts | 15 +- src/error.ts | 33 +-- src/mongo_client.ts | 58 ++--- src/operations/operation.ts | 12 +- src/sdam/monitor.ts | 68 +++-- src/sdam/server.ts | 3 +- src/sdam/topology.ts | 34 ++- src/sessions.ts | 87 +++---- .../change-streams/change_stream.test.ts | 19 +- .../connection.test.ts | 7 +- test/integration/crud/misc_cursors.test.js | 7 +- test/tools/utils.ts | 12 - test/unit/cmap/connection.test.ts | 9 +- test/unit/error.test.ts | 10 +- test/unit/mongo_client.test.ts | 15 +- test/unit/sdam/topology.test.ts | 5 +- test/unit/sessions.test.ts | 52 ++-- 20 files changed, 302 insertions(+), 461 deletions(-) diff --git a/src/bulk/common.ts b/src/bulk/common.ts index 22012207a09..70df6544504 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -30,9 +30,6 @@ import { } from '../utils'; import { WriteConcern } from '../write_concern'; -/** @internal */ -const kServerError = Symbol('serverError'); - /** @public */ export const BatchType = Object.freeze({ INSERT: 1, @@ -315,29 +312,29 @@ export interface WriteConcernErrorData { */ export class WriteConcernError { /** @internal */ - [kServerError]: WriteConcernErrorData; + private serverError: WriteConcernErrorData; constructor(error: WriteConcernErrorData) { - this[kServerError] = error; + this.serverError = error; } /** Write concern error code. */ get code(): number | undefined { - return this[kServerError].code; + return this.serverError.code; } /** Write concern error message. */ get errmsg(): string | undefined { - return this[kServerError].errmsg; + return this.serverError.errmsg; } /** Write concern error info. */ get errInfo(): Document | undefined { - return this[kServerError].errInfo; + return this.serverError.errInfo; } toJSON(): WriteConcernErrorData { - return this[kServerError]; + return this.serverError; } toString(): string { diff --git a/src/change_stream.ts b/src/change_stream.ts index c7b21b7a202..44d07b621d2 100644 --- a/src/change_stream.ts +++ b/src/change_stream.ts @@ -24,13 +24,6 @@ import type { ServerSessionId } from './sessions'; import { CSOTTimeoutContext, type TimeoutContext } from './timeout'; import { filterOptions, getTopology, type MongoDBNamespace, squashError } from './utils'; -/** @internal */ -const kCursorStream = Symbol('cursorStream'); -/** @internal */ -const kClosed = Symbol('closed'); -/** @internal */ -const kMode = Symbol('mode'); - const CHANGE_STREAM_OPTIONS = [ 'resumeAfter', 'startAfter', @@ -584,14 +577,14 @@ export class ChangeStream< namespace: MongoDBNamespace; type: symbol; /** @internal */ - cursor: ChangeStreamCursor; + private cursor: ChangeStreamCursor; streamOptions?: CursorStreamOptions; /** @internal */ - [kCursorStream]?: Readable & AsyncIterable; + private cursorStream?: Readable & AsyncIterable; /** @internal */ - [kClosed]: boolean; + private isClosed: boolean; /** @internal */ - [kMode]: false | 'iterator' | 'emitter'; + private mode: false | 'iterator' | 'emitter'; /** @event */ static readonly RESPONSE = RESPONSE; @@ -668,8 +661,8 @@ export class ChangeStream< // Create contained Change Stream cursor this.cursor = this._createChangeStreamCursor(options); - this[kClosed] = false; - this[kMode] = false; + this.isClosed = false; + this.mode = false; // Listen for any `change` listeners being added to ChangeStream this.on('newListener', eventName => { @@ -680,7 +673,7 @@ export class ChangeStream< this.on('removeListener', eventName => { if (eventName === 'change' && this.listenerCount('change') === 0 && this.cursor) { - this[kCursorStream]?.removeAllListeners('data'); + this.cursorStream?.removeAllListeners('data'); } }); @@ -692,11 +685,6 @@ export class ChangeStream< } } - /** @internal */ - get cursorStream(): (Readable & AsyncIterable) | undefined { - return this[kCursorStream]; - } - /** The cached resume token that is used to resume after the most recently returned change. */ get resumeToken(): ResumeToken { return this.cursor?.resumeToken; @@ -806,7 +794,7 @@ export class ChangeStream< } async *[Symbol.asyncIterator](): AsyncGenerator { - if (this.closed) { + if (this.isClosed) { return; } @@ -826,8 +814,8 @@ export class ChangeStream< } /** Is the cursor closed */ - get closed(): boolean { - return this[kClosed] || this.cursor.closed; + public get closed(): boolean { + return this.isClosed || this.cursor.closed; } /** @@ -836,7 +824,7 @@ export class ChangeStream< async close(): Promise { this.timeoutContext?.clear(); this.timeoutContext = undefined; - this[kClosed] = true; + this.isClosed = true; const cursor = this.cursor; try { @@ -855,7 +843,7 @@ export class ChangeStream< * @throws MongoChangeStreamError if the underlying cursor or the change stream is closed */ stream(options?: CursorStreamOptions): Readable & AsyncIterable { - if (this.closed) { + if (this.isClosed) { throw new MongoChangeStreamError(CHANGESTREAM_CLOSED_ERROR); } @@ -865,24 +853,24 @@ export class ChangeStream< /** @internal */ private _setIsEmitter(): void { - if (this[kMode] === 'iterator') { + if (this.mode === 'iterator') { // TODO(NODE-3485): Replace with MongoChangeStreamModeError throw new MongoAPIError( 'ChangeStream cannot be used as an EventEmitter after being used as an iterator' ); } - this[kMode] = 'emitter'; + this.mode = 'emitter'; } /** @internal */ private _setIsIterator(): void { - if (this[kMode] === 'emitter') { + if (this.mode === 'emitter') { // TODO(NODE-3485): Replace with MongoChangeStreamModeError throw new MongoAPIError( 'ChangeStream cannot be used as an iterator after being used as an EventEmitter' ); } - this[kMode] = 'iterator'; + this.mode = 'iterator'; } /** @@ -947,8 +935,8 @@ export class ChangeStream< /** @internal */ private _streamEvents(cursor: ChangeStreamCursor): void { this._setIsEmitter(); - const stream = this[kCursorStream] ?? cursor.stream(); - this[kCursorStream] = stream; + const stream = this.cursorStream ?? cursor.stream(); + this.cursorStream = stream; stream.on('data', change => { try { const processedChange = this._processChange(change); @@ -963,18 +951,18 @@ export class ChangeStream< /** @internal */ private _endStream(): void { - const cursorStream = this[kCursorStream]; + const cursorStream = this.cursorStream; if (cursorStream) { ['data', 'close', 'end', 'error'].forEach(event => cursorStream.removeAllListeners(event)); cursorStream.destroy(); } - this[kCursorStream] = undefined; + this.cursorStream = undefined; } /** @internal */ private _processChange(change: TChange | null): TChange { - if (this[kClosed]) { + if (this.isClosed) { // TODO(NODE-3485): Replace with MongoChangeStreamClosedError throw new MongoAPIError(CHANGESTREAM_CLOSED_ERROR); } @@ -1002,7 +990,7 @@ export class ChangeStream< /** @internal */ private _processErrorStreamMode(changeStreamError: AnyError, cursorInitialized: boolean) { // If the change stream has been closed explicitly, do not process error. - if (this[kClosed]) return; + if (this.isClosed) return; if ( cursorInitialized && @@ -1034,7 +1022,7 @@ export class ChangeStream< /** @internal */ private async _processErrorIteratorMode(changeStreamError: AnyError, cursorInitialized: boolean) { - if (this[kClosed]) { + if (this.isClosed) { // TODO(NODE-3485): Replace with MongoChangeStreamClosedError throw new MongoAPIError(CHANGESTREAM_CLOSED_ERROR); } diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index ff4a727be12..97b9ff8fe72 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -52,35 +52,6 @@ import { } from './errors'; import { ConnectionPoolMetrics } from './metrics'; -/** @internal */ -const kServer = Symbol('server'); -/** @internal */ -const kConnections = Symbol('connections'); -/** @internal */ -const kPending = Symbol('pending'); -/** @internal */ -const kCheckedOut = Symbol('checkedOut'); -/** @internal */ -const kMinPoolSizeTimer = Symbol('minPoolSizeTimer'); -/** @internal */ -const kGeneration = Symbol('generation'); -/** @internal */ -const kServiceGenerations = Symbol('serviceGenerations'); -/** @internal */ -const kConnectionCounter = Symbol('connectionCounter'); -/** @internal */ -const kCancellationToken = Symbol('cancellationToken'); -/** @internal */ -const kWaitQueue = Symbol('waitQueue'); -/** @internal */ -const kCancelled = Symbol('cancelled'); -/** @internal */ -const kMetrics = Symbol('metrics'); -/** @internal */ -const kProcessingWaitQueue = Symbol('processingWaitQueue'); -/** @internal */ -const kPoolState = Symbol('poolState'); - /** @public */ export interface ConnectionPoolOptions extends Omit { /** The maximum number of connections that may be associated with a pool at a given time. This includes in use and available connections. */ @@ -103,7 +74,7 @@ export interface ConnectionPoolOptions extends Omit void; reject: (err: AnyError) => void; - [kCancelled]?: boolean; + cancelled: boolean; checkoutTime: number; } @@ -114,6 +85,8 @@ export const PoolState = Object.freeze({ closed: 'closed' } as const); +type PoolState = (typeof PoolState)[keyof typeof PoolState]; + /** * @public * @deprecated This interface is deprecated and will be removed in a future release as it is not used @@ -143,26 +116,21 @@ export type ConnectionPoolEvents = { * @internal */ export class ConnectionPool extends TypedEventEmitter { - options: Readonly; - [kPoolState]: (typeof PoolState)[keyof typeof PoolState]; - [kServer]: Server; - [kConnections]: List; - [kPending]: number; - [kCheckedOut]: Set; - [kMinPoolSizeTimer]?: NodeJS.Timeout; - /** - * An integer representing the SDAM generation of the pool - */ - [kGeneration]: number; - /** - * A map of generations to service ids - */ - [kServiceGenerations]: Map; - [kConnectionCounter]: Generator; - [kCancellationToken]: CancellationToken; - [kWaitQueue]: List; - [kMetrics]: ConnectionPoolMetrics; - [kProcessingWaitQueue]: boolean; + public options: Readonly; + public generation: number; + public serviceGenerations: Map; + + private poolState: PoolState; + private server: Server; + private connections: List; + private pending: number; + private checkedOut: Set; + private minPoolSizeTimer?: NodeJS.Timeout; + private connectionCounter: Generator; + private cancellationToken: CancellationToken; + private waitQueue: List; + private metrics: ConnectionPoolMetrics; + private processingWaitQueue: boolean; /** * Emitted when the connection pool is created. @@ -241,22 +209,22 @@ export class ConnectionPool extends TypedEventEmitter { ); } - this[kPoolState] = PoolState.paused; - this[kServer] = server; - this[kConnections] = new List(); - this[kPending] = 0; - this[kCheckedOut] = new Set(); - this[kMinPoolSizeTimer] = undefined; - this[kGeneration] = 0; - this[kServiceGenerations] = new Map(); - this[kConnectionCounter] = makeCounter(1); - this[kCancellationToken] = new CancellationToken(); - this[kCancellationToken].setMaxListeners(Infinity); - this[kWaitQueue] = new List(); - this[kMetrics] = new ConnectionPoolMetrics(); - this[kProcessingWaitQueue] = false; - - this.mongoLogger = this[kServer].topology.client?.mongoLogger; + this.poolState = PoolState.paused; + this.server = server; + this.connections = new List(); + this.pending = 0; + this.checkedOut = new Set(); + this.minPoolSizeTimer = undefined; + this.generation = 0; + this.serviceGenerations = new Map(); + this.connectionCounter = makeCounter(1); + this.cancellationToken = new CancellationToken(); + this.cancellationToken.setMaxListeners(Infinity); + this.waitQueue = new List(); + this.metrics = new ConnectionPoolMetrics(); + this.processingWaitQueue = false; + + this.mongoLogger = this.server.topology.client?.mongoLogger; this.component = 'connection'; process.nextTick(() => { @@ -275,12 +243,7 @@ export class ConnectionPool extends TypedEventEmitter { * TODO(NODE-3263): We can remove this property once shell no longer needs it */ get closed(): boolean { - return this[kPoolState] === PoolState.closed; - } - - /** An integer representing the SDAM generation of the pool */ - get generation(): number { - return this[kGeneration]; + return this.poolState === PoolState.closed; } /** An integer expressing how many total connections (available + pending + in use) the pool currently has */ @@ -292,31 +255,27 @@ export class ConnectionPool extends TypedEventEmitter { /** An integer expressing how many connections are currently available in the pool. */ get availableConnectionCount(): number { - return this[kConnections].length; + return this.connections.length; } get pendingConnectionCount(): number { - return this[kPending]; + return this.pending; } get currentCheckedOutCount(): number { - return this[kCheckedOut].size; + return this.checkedOut.size; } get waitQueueSize(): number { - return this[kWaitQueue].length; + return this.waitQueue.length; } get loadBalanced(): boolean { return this.options.loadBalanced; } - get serviceGenerations(): Map { - return this[kServiceGenerations]; - } - get serverError() { - return this[kServer].description.error; + return this.server.description.error; } /** @@ -327,26 +286,26 @@ export class ConnectionPool extends TypedEventEmitter { * This property may be removed as a part of NODE-3263. */ get checkedOutConnections() { - return this[kCheckedOut]; + return this.checkedOut; } /** * Get the metrics information for the pool when a wait queue timeout occurs. */ private waitQueueErrorMetrics(): string { - return this[kMetrics].info(this.options.maxPoolSize); + return this.metrics.info(this.options.maxPoolSize); } /** * Set the pool state to "ready" */ ready(): void { - if (this[kPoolState] !== PoolState.paused) { + if (this.poolState !== PoolState.paused) { return; } - this[kPoolState] = PoolState.ready; + this.poolState = PoolState.ready; this.emitAndLog(ConnectionPool.CONNECTION_POOL_READY, new ConnectionPoolReadyEvent(this)); - clearTimeout(this[kMinPoolSizeTimer]); + clearTimeout(this.minPoolSizeTimer); this.ensureMinPoolSize(); } @@ -369,10 +328,11 @@ export class ConnectionPool extends TypedEventEmitter { const waitQueueMember: WaitQueueMember = { resolve, reject, + cancelled: false, checkoutTime }; - this[kWaitQueue].push(waitQueueMember); + this.waitQueue.push(waitQueueMember); process.nextTick(() => this.processWaitQueue()); try { @@ -381,7 +341,7 @@ export class ConnectionPool extends TypedEventEmitter { } catch (error) { if (TimeoutError.is(error)) { timeout?.clear(); - waitQueueMember[kCancelled] = true; + waitQueueMember.cancelled = true; this.emitAndLog( ConnectionPool.CONNECTION_CHECK_OUT_FAILED, @@ -412,7 +372,7 @@ export class ConnectionPool extends TypedEventEmitter { * @param connection - The connection to check in */ checkIn(connection: Connection): void { - if (!this[kCheckedOut].has(connection)) { + if (!this.checkedOut.has(connection)) { return; } const poolClosed = this.closed; @@ -421,10 +381,10 @@ export class ConnectionPool extends TypedEventEmitter { if (!willDestroy) { connection.markAvailable(); - this[kConnections].unshift(connection); + this.connections.unshift(connection); } - this[kCheckedOut].delete(connection); + this.checkedOut.delete(connection); this.emitAndLog( ConnectionPool.CONNECTION_CHECKED_IN, new ConnectionCheckedInEvent(this, connection) @@ -475,10 +435,10 @@ export class ConnectionPool extends TypedEventEmitter { } // handle non load-balanced case const interruptInUseConnections = options.interruptInUseConnections ?? false; - const oldGeneration = this[kGeneration]; - this[kGeneration] += 1; - const alreadyPaused = this[kPoolState] === PoolState.paused; - this[kPoolState] = PoolState.paused; + const oldGeneration = this.generation; + this.generation += 1; + const alreadyPaused = this.poolState === PoolState.paused; + this.poolState = PoolState.paused; this.clearMinPoolSizeTimer(); if (!alreadyPaused) { @@ -503,7 +463,7 @@ export class ConnectionPool extends TypedEventEmitter { * Only connections where `connection.generation <= minGeneration` are killed. */ private interruptInUseConnections(minGeneration: number) { - for (const connection of this[kCheckedOut]) { + for (const connection of this.checkedOut) { if (connection.generation <= minGeneration) { connection.onError(new PoolClearedOnNetworkError(this)); this.checkIn(connection); @@ -518,25 +478,25 @@ export class ConnectionPool extends TypedEventEmitter { } // immediately cancel any in-flight connections - this[kCancellationToken].emit('cancel'); + this.cancellationToken.emit('cancel'); // end the connection counter - if (typeof this[kConnectionCounter].return === 'function') { - this[kConnectionCounter].return(undefined); + if (typeof this.connectionCounter.return === 'function') { + this.connectionCounter.return(undefined); } - this[kPoolState] = PoolState.closed; + this.poolState = PoolState.closed; this.clearMinPoolSizeTimer(); this.processWaitQueue(); - for (const conn of this[kConnections]) { + for (const conn of this.connections) { this.emitAndLog( ConnectionPool.CONNECTION_CLOSED, new ConnectionClosedEvent(this, conn, 'poolClosed') ); conn.destroy(); } - this[kConnections].clear(); + this.connections.clear(); this.emitAndLog(ConnectionPool.CONNECTION_POOL_CLOSED, new ConnectionPoolClosedEvent(this)); } @@ -557,7 +517,7 @@ export class ConnectionPool extends TypedEventEmitter { } const resolvedCredentials = credentials.resolveAuthMechanism(connection.hello); - const provider = this[kServer].topology.client.s.authProviders.getOrCreateProvider( + const provider = this.server.topology.client.s.authProviders.getOrCreateProvider( resolvedCredentials.mechanism, resolvedCredentials.mechanismProperties ); @@ -575,7 +535,7 @@ export class ConnectionPool extends TypedEventEmitter { /** Clear the min pool size timer */ private clearMinPoolSizeTimer(): void { - const minPoolSizeTimer = this[kMinPoolSizeTimer]; + const minPoolSizeTimer = this.minPoolSizeTimer; if (minPoolSizeTimer) { clearTimeout(minPoolSizeTimer); } @@ -601,7 +561,7 @@ export class ConnectionPool extends TypedEventEmitter { return connection.generation !== generation; } - return connection.generation !== this[kGeneration]; + return connection.generation !== this.generation; } private connectionIsIdle(connection: Connection) { @@ -627,14 +587,14 @@ export class ConnectionPool extends TypedEventEmitter { private createConnection(callback: Callback) { const connectOptions: ConnectionOptions = { ...this.options, - id: this[kConnectionCounter].next().value, - generation: this[kGeneration], - cancellationToken: this[kCancellationToken], + id: this.connectionCounter.next().value, + generation: this.generation, + cancellationToken: this.cancellationToken, mongoLogger: this.mongoLogger, - authProviders: this[kServer].topology.client.s.authProviders + authProviders: this.server.topology.client.s.authProviders }; - this[kPending]++; + this.pending++; // This is our version of a "virtual" no-I/O connection as the spec requires const connectionCreatedTime = now(); this.emitAndLog( @@ -645,8 +605,8 @@ export class ConnectionPool extends TypedEventEmitter { connect(connectOptions).then( connection => { // The pool might have closed since we started trying to create a connection - if (this[kPoolState] !== PoolState.ready) { - this[kPending]--; + if (this.poolState !== PoolState.ready) { + this.pending--; connection.destroy(); callback(this.closed ? new PoolClosedError(this) : new PoolClearedError(this)); return; @@ -658,8 +618,8 @@ export class ConnectionPool extends TypedEventEmitter { } if (this.loadBalanced) { - connection.on(Connection.PINNED, pinType => this[kMetrics].markPinned(pinType)); - connection.on(Connection.UNPINNED, pinType => this[kMetrics].markUnpinned(pinType)); + connection.on(Connection.PINNED, pinType => this.metrics.markPinned(pinType)); + connection.on(Connection.UNPINNED, pinType => this.metrics.markUnpinned(pinType)); const serviceId = connection.serviceId; if (serviceId) { @@ -680,12 +640,12 @@ export class ConnectionPool extends TypedEventEmitter { new ConnectionReadyEvent(this, connection, connectionCreatedTime) ); - this[kPending]--; + this.pending--; callback(undefined, connection); }, error => { - this[kPending]--; - this[kServer].handleError(error); + this.pending--; + this.server.handleError(error); this.emitAndLog( ConnectionPool.CONNECTION_CLOSED, new ConnectionClosedEvent( @@ -706,11 +666,11 @@ export class ConnectionPool extends TypedEventEmitter { private ensureMinPoolSize() { const minPoolSize = this.options.minPoolSize; - if (this[kPoolState] !== PoolState.ready || minPoolSize === 0) { + if (this.poolState !== PoolState.ready || minPoolSize === 0) { return; } - this[kConnections].prune(connection => this.destroyConnectionIfPerished(connection)); + this.connections.prune(connection => this.destroyConnectionIfPerished(connection)); if ( this.totalConnectionCount < minPoolSize && @@ -721,20 +681,20 @@ export class ConnectionPool extends TypedEventEmitter { // the connection to a checkout request this.createConnection((err, connection) => { if (!err && connection) { - this[kConnections].push(connection); + this.connections.push(connection); process.nextTick(() => this.processWaitQueue()); } - if (this[kPoolState] === PoolState.ready) { - clearTimeout(this[kMinPoolSizeTimer]); - this[kMinPoolSizeTimer] = setTimeout( + if (this.poolState === PoolState.ready) { + clearTimeout(this.minPoolSizeTimer); + this.minPoolSizeTimer = setTimeout( () => this.ensureMinPoolSize(), this.options.minPoolSizeCheckFrequencyMS ); } }); } else { - clearTimeout(this[kMinPoolSizeTimer]); - this[kMinPoolSizeTimer] = setTimeout( + clearTimeout(this.minPoolSizeTimer); + this.minPoolSizeTimer = setTimeout( () => this.ensureMinPoolSize(), this.options.minPoolSizeCheckFrequencyMS ); @@ -742,31 +702,31 @@ export class ConnectionPool extends TypedEventEmitter { } private processWaitQueue() { - if (this[kProcessingWaitQueue]) { + if (this.processingWaitQueue) { return; } - this[kProcessingWaitQueue] = true; + this.processingWaitQueue = true; while (this.waitQueueSize) { - const waitQueueMember = this[kWaitQueue].first(); + const waitQueueMember = this.waitQueue.first(); if (!waitQueueMember) { - this[kWaitQueue].shift(); + this.waitQueue.shift(); continue; } - if (waitQueueMember[kCancelled]) { - this[kWaitQueue].shift(); + if (waitQueueMember.cancelled) { + this.waitQueue.shift(); continue; } - if (this[kPoolState] !== PoolState.ready) { + if (this.poolState !== PoolState.ready) { const reason = this.closed ? 'poolClosed' : 'connectionError'; const error = this.closed ? new PoolClosedError(this) : new PoolClearedError(this); this.emitAndLog( ConnectionPool.CONNECTION_CHECK_OUT_FAILED, new ConnectionCheckOutFailedEvent(this, reason, waitQueueMember.checkoutTime, error) ); - this[kWaitQueue].shift(); + this.waitQueue.shift(); waitQueueMember.reject(error); continue; } @@ -775,19 +735,19 @@ export class ConnectionPool extends TypedEventEmitter { break; } - const connection = this[kConnections].shift(); + const connection = this.connections.shift(); if (!connection) { break; } if (!this.destroyConnectionIfPerished(connection)) { - this[kCheckedOut].add(connection); + this.checkedOut.add(connection); this.emitAndLog( ConnectionPool.CONNECTION_CHECKED_OUT, new ConnectionCheckedOutEvent(this, connection, waitQueueMember.checkoutTime) ); - this[kWaitQueue].shift(); + this.waitQueue.shift(); waitQueueMember.resolve(connection); } } @@ -798,14 +758,14 @@ export class ConnectionPool extends TypedEventEmitter { this.pendingConnectionCount < maxConnecting && (maxPoolSize === 0 || this.totalConnectionCount < maxPoolSize) ) { - const waitQueueMember = this[kWaitQueue].shift(); - if (!waitQueueMember || waitQueueMember[kCancelled]) { + const waitQueueMember = this.waitQueue.shift(); + if (!waitQueueMember || waitQueueMember.cancelled) { continue; } this.createConnection((err, connection) => { - if (waitQueueMember[kCancelled]) { + if (waitQueueMember.cancelled) { if (!err && connection) { - this[kConnections].push(connection); + this.connections.push(connection); } } else { if (err) { @@ -821,7 +781,7 @@ export class ConnectionPool extends TypedEventEmitter { ); waitQueueMember.reject(err); } else if (connection) { - this[kCheckedOut].add(connection); + this.checkedOut.add(connection); this.emitAndLog( ConnectionPool.CONNECTION_CHECKED_OUT, new ConnectionCheckedOutEvent(this, connection, waitQueueMember.checkoutTime) @@ -832,7 +792,7 @@ export class ConnectionPool extends TypedEventEmitter { process.nextTick(() => this.processWaitQueue()); }); } - this[kProcessingWaitQueue] = false; + this.processingWaitQueue = false; } } diff --git a/src/encrypter.ts b/src/encrypter.ts index fbcf7c195d9..06572fe2d0d 100644 --- a/src/encrypter.ts +++ b/src/encrypter.ts @@ -7,9 +7,6 @@ import { MongoInvalidArgumentError, MongoMissingDependencyError } from './error' import { MongoClient, type MongoClientOptions } from './mongo_client'; import { type Callback } from './utils'; -/** @internal */ -const kInternalClient = Symbol('internalClient'); - /** @internal */ export interface EncrypterOptions { autoEncryption: AutoEncryptionOptions; @@ -18,7 +15,7 @@ export interface EncrypterOptions { /** @internal */ export class Encrypter { - [kInternalClient]: MongoClient | null; + private internalClient: MongoClient | null; bypassAutoEncryption: boolean; needsConnecting: boolean; autoEncrypter: AutoEncrypter; @@ -28,7 +25,7 @@ export class Encrypter { throw new MongoInvalidArgumentError('Option "autoEncryption" must be specified'); } // initialize to null, if we call getInternalClient, we may set this it is important to not overwrite those function calls. - this[kInternalClient] = null; + this.internalClient = null; this.bypassAutoEncryption = !!options.autoEncryption.bypassAutoEncryption; this.needsConnecting = false; @@ -61,7 +58,7 @@ export class Encrypter { getInternalClient(client: MongoClient, uri: string, options: MongoClientOptions): MongoClient { // TODO(NODE-4144): Remove new variable for type narrowing - let internalClient = this[kInternalClient]; + let internalClient = this.internalClient; if (internalClient == null) { const clonedOptions: MongoClientOptions = {}; @@ -77,7 +74,7 @@ export class Encrypter { clonedOptions.minPoolSize = 0; internalClient = new MongoClient(uri, clonedOptions); - this[kInternalClient] = internalClient; + this.internalClient = internalClient; for (const eventName of MONGO_CLIENT_EVENTS) { for (const listener of client.listeners(eventName)) { @@ -96,7 +93,7 @@ export class Encrypter { async connectInternalClient(): Promise { // TODO(NODE-4144): Remove new variable for type narrowing - const internalClient = this[kInternalClient]; + const internalClient = this.internalClient; if (this.needsConnecting && internalClient != null) { this.needsConnecting = false; await internalClient.connect(); @@ -114,7 +111,7 @@ export class Encrypter { } catch (autoEncrypterError) { error = autoEncrypterError; } - const internalClient = this[kInternalClient]; + const internalClient = this.internalClient; if (internalClient != null && client !== internalClient) { return await internalClient.close(force); } diff --git a/src/error.ts b/src/error.ts index 8f0adb0c440..7c06a79d323 100644 --- a/src/error.ts +++ b/src/error.ts @@ -10,9 +10,6 @@ import type { TopologyDescription } from './sdam/topology_description'; /** @public */ export type AnyError = MongoError | Error; -/** @internal */ -const kErrorLabels = Symbol('errorLabels'); - /** * @internal * The legacy error message from the server that indicates the node is not a writable primary @@ -128,8 +125,7 @@ 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 { - /** @internal */ - [kErrorLabels]: Set; + public readonly errorLabels: string[] = []; /** * This is a number in MongoServerError and a string in MongoDriverError * @privateRemarks @@ -153,7 +149,6 @@ export class MongoError extends Error { **/ constructor(message: string, options?: { cause?: Error }) { super(message, options); - this[kErrorLabels] = new Set(); } /** @internal */ @@ -188,15 +183,11 @@ export class MongoError extends Error { * @returns returns true if the error has the provided error label */ hasErrorLabel(label: string): boolean { - return this[kErrorLabels].has(label); + return this.errorLabels.includes(label); } addErrorLabel(label: string): void { - this[kErrorLabels].add(label); - } - - get errorLabels(): string[] { - return Array.from(this[kErrorLabels]); + if (!this.hasErrorLabel(label)) this.errorLabels.push(label); } } @@ -228,8 +219,9 @@ export class MongoServerError extends MongoError { **/ constructor(message: ErrorDescription) { super(message.message || message.errmsg || message.$err || 'n/a'); + if (message.errorLabels) { - this[kErrorLabels] = new Set(message.errorLabels); + for (const label of message.errorLabels) this.addErrorLabel(label); } this.errorResponse = message; @@ -1028,12 +1020,6 @@ export class MongoTopologyClosedError extends MongoAPIError { } } -/** @internal */ -const kBeforeHandshake = Symbol('beforeHandshake'); -export function isNetworkErrorBeforeHandshake(err: MongoNetworkError): boolean { - return err[kBeforeHandshake] === true; -} - /** @public */ export interface MongoNetworkErrorOptions { /** Indicates the timeout happened before a connection handshake completed */ @@ -1048,7 +1034,7 @@ export interface MongoNetworkErrorOptions { */ export class MongoNetworkError extends MongoError { /** @internal */ - [kBeforeHandshake]?: boolean; + private beforeHandshake?: boolean; /** * **Do not use this constructor!** @@ -1065,13 +1051,18 @@ export class MongoNetworkError extends MongoError { super(message, { cause: options?.cause }); if (options && typeof options.beforeHandshake === 'boolean') { - this[kBeforeHandshake] = options.beforeHandshake; + this.beforeHandshake = options.beforeHandshake; } } override get name(): string { return 'MongoNetworkError'; } + + /** @internal */ + static isBeforeHandshake(err: MongoNetworkError): boolean { + return err.beforeHandshake === true; + } } /** diff --git a/src/mongo_client.ts b/src/mongo_client.ts index bab3d2c0f4d..dea069cc949 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -333,9 +333,6 @@ export type MongoClientEvents = Pick implements /** * The consolidate, parsed, transformed and merged options. - * @internal */ - [kOptions]: MongoOptions; + public readonly options: Readonly< + Omit + > & + Pick; constructor(url: string, options?: MongoClientOptions) { super(); - this[kOptions] = parseOptions(url, this, options); + this.options = parseOptions(url, this, options); - const shouldSetLogger = Object.values( - this[kOptions].mongoLoggerOptions.componentSeverities - ).some(value => value !== SeverityLevel.OFF); + const shouldSetLogger = Object.values(this.options.mongoLoggerOptions.componentSeverities).some( + value => value !== SeverityLevel.OFF + ); this.mongoLogger = shouldSetLogger - ? new MongoLogger(this[kOptions].mongoLoggerOptions) + ? new MongoLogger(this.options.mongoLoggerOptions) : undefined; // eslint-disable-next-line @typescript-eslint/no-this-alias @@ -389,7 +388,7 @@ export class MongoClient extends TypedEventEmitter implements // The internal state this.s = { url, - bsonOptions: resolveBSONOptions(this[kOptions]), + bsonOptions: resolveBSONOptions(this.options), namespace: ns('admin'), hasBeenClosed: false, sessionPool: new ServerSessionPool(this), @@ -397,16 +396,16 @@ export class MongoClient extends TypedEventEmitter implements authProviders: new MongoClientAuthProviders(), get options() { - return client[kOptions]; + return client.options; }, get readConcern() { - return client[kOptions].readConcern; + return client.options.readConcern; }, get writeConcern() { - return client[kOptions].writeConcern; + return client.options.writeConcern; }, get readPreference() { - return client[kOptions].readPreference; + return client.options.readPreference; }, get isMongoClient(): true { return true; @@ -428,15 +427,15 @@ export class MongoClient extends TypedEventEmitter implements /** @internal */ private checkForNonGenuineHosts() { - const documentDBHostnames = this[kOptions].hosts.filter((hostAddress: HostAddress) => + const documentDBHostnames = this.options.hosts.filter((hostAddress: HostAddress) => isHostMatch(DOCUMENT_DB_CHECK, hostAddress.host) ); - const srvHostIsDocumentDB = isHostMatch(DOCUMENT_DB_CHECK, this[kOptions].srvHost); + const srvHostIsDocumentDB = isHostMatch(DOCUMENT_DB_CHECK, this.options.srvHost); - const cosmosDBHostnames = this[kOptions].hosts.filter((hostAddress: HostAddress) => + const cosmosDBHostnames = this.options.hosts.filter((hostAddress: HostAddress) => isHostMatch(COSMOS_DB_CHECK, hostAddress.host) ); - const srvHostIsCosmosDB = isHostMatch(COSMOS_DB_CHECK, this[kOptions].srvHost); + const srvHostIsCosmosDB = isHostMatch(COSMOS_DB_CHECK, this.options.srvHost); if (documentDBHostnames.length !== 0 || srvHostIsDocumentDB) { this.mongoLogger?.info('client', DOCUMENT_DB_MSG); @@ -445,28 +444,23 @@ export class MongoClient extends TypedEventEmitter implements } } - /** @see MongoOptions */ - get options(): Readonly { - return Object.freeze({ ...this[kOptions] }); - } - get serverApi(): Readonly { - return this[kOptions].serverApi && Object.freeze({ ...this[kOptions].serverApi }); + return this.options.serverApi && Object.freeze({ ...this.options.serverApi }); } /** * Intended for APM use only * @internal */ get monitorCommands(): boolean { - return this[kOptions].monitorCommands; + return this.options.monitorCommands; } set monitorCommands(value: boolean) { - this[kOptions].monitorCommands = value; + this.options.monitorCommands = value; } /** @internal */ get autoEncrypter(): AutoEncrypter | undefined { - return this[kOptions].autoEncrypter; + return this.options.autoEncrypter; } get readConcern(): ReadConcern | undefined { @@ -551,7 +545,7 @@ export class MongoClient extends TypedEventEmitter implements return this; } - const options = this[kOptions]; + const options = this.options; if (options.tls) { if (typeof options.tlsCAFile === 'string') { @@ -685,7 +679,7 @@ export class MongoClient extends TypedEventEmitter implements topology.close(); - const { encrypter } = this[kOptions]; + const { encrypter } = this.options; if (encrypter) { await encrypter.close(this, force); } @@ -706,7 +700,7 @@ export class MongoClient extends TypedEventEmitter implements } // Copy the options and add out internal override of the not shared flag - const finalOptions = Object.assign({}, this[kOptions], options); + const finalOptions = Object.assign({}, this.options, options); // Return the db object const db = new Db(this, dbName, finalOptions); @@ -748,7 +742,7 @@ export class MongoClient extends TypedEventEmitter implements this, this.s.sessionPool, { explicit: true, ...options }, - this[kOptions] + this.options ); this.s.activeSessions.add(session); session.once('ended', () => { diff --git a/src/operations/operation.ts b/src/operations/operation.ts index 1c5be203516..029047543a3 100644 --- a/src/operations/operation.ts +++ b/src/operations/operation.ts @@ -42,9 +42,6 @@ export interface OperationOptions extends BSONSerializeOptions { timeoutMS?: number; } -/** @internal */ -const kSession = Symbol('session'); - /** * This class acts as a parent class for any operation and is responsible for setting this.options, * as well as setting and getting a session. @@ -67,7 +64,7 @@ export abstract class AbstractOperation { /** Specifies the time an operation will run until it throws a timeout error. */ timeoutMS?: number; - [kSession]: ClientSession | undefined; + private _session: ClientSession | undefined; static aspects?: Set; @@ -79,7 +76,7 @@ export abstract class AbstractOperation { // Pull the BSON serialize options from the already-resolved options this.bsonOptions = resolveBSONOptions(options); - this[kSession] = options.session != null ? options.session : undefined; + this._session = options.session != null ? options.session : undefined; this.options = options; this.bypassPinningCheck = !!options.bypassPinningCheck; @@ -105,12 +102,13 @@ export abstract class AbstractOperation { return ctor.aspects.has(aspect); } + // Make sure the session is not writable from outside this class. get session(): ClientSession | undefined { - return this[kSession]; + return this._session; } clearSession() { - this[kSession] = undefined; + this._session = undefined; } resetBatch(): boolean { diff --git a/src/sdam/monitor.ts b/src/sdam/monitor.ts index 4c4fa3bb498..65fb0403791 100644 --- a/src/sdam/monitor.ts +++ b/src/sdam/monitor.ts @@ -25,13 +25,6 @@ import { import { Server } from './server'; import type { TopologyVersion } from './server_description'; -/** @internal */ -const kServer = Symbol('server'); -/** @internal */ -const kMonitorId = Symbol('monitorId'); -/** @internal */ -const kCancellationToken = Symbol('cancellationToken'); - const STATE_IDLE = 'idle'; const STATE_MONITORING = 'monitoring'; const stateTransition = makeStateMachine({ @@ -96,11 +89,11 @@ export class Monitor extends TypedEventEmitter { >; connectOptions: ConnectionOptions; isRunningInFaasEnv: boolean; - [kServer]: Server; + server: Server; connection: Connection | null; - [kCancellationToken]: CancellationToken; + cancellationToken: CancellationToken; /** @internal */ - [kMonitorId]?: MonitorInterval; + monitorId?: MonitorInterval; rttPinger?: RTTPinger; /** @internal */ override component = MongoLoggableComponent.TOPOLOGY; @@ -110,11 +103,11 @@ export class Monitor extends TypedEventEmitter { constructor(server: Server, options: MonitorOptions) { super(); - this[kServer] = server; + this.server = server; this.connection = null; - this[kCancellationToken] = new CancellationToken(); - this[kCancellationToken].setMaxListeners(Infinity); - this[kMonitorId] = undefined; + this.cancellationToken = new CancellationToken(); + this.cancellationToken.setMaxListeners(Infinity); + this.monitorId = undefined; this.s = { state: STATE_CLOSED }; @@ -126,10 +119,10 @@ export class Monitor extends TypedEventEmitter { serverMonitoringMode: options.serverMonitoringMode }); this.isRunningInFaasEnv = getFAASEnv() != null; - this.mongoLogger = this[kServer].topology.client?.mongoLogger; + this.mongoLogger = this.server.topology.client?.mongoLogger; this.rttSampler = new RTTSampler(10); - const cancellationToken = this[kCancellationToken]; + const cancellationToken = this.cancellationToken; // TODO: refactor this to pull it directly from the pool, requires new ConnectionPool integration const connectOptions = { id: '' as const, @@ -162,7 +155,7 @@ export class Monitor extends TypedEventEmitter { // start const heartbeatFrequencyMS = this.options.heartbeatFrequencyMS; const minHeartbeatFrequencyMS = this.options.minHeartbeatFrequencyMS; - this[kMonitorId] = new MonitorInterval(monitorServer(this), { + this.monitorId = new MonitorInterval(monitorServer(this), { heartbeatFrequencyMS: heartbeatFrequencyMS, minHeartbeatFrequencyMS: minHeartbeatFrequencyMS, immediate: true @@ -174,11 +167,11 @@ export class Monitor extends TypedEventEmitter { return; } - this[kMonitorId]?.wake(); + this.monitorId?.wake(); } reset(): void { - const topologyVersion = this[kServer].description.topologyVersion; + const topologyVersion = this.server.description.topologyVersion; if (isInCloseState(this) || topologyVersion == null) { return; } @@ -192,7 +185,7 @@ export class Monitor extends TypedEventEmitter { // restart monitoring const heartbeatFrequencyMS = this.options.heartbeatFrequencyMS; const minHeartbeatFrequencyMS = this.options.minHeartbeatFrequencyMS; - this[kMonitorId] = new MonitorInterval(monitorServer(this), { + this.monitorId = new MonitorInterval(monitorServer(this), { heartbeatFrequencyMS: heartbeatFrequencyMS, minHeartbeatFrequencyMS: minHeartbeatFrequencyMS }); @@ -233,13 +226,13 @@ export class Monitor extends TypedEventEmitter { } function resetMonitorState(monitor: Monitor) { - monitor[kMonitorId]?.stop(); - monitor[kMonitorId] = undefined; + monitor.monitorId?.stop(); + monitor.monitorId = undefined; monitor.rttPinger?.close(); monitor.rttPinger = undefined; - monitor[kCancellationToken].emit('cancel'); + monitor.cancellationToken.emit('cancel'); monitor.connection?.destroy(); monitor.connection = null; @@ -266,11 +259,11 @@ function useStreamingProtocol(monitor: Monitor, topologyVersion: TopologyVersion function checkServer(monitor: Monitor, callback: Callback) { let start: number; let awaited: boolean; - const topologyVersion = monitor[kServer].description.topologyVersion; + const topologyVersion = monitor.server.description.topologyVersion; const isAwaitable = useStreamingProtocol(monitor, topologyVersion); monitor.emitAndLogHeartbeat( Server.SERVER_HEARTBEAT_STARTED, - monitor[kServer].topology.s.id, + monitor.server.topology.s.id, undefined, new ServerHeartbeatStartedEvent(monitor.address, isAwaitable) ); @@ -280,7 +273,7 @@ function checkServer(monitor: Monitor, callback: Callback) { monitor.connection = null; monitor.emitAndLogHeartbeat( Server.SERVER_HEARTBEAT_FAILED, - monitor[kServer].topology.s.id, + monitor.server.topology.s.id, undefined, new ServerHeartbeatFailedEvent(monitor.address, calculateDurationInMs(start), err, awaited) ); @@ -315,7 +308,7 @@ function checkServer(monitor: Monitor, callback: Callback) { monitor.emitAndLogHeartbeat( Server.SERVER_HEARTBEAT_SUCCEEDED, - monitor[kServer].topology.s.id, + monitor.server.topology.s.id, hello.connectionId, new ServerHeartbeatSucceededEvent(monitor.address, duration, hello, isAwaitable) ); @@ -325,7 +318,7 @@ function checkServer(monitor: Monitor, callback: Callback) { // event, otherwise the "check" is complete and return to the main monitor loop monitor.emitAndLogHeartbeat( Server.SERVER_HEARTBEAT_STARTED, - monitor[kServer].topology.s.id, + monitor.server.topology.s.id, undefined, new ServerHeartbeatStartedEvent(monitor.address, true) ); @@ -378,7 +371,6 @@ function checkServer(monitor: Monitor, callback: Callback) { awaited = false; connection .command(ns('admin.$cmd'), cmd, options) - .then(onHeartbeatSucceeded, onHeartbeatFailed); return; @@ -409,7 +401,7 @@ function checkServer(monitor: Monitor, callback: Callback) { monitor.connection = connection; monitor.emitAndLogHeartbeat( Server.SERVER_HEARTBEAT_SUCCEEDED, - monitor[kServer].topology.s.id, + monitor.server.topology.s.id, connection.hello?.connectionId, new ServerHeartbeatSucceededEvent( monitor.address, @@ -447,7 +439,7 @@ function monitorServer(monitor: Monitor) { checkServer(monitor, (err, hello) => { if (err) { // otherwise an error occurred on initial discovery, also bail - if (monitor[kServer].description.type === ServerType.Unknown) { + if (monitor.server.description.type === ServerType.Unknown) { return done(); } } @@ -456,7 +448,7 @@ function monitorServer(monitor: Monitor) { if (useStreamingProtocol(monitor, hello?.topologyVersion)) { setTimeout(() => { if (!isInCloseState(monitor)) { - monitor[kMonitorId]?.wake(); + monitor.monitorId?.wake(); } }, 0); } @@ -484,9 +476,9 @@ export interface RTTPingerOptions extends ConnectionOptions { export class RTTPinger { connection?: Connection; /** @internal */ - [kCancellationToken]: CancellationToken; + cancellationToken: CancellationToken; /** @internal */ - [kMonitorId]: NodeJS.Timeout; + monitorId: NodeJS.Timeout; /** @internal */ monitor: Monitor; closed: boolean; @@ -495,13 +487,13 @@ export class RTTPinger { constructor(monitor: Monitor) { this.connection = undefined; - this[kCancellationToken] = monitor[kCancellationToken]; + this.cancellationToken = monitor.cancellationToken; this.closed = false; this.monitor = monitor; this.latestRtt = monitor.latestRtt ?? undefined; const heartbeatFrequencyMS = monitor.options.heartbeatFrequencyMS; - this[kMonitorId] = setTimeout(() => this.measureRoundTripTime(), heartbeatFrequencyMS); + this.monitorId = setTimeout(() => this.measureRoundTripTime(), heartbeatFrequencyMS); } get roundTripTime(): number { @@ -514,7 +506,7 @@ export class RTTPinger { close(): void { this.closed = true; - clearTimeout(this[kMonitorId]); + clearTimeout(this.monitorId); this.connection?.destroy(); this.connection = undefined; @@ -531,7 +523,7 @@ export class RTTPinger { } this.latestRtt = calculateDurationInMs(start); - this[kMonitorId] = setTimeout( + this.monitorId = setTimeout( () => this.measureRoundTripTime(), this.monitor.options.heartbeatFrequencyMS ); diff --git a/src/sdam/server.ts b/src/sdam/server.ts index 47a390277d6..0f8140c68ef 100644 --- a/src/sdam/server.ts +++ b/src/sdam/server.ts @@ -22,7 +22,6 @@ import { } from '../constants'; import { type AnyError, - isNetworkErrorBeforeHandshake, isNodeShuttingDownError, isSDAMUnrecoverableError, MONGODB_ERROR_CODES, @@ -381,7 +380,7 @@ export class Server extends TypedEventEmitter { const isNetworkNonTimeoutError = error instanceof MongoNetworkError && !(error instanceof MongoNetworkTimeoutError); - const isNetworkTimeoutBeforeHandshakeError = isNetworkErrorBeforeHandshake(error); + const isNetworkTimeoutBeforeHandshakeError = MongoNetworkError.isBeforeHandshake(error); const isAuthHandshakeError = error.hasErrorLabel(MongoErrorLabel.HandshakeError); if (isNetworkNonTimeoutError || isNetworkTimeoutBeforeHandshakeError || isAuthHandshakeError) { // In load balanced mode we never mark the server as unknown and always diff --git a/src/sdam/topology.ts b/src/sdam/topology.ts index fcfc05d06ad..61a3bb84fec 100644 --- a/src/sdam/topology.ts +++ b/src/sdam/topology.ts @@ -88,11 +88,6 @@ const stateTransition = makeStateMachine({ [STATE_CLOSING]: [STATE_CLOSING, STATE_CLOSED] }); -/** @internal */ -const kCancelled = Symbol('cancelled'); -/** @internal */ -const kWaitQueue = Symbol('waitQueue'); - /** @internal */ export type ServerSelectionCallback = Callback; @@ -105,7 +100,7 @@ export interface ServerSelectionRequest { startTime: number; resolve: (server: Server) => void; reject: (error: MongoError) => void; - [kCancelled]?: boolean; + cancelled: boolean; operationName: string; waitingLogged: boolean; previousServer?: ServerDescription; @@ -208,7 +203,7 @@ export class Topology extends TypedEventEmitter { /** @internal */ s: TopologyPrivate; /** @internal */ - [kWaitQueue]: List; + waitQueue: List; /** @internal */ hello?: Document; /** @internal */ @@ -293,7 +288,7 @@ export class Topology extends TypedEventEmitter { serverDescriptions.set(hostAddress.toString(), new ServerDescription(hostAddress)); } - this[kWaitQueue] = new List(); + this.waitQueue = new List(); this.s = { // the id of this topology id: topologyId, @@ -506,7 +501,7 @@ export class Topology extends TypedEventEmitter { stateTransition(this, STATE_CLOSING); - drainWaitQueue(this[kWaitQueue], new MongoTopologyClosedError()); + drainWaitQueue(this.waitQueue, new MongoTopologyClosedError()); if (this.s.srvPoller) { this.s.srvPoller.stop(); @@ -601,13 +596,14 @@ export class Topology extends TypedEventEmitter { transaction, resolve, reject, + cancelled: false, startTime: now(), operationName: options.operationName, waitingLogged: false, previousServer: options.previousServer }; - this[kWaitQueue].push(waitQueueMember); + this.waitQueue.push(waitQueueMember); processWaitQueue(this); try { @@ -620,7 +616,7 @@ export class Topology extends TypedEventEmitter { } catch (error) { if (TimeoutError.is(error)) { // Timeout - waitQueueMember[kCancelled] = true; + waitQueueMember.cancelled = true; const timeoutError = new MongoServerSelectionError( `Server selection timed out after ${timeout?.duration} ms`, this.description @@ -721,7 +717,7 @@ export class Topology extends TypedEventEmitter { updateServers(this, serverDescription); // attempt to resolve any outstanding server selection attempts - if (this[kWaitQueue].length > 0) { + if (this.waitQueue.length > 0) { processWaitQueue(this); } @@ -910,7 +906,7 @@ function drainWaitQueue(queue: List, drainError: MongoDr continue; } - if (!waitQueueMember[kCancelled]) { + if (!waitQueueMember.cancelled) { if ( waitQueueMember.mongoLogger?.willLog( MongoLoggableComponent.SERVER_SELECTION, @@ -934,20 +930,20 @@ function drainWaitQueue(queue: List, drainError: MongoDr function processWaitQueue(topology: Topology) { if (topology.s.state === STATE_CLOSED) { - drainWaitQueue(topology[kWaitQueue], new MongoTopologyClosedError()); + drainWaitQueue(topology.waitQueue, new MongoTopologyClosedError()); return; } const isSharded = topology.description.type === TopologyType.Sharded; const serverDescriptions = Array.from(topology.description.servers.values()); - const membersToProcess = topology[kWaitQueue].length; + const membersToProcess = topology.waitQueue.length; for (let i = 0; i < membersToProcess; ++i) { - const waitQueueMember = topology[kWaitQueue].shift(); + const waitQueueMember = topology.waitQueue.shift(); if (!waitQueueMember) { continue; } - if (waitQueueMember[kCancelled]) { + if (waitQueueMember.cancelled) { continue; } @@ -1006,7 +1002,7 @@ function processWaitQueue(topology: Topology) { } waitQueueMember.waitingLogged = true; } - topology[kWaitQueue].push(waitQueueMember); + topology.waitQueue.push(waitQueueMember); continue; } else if (selectedDescriptions.length === 1) { selectedServer = topology.s.servers.get(selectedDescriptions[0].address); @@ -1069,7 +1065,7 @@ function processWaitQueue(topology: Topology) { waitQueueMember.resolve(selectedServer); } - if (topology[kWaitQueue].length > 0) { + if (topology.waitQueue.length > 0) { // ensure all server monitors attempt monitoring soon for (const [, server] of topology.s.servers) { process.nextTick(function scheduleServerCheck() { diff --git a/src/sessions.ts b/src/sessions.ts index 26c829f4ac2..cb22538dd50 100644 --- a/src/sessions.ts +++ b/src/sessions.ts @@ -83,17 +83,6 @@ export type ClientSessionEvents = { ended(session: ClientSession): void; }; -/** @internal */ -const kServerSession = Symbol('serverSession'); -/** @internal */ -const kSnapshotTime = Symbol('snapshotTime'); -/** @internal */ -const kSnapshotEnabled = Symbol('snapshotEnabled'); -/** @internal */ -const kPinnedConnection = Symbol('pinnedConnection'); -/** @internal Accumulates total number of increments to add to txnNumber when applying session to command */ -const kTxnNumberIncrement = Symbol('txnNumberIncrement'); - /** @public */ export interface EndSessionOptions { /** @@ -132,20 +121,22 @@ export class ClientSession owner?: symbol | AbstractCursor; defaultTransactionOptions: TransactionOptions; transaction: Transaction; - /** @internal + /** + * @internal * Keeps track of whether or not the current transaction has attempted to be committed. Is - * initially undefined. Gets set to false when startTransaction is called. When commitTransaction is sent to server, if the commitTransaction succeeds, it is then set to undefined, otherwise, set to true */ - commitAttempted?: boolean; - /** @internal */ - private [kServerSession]: ServerSession | null; + * initially undefined. Gets set to false when startTransaction is called. When commitTransaction is sent to server, if the commitTransaction succeeds, it is then set to undefined, otherwise, set to true + */ + private commitAttempted?: boolean; + public readonly snapshotEnabled: boolean; + /** @internal */ - [kSnapshotTime]?: Timestamp; + private _serverSession: ServerSession | null; /** @internal */ - [kSnapshotEnabled] = false; + public snapshotTime?: Timestamp; /** @internal */ - [kPinnedConnection]?: Connection; + public pinnedConnection?: Connection; /** @internal */ - [kTxnNumberIncrement]: number; + public txnNumberIncrement: number; /** * @experimental * Specifies the time an operation in a given `ClientSession` will run until it throws a timeout error @@ -184,12 +175,14 @@ export class ClientSession options = options ?? {}; if (options.snapshot === true) { - this[kSnapshotEnabled] = true; + this.snapshotEnabled = true; if (options.causalConsistency === true) { throw new MongoInvalidArgumentError( 'Properties "causalConsistency" and "snapshot" are mutually exclusive' ); } + } else { + this.snapshotEnabled = false; } this.client = client; @@ -199,8 +192,8 @@ export class ClientSession this.timeoutMS = options.defaultTimeoutMS ?? client.s.options?.timeoutMS; this.explicit = !!options.explicit; - this[kServerSession] = this.explicit ? this.sessionPool.acquire() : null; - this[kTxnNumberIncrement] = 0; + this._serverSession = this.explicit ? this.sessionPool.acquire() : null; + this.txnNumberIncrement = 0; const defaultCausalConsistencyValue = this.explicit && options.snapshot !== true; this.supports = { @@ -218,11 +211,11 @@ export class ClientSession /** The server id associated with this session */ get id(): ServerSessionId | undefined { - return this[kServerSession]?.id; + return this.serverSession?.id; } get serverSession(): ServerSession { - let serverSession = this[kServerSession]; + let serverSession = this._serverSession; if (serverSession == null) { if (this.explicit) { throw new MongoRuntimeError('Unexpected null serverSession for an explicit session'); @@ -231,32 +224,22 @@ export class ClientSession throw new MongoRuntimeError('Unexpected null serverSession for an ended implicit session'); } serverSession = this.sessionPool.acquire(); - this[kServerSession] = serverSession; + this._serverSession = serverSession; } return serverSession; } - /** Whether or not this session is configured for snapshot reads */ - get snapshotEnabled(): boolean { - return this[kSnapshotEnabled]; - } - get loadBalanced(): boolean { return this.client.topology?.description.type === TopologyType.LoadBalanced; } - /** @internal */ - get pinnedConnection(): Connection | undefined { - return this[kPinnedConnection]; - } - /** @internal */ pin(conn: Connection): void { - if (this[kPinnedConnection]) { + if (this.pinnedConnection) { throw TypeError('Cannot pin multiple connections to the same session'); } - this[kPinnedConnection] = conn; + this.pinnedConnection = conn; conn.emit( PINNED, this.inTransaction() ? ConnectionPoolMetrics.TXN : ConnectionPoolMetrics.CURSOR @@ -273,7 +256,7 @@ export class ClientSession } get isPinned(): boolean { - return this.loadBalanced ? !!this[kPinnedConnection] : this.transaction.isPinned; + return this.loadBalanced ? !!this.pinnedConnection : this.transaction.isPinned; } /** @@ -295,12 +278,12 @@ export class ClientSession squashError(error); } finally { if (!this.hasEnded) { - const serverSession = this[kServerSession]; + const serverSession = this.serverSession; if (serverSession != null) { // release the server session back to the pool this.sessionPool.release(serverSession); // Store a clone of the server session for reference (debugging) - this[kServerSession] = new ServerSession(serverSession); + this._serverSession = new ServerSession(serverSession); } // mark the session as ended, and emit a signal this.hasEnded = true; @@ -391,7 +374,7 @@ export class ClientSession * This is because the serverSession is lazily acquired after a connection is obtained */ incrementTransactionNumber(): void { - this[kTxnNumberIncrement] += 1; + this.txnNumberIncrement += 1; } /** @returns whether this session is currently in a transaction or not */ @@ -410,7 +393,7 @@ export class ClientSession * @param options - Options for the transaction */ startTransaction(options?: TransactionOptions): void { - if (this[kSnapshotEnabled]) { + if (this.snapshotEnabled) { throw new MongoCompatibilityError('Transactions are not supported in snapshot sessions'); } @@ -908,7 +891,7 @@ export function maybeClearPinnedConnection( options?: EndSessionOptions ): void { // unpin a connection if it has been pinned - const conn = session[kPinnedConnection]; + const conn = session.pinnedConnection; const error = options?.error; if ( @@ -929,7 +912,7 @@ export function maybeClearPinnedConnection( if (options?.error == null || options?.force) { loadBalancer.pool.checkIn(conn); - session[kPinnedConnection] = undefined; + session.pinnedConnection = undefined; conn.emit( UNPINNED, session.transaction.state !== TxnState.NO_TRANSACTION @@ -1123,8 +1106,8 @@ export function applySession( const isRetryableWrite = !!options.willRetryWrite; if (isRetryableWrite || inTxnOrTxnCommand) { - serverSession.txnNumber += session[kTxnNumberIncrement]; - session[kTxnNumberIncrement] = 0; + serverSession.txnNumber += session.txnNumberIncrement; + session.txnNumberIncrement = 0; // TODO(NODE-2674): Preserve int64 sent from MongoDB command.txnNumber = Long.fromNumber(serverSession.txnNumber); } @@ -1141,10 +1124,10 @@ export function applySession( ) { command.readConcern = command.readConcern || {}; Object.assign(command.readConcern, { afterClusterTime: session.operationTime }); - } else if (session[kSnapshotEnabled]) { + } else if (session.snapshotEnabled) { command.readConcern = command.readConcern || { level: ReadConcernLevel.snapshot }; - if (session[kSnapshotTime] != null) { - Object.assign(command.readConcern, { atClusterTime: session[kSnapshotTime] }); + if (session.snapshotTime != null) { + Object.assign(command.readConcern, { atClusterTime: session.snapshotTime }); } } @@ -1187,12 +1170,12 @@ export function updateSessionFromResponse(session: ClientSession, document: Mong session.transaction._recoveryToken = document.recoveryToken; } - if (session?.[kSnapshotEnabled] && session[kSnapshotTime] == null) { + if (session?.snapshotEnabled && session.snapshotTime == null) { // find and aggregate commands return atClusterTime on the cursor // distinct includes it in the response body const atClusterTime = document.atClusterTime; if (atClusterTime) { - session[kSnapshotTime] = atClusterTime; + session.snapshotTime = atClusterTime; } } } diff --git a/test/integration/change-streams/change_stream.test.ts b/test/integration/change-streams/change_stream.test.ts index 9e171f0ee63..baabdcb3b23 100644 --- a/test/integration/change-streams/change_stream.test.ts +++ b/test/integration/change-streams/change_stream.test.ts @@ -22,13 +22,7 @@ import { type ResumeToken } from '../../mongodb'; import * as mock from '../../tools/mongodb-mock/index'; -import { - type FailPoint, - getSymbolFrom, - sleep, - TestBuilder, - UnifiedTestSuiteBuilder -} from '../../tools/utils'; +import { type FailPoint, sleep, TestBuilder, UnifiedTestSuiteBuilder } from '../../tools/utils'; import { delay, filterForCommands } from '../shared'; const initIteratorMode = async (cs: ChangeStream) => { @@ -719,7 +713,7 @@ describe('Change Streams', function () { }); describe('should error when used as iterator and emitter concurrently', function () { - let client, coll, changeStream, kMode; + let client, coll, changeStream; beforeEach(async function () { client = this.configuration.newClient(); @@ -727,7 +721,6 @@ describe('Change Streams', function () { coll = client.db(this.configuration.db).collection('tester'); changeStream = coll.watch(); - kMode = getSymbolFrom(changeStream, 'mode'); }); afterEach(async function () { @@ -738,14 +731,14 @@ describe('Change Streams', function () { it('should throw when mixing event listeners with iterator methods', { metadata: { requires: { topology: 'replicaset' } }, async test() { - expect(changeStream).to.have.property(kMode, false); + expect(changeStream).to.have.property('mode', false); changeStream.on('change', () => { // ChangeStream detects emitter usage via 'newListener' event // so this covers all emitter methods }); await once(changeStream.cursor, 'init'); - expect(changeStream).to.have.property(kMode, 'emitter'); + expect(changeStream).to.have.property('mode', 'emitter'); const errRegex = /ChangeStream cannot be used as an iterator/; @@ -764,10 +757,10 @@ describe('Change Streams', function () { metadata: { requires: { topology: 'replicaset' } }, async test() { await initIteratorMode(changeStream); - expect(changeStream).to.have.property(kMode, false); + expect(changeStream).to.have.property('mode', false); const res = await changeStream.tryNext(); expect(res).to.not.exist; - expect(changeStream).to.have.property(kMode, 'iterator'); + expect(changeStream).to.have.property('mode', 'iterator'); expect(() => { changeStream.on('change', () => { diff --git a/test/integration/connection-monitoring-and-pooling/connection.test.ts b/test/integration/connection-monitoring-and-pooling/connection.test.ts index 1192dfdbcd4..4307ee32f21 100644 --- a/test/integration/connection-monitoring-and-pooling/connection.test.ts +++ b/test/integration/connection-monitoring-and-pooling/connection.test.ts @@ -22,7 +22,7 @@ import { } from '../../mongodb'; import * as mock from '../../tools/mongodb-mock/index'; import { skipBrokenAuthTestBeforeEachHook } from '../../tools/runner/hooks/configuration'; -import { getSymbolFrom, sleep } from '../../tools/utils'; +import { sleep } from '../../tools/utils'; import { assert as test, setupDatabase } from '../shared'; const commonConnectOptions = { @@ -291,10 +291,9 @@ describe('Connection', function () { // Get the only connection const pool = [...client.topology.s.servers.values()][0].pool; - const connections = pool[getSymbolFrom(pool, 'connections')]; - expect(connections).to.have.lengthOf(1); + expect(pool.connections).to.have.lengthOf(1); - const connection = connections.first(); + const connection = pool.connections.first(); const socket: EventEmitter = connection.socket; // Spy on the socket event listeners diff --git a/test/integration/crud/misc_cursors.test.js b/test/integration/crud/misc_cursors.test.js index dacad6cc7e6..b8de060b6bc 100644 --- a/test/integration/crud/misc_cursors.test.js +++ b/test/integration/crud/misc_cursors.test.js @@ -13,7 +13,6 @@ const { setTimeout } = require('timers'); const { ReadPreference, MongoExpiredSessionError } = require('../../mongodb'); const { ServerType } = require('../../mongodb'); const { formatSort } = require('../../mongodb'); -const { getSymbolFrom } = require('../../tools/utils'); describe('Cursor', function () { before(function () { @@ -3643,8 +3642,7 @@ describe('Cursor', function () { expect(doc).to.exist; const clonedCursor = cursor.clone(); expect(clonedCursor.cursorOptions.session).to.not.exist; - const kServerSession = getSymbolFrom(clonedCursor.session, 'serverSession'); - expect(clonedCursor.session).to.have.property(kServerSession, null); // session is brand new and has not been used + expect(clonedCursor.session).to.have.property('_serverSession', null); // session is brand new and has not been used }) .finally(() => { return cursor.close(); @@ -3664,8 +3662,7 @@ describe('Cursor', function () { expect(doc).to.exist; const clonedCursor = cursor.clone(); expect(clonedCursor.cursorOptions.session).to.not.exist; - const kServerSession = getSymbolFrom(clonedCursor.session, 'serverSession'); - expect(clonedCursor.session).to.have.property(kServerSession, null); // session is brand new and has not been used + expect(clonedCursor.session).to.have.property('_serverSession', null); // session is brand new and has not been used }) .finally(() => { return cursor.close(); diff --git a/test/tools/utils.ts b/test/tools/utils.ts index 6ddf48d8b01..1829fd4412c 100644 --- a/test/tools/utils.ts +++ b/test/tools/utils.ts @@ -120,18 +120,6 @@ export function getEncryptExtraOptions(): { return {}; } -export function getSymbolFrom(target: any, symbolName: any, assertExists = true) { - const symbol = Object.getOwnPropertySymbols(target).filter( - s => s.toString() === `Symbol(${symbolName})` - )[0]; - - if (assertExists && !symbol) { - throw new Error(`Did not find Symbol(${symbolName}) on ${target}`); - } - - return symbol; -} - export function getEnvironmentalOptions() { const options = {}; if (process.env.MONGODB_API_VERSION) { diff --git a/test/unit/cmap/connection.test.ts b/test/unit/cmap/connection.test.ts index df3ea0f53ee..aa3e86e2dc6 100644 --- a/test/unit/cmap/connection.test.ts +++ b/test/unit/cmap/connection.test.ts @@ -15,7 +15,6 @@ import { SizedMessageTransform } from '../../mongodb'; import * as mock from '../../tools/mongodb-mock/index'; -import { getSymbolFrom } from '../../tools/utils'; const connectionOptionsDefaults = { id: 0, @@ -102,9 +101,7 @@ describe('new Connection()', function () { .command(ns('$admin.cmd'), { ping: 1 }, { socketTimeoutMS: 50 }) .catch(error => error); - const beforeHandshakeSymbol = getSymbolFrom(error, 'beforeHandshake', false); - expect(beforeHandshakeSymbol).to.be.a('symbol'); - expect(error).to.have.property(beforeHandshakeSymbol, false); + expect(error).to.have.property('beforeHandshake', false); }); it('calls the command function through command', async function () { @@ -143,9 +140,7 @@ describe('new Connection()', function () { const error = await connect(options).catch(error => error); - const beforeHandshakeSymbol = getSymbolFrom(error, 'beforeHandshake', false); - expect(beforeHandshakeSymbol).to.be.a('symbol'); - expect(error).to.have.property(beforeHandshakeSymbol, true); + expect(error).to.have.property('beforeHandshake', true); }); describe('NODE-6370: regression test', function () { diff --git a/test/unit/error.test.ts b/test/unit/error.test.ts index dca792bd382..9464209a929 100644 --- a/test/unit/error.test.ts +++ b/test/unit/error.test.ts @@ -38,7 +38,7 @@ import { } from '../mongodb'; import { ReplSetFixture } from '../tools/common'; import { cleanup } from '../tools/mongodb-mock/index'; -import { getSymbolFrom, topologyWithPlaceholderClient } from '../tools/utils'; +import { topologyWithPlaceholderClient } from '../tools/utils'; describe('MongoErrors', () => { let errorClassesFromEntryPoint = Object.fromEntries( @@ -291,19 +291,19 @@ describe('MongoErrors', () => { describe('when MongoNetworkError is constructed', () => { it('should only define beforeHandshake symbol if boolean option passed in', function () { const errorWithOptionTrue = new MongoNetworkError('', { beforeHandshake: true }); - expect(getSymbolFrom(errorWithOptionTrue, 'beforeHandshake', false)).to.be.a('symbol'); + expect(errorWithOptionTrue).to.have.property('beforeHandshake', true); const errorWithOptionFalse = new MongoNetworkError('', { beforeHandshake: false }); - expect(getSymbolFrom(errorWithOptionFalse, 'beforeHandshake', false)).to.be.a('symbol'); + expect(errorWithOptionFalse).to.have.property('beforeHandshake', false); const errorWithBadOption = new MongoNetworkError('', { // @ts-expect-error: beforeHandshake must be a boolean value beforeHandshake: 'not boolean' }); - expect(getSymbolFrom(errorWithBadOption, 'beforeHandshake', false)).to.be.an('undefined'); + expect(errorWithBadOption).to.not.have.property('beforeHandshake'); const errorWithoutOption = new MongoNetworkError(''); - expect(getSymbolFrom(errorWithoutOption, 'beforeHandshake', false)).to.be.an('undefined'); + expect(errorWithoutOption).to.not.have.property('beforeHandshake'); }); }); diff --git a/test/unit/mongo_client.test.ts b/test/unit/mongo_client.test.ts index 79663a26034..194721350ac 100644 --- a/test/unit/mongo_client.test.ts +++ b/test/unit/mongo_client.test.ts @@ -22,14 +22,8 @@ import { SeverityLevel, WriteConcern } from '../mongodb'; -import { getSymbolFrom } from '../tools/utils'; describe('MongoClient', function () { - it('MongoClient should always freeze public options', function () { - const client = new MongoClient('mongodb://localhost:27017'); - expect(client.options).to.be.frozen; - }); - it('programmatic options should override URI options', function () { const options = parseOptions('mongodb://localhost:27017/test?directConnection=true', { directConnection: false @@ -620,8 +614,8 @@ describe('MongoClient', function () { expect(clientOptions).to.have.property('monitorCommands', false); expect(client.s.options).to.have.property('monitorCommands', false); expect(client).to.have.property('monitorCommands', false); - const optionsSym = getSymbolFrom(client, 'options'); - expect(client[optionsSym]).to.have.property('monitorCommands', false); + + expect(client.options).to.have.property('monitorCommands', false); }); it('respects monitorCommands option passed in', function () { @@ -631,14 +625,13 @@ describe('MongoClient', function () { const testTable = [ [clientViaOpt, clientViaOpt.options], [clientViaUri, clientViaUri.options] - ]; + ] as const; for (const [client, clientOptions] of testTable) { expect(clientOptions).to.have.property('monitorCommands', true); expect(client.s.options).to.have.property('monitorCommands', true); expect(client).to.have.property('monitorCommands', true); - const optionsSym = getSymbolFrom(client, 'options'); - expect(client[optionsSym]).to.have.property('monitorCommands', true); + expect(client.options).to.have.property('monitorCommands', true); } }); }); diff --git a/test/unit/sdam/topology.test.ts b/test/unit/sdam/topology.test.ts index 5264b5d9c45..8b7be4b4f04 100644 --- a/test/unit/sdam/topology.test.ts +++ b/test/unit/sdam/topology.test.ts @@ -24,7 +24,7 @@ import { TopologyType } from '../../mongodb'; import * as mock from '../../tools/mongodb-mock/index'; -import { getSymbolFrom, topologyWithPlaceholderClient } from '../../tools/utils'; +import { topologyWithPlaceholderClient } from '../../tools/utils'; describe('Topology (unit)', function () { let client, topology; @@ -354,8 +354,7 @@ describe('Topology (unit)', function () { afterEach(() => { // The srv event starts a monitor that we need to clean up for (const [, server] of topology.s.servers) { - const kMonitorId = getSymbolFrom(server.monitor, 'monitorId'); - server.monitor[kMonitorId].stop(); + server.monitor.monitorId.stop(); } }); diff --git a/test/unit/sessions.test.ts b/test/unit/sessions.test.ts index 390b89233ab..128e1b08e4b 100644 --- a/test/unit/sessions.test.ts +++ b/test/unit/sessions.test.ts @@ -15,7 +15,6 @@ import { } from '../mongodb'; import { genClusterTime } from '../tools/common'; import * as mock from '../tools/mongodb-mock/index'; -import { getSymbolFrom } from '../tools/utils'; describe('Sessions - unit', function () { let client; @@ -253,24 +252,21 @@ describe('Sessions - unit', function () { it('should acquire a serverSession in the constructor if the session is explicit', () => { const session = new ClientSession(client, serverSessionPool, { explicit: true }); - const serverSessionSymbol = getSymbolFrom(session, 'serverSession'); - expect(session).to.have.property(serverSessionSymbol).that.is.an.instanceOf(ServerSession); + expect(session).to.have.property('_serverSession').that.is.an.instanceOf(ServerSession); }); it('should leave serverSession null if the session is implicit', () => { // implicit via false (this should not be allowed...) let session = new ClientSession(client, serverSessionPool, { explicit: false }); - const serverSessionSymbol = getSymbolFrom(session, 'serverSession'); - expect(session).to.have.property(serverSessionSymbol, null); + expect(session).to.have.property('_serverSession', null); // implicit via omission session = new ClientSession(client, serverSessionPool, {}); - expect(session).to.have.property(serverSessionSymbol, null); + expect(session).to.have.property('_serverSession', null); }); it('should start the txnNumberIncrement at zero', () => { const session = new ClientSession(client, serverSessionPool); - const txnNumberIncrementSymbol = getSymbolFrom(session, 'txnNumberIncrement'); - expect(session).to.have.property(txnNumberIncrementSymbol, 0); + expect(session).to.have.property('txnNumberIncrement', 0); }); describe('defaultTimeoutMS', function () { @@ -329,34 +325,25 @@ describe('Sessions - unit', function () { }); describe('get serverSession()', () => { - let serverSessionSymbol; - - before(() => { - serverSessionSymbol = getSymbolFrom( - new ClientSession(client, serverSessionPool, {}), - 'serverSession' - ); - }); - describe('from an explicit session', () => { it('should always have a non-null serverSession after construction', () => { const session = new ClientSession(client, serverSessionPool, { explicit: true }); - expect(session).to.have.a.property(serverSessionSymbol).be.an.instanceOf(ServerSession); + expect(session).to.have.a.property('_serverSession').be.an.instanceOf(ServerSession); expect(session.serverSession).be.an.instanceOf(ServerSession); }); it('should always have non-null serverSession even if it is ended before getter called', () => { const session = new ClientSession(client, serverSessionPool, { explicit: true }); session.hasEnded = true; - expect(session).to.have.a.property(serverSessionSymbol).be.an.instanceOf(ServerSession); + expect(session).to.have.a.property('_serverSession').be.an.instanceOf(ServerSession); expect(session.serverSession).be.an.instanceOf(ServerSession); }); it('should throw if the serverSession at the symbol property goes missing', () => { const session = new ClientSession(client, serverSessionPool, { explicit: true }); // We really want to make sure a ClientSession is not separated from its serverSession - session[serverSessionSymbol] = null; - expect(session).to.have.a.property(serverSessionSymbol).be.null; + session['_serverSession'] = null; + expect(session).to.have.a.property('_serverSession').be.null; expect(() => session.serverSession).throw(MongoRuntimeError); }); }); @@ -364,14 +351,14 @@ describe('Sessions - unit', function () { describe('from an implicit session', () => { it('should throw if the session ended before serverSession was acquired', () => { const session = new ClientSession(client, serverSessionPool, { explicit: false }); // make an implicit session - expect(session).to.have.property(serverSessionSymbol, null); + expect(session).to.have.property('_serverSession', null); session.hasEnded = true; expect(() => session.serverSession).to.throw(MongoRuntimeError); }); it('should acquire a serverSession if clientSession.hasEnded is false and serverSession is not set', () => { const session = new ClientSession(client, serverSessionPool, { explicit: false }); // make an implicit session - expect(session).to.have.property(serverSessionSymbol, null); + expect(session).to.have.property('_serverSession', null); session.hasEnded = false; const acquireSpy = sinon.spy(serverSessionPool, 'acquire'); expect(session.serverSession).to.be.instanceOf(ServerSession); @@ -381,7 +368,7 @@ describe('Sessions - unit', function () { it('should return the existing serverSession and not acquire a new one if one is already set', () => { const session = new ClientSession(client, serverSessionPool, { explicit: false }); // make an implicit session - expect(session).to.have.property(serverSessionSymbol, null); + expect(session).to.have.property('_serverSession', null); const acquireSpy = sinon.spy(serverSessionPool, 'acquire'); const firstServerSessionGetResult = session.serverSession; expect(firstServerSessionGetResult).to.be.instanceOf(ServerSession); @@ -404,7 +391,7 @@ describe('Sessions - unit', function () { it('should return the existing serverSession and not acquire a new one if one is already set and session is ended', () => { const session = new ClientSession(client, serverSessionPool, { explicit: false }); // make an implicit session - expect(session).to.have.property(serverSessionSymbol, null); + expect(session).to.have.property('_serverSession', null); const acquireSpy = sinon.spy(serverSessionPool, 'acquire'); const firstServerSessionGetResult = session.serverSession; expect(firstServerSessionGetResult).to.be.instanceOf(ServerSession); @@ -432,43 +419,38 @@ describe('Sessions - unit', function () { describe('incrementTransactionNumber()', () => { it('should not allocate serverSession', () => { const session = new ClientSession(client, serverSessionPool); - const txnNumberIncrementSymbol = getSymbolFrom(session, 'txnNumberIncrement'); session.incrementTransactionNumber(); - expect(session).to.have.property(txnNumberIncrementSymbol, 1); + expect(session).to.have.property('txnNumberIncrement', 1); - const serverSessionSymbol = getSymbolFrom(session, 'serverSession'); - expect(session).to.have.property(serverSessionSymbol, null); + expect(session).to.have.property('_serverSession', null); }); it('should save increments to txnNumberIncrement symbol', () => { const session = new ClientSession(client, serverSessionPool); - const txnNumberIncrementSymbol = getSymbolFrom(session, 'txnNumberIncrement'); session.incrementTransactionNumber(); session.incrementTransactionNumber(); session.incrementTransactionNumber(); - expect(session).to.have.property(txnNumberIncrementSymbol, 3); + expect(session).to.have.property('txnNumberIncrement', 3); }); }); describe('applySession()', () => { it('should allocate serverSession', () => { const session = new ClientSession(client, serverSessionPool); - const serverSessionSymbol = getSymbolFrom(session, 'serverSession'); const command = { magic: 1 }; const result = applySession(session, command, {}); expect(result).to.not.exist; expect(command).to.have.property('lsid'); - expect(session).to.have.property(serverSessionSymbol).that.is.instanceOf(ServerSession); + expect(session).to.have.property('_serverSession').that.is.instanceOf(ServerSession); }); it('should apply saved txnNumberIncrements', () => { const session = new ClientSession(client, serverSessionPool); - const serverSessionSymbol = getSymbolFrom(session, 'serverSession'); session.incrementTransactionNumber(); session.incrementTransactionNumber(); @@ -484,7 +466,7 @@ describe('Sessions - unit', function () { expect(command).to.have.property('lsid'); expect(command).to.have.property('txnNumber').instanceOf(Long); expect(command.txnNumber.toNumber()).to.equal(3); - expect(session).to.have.property(serverSessionSymbol).that.is.instanceOf(ServerSession); + expect(session).to.have.property('_serverSession').that.is.instanceOf(ServerSession); }); }); }); From ff0abd4cbf69601b309d411472214cee9d7dbb3b Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 11 Dec 2024 14:30:36 -0500 Subject: [PATCH 2/4] chore: comments --- src/cmap/connection_pool.ts | 2 ++ src/encrypter.ts | 2 -- src/error.ts | 23 ++++++++++------------- src/sdam/server.ts | 3 ++- src/sessions.ts | 14 +++++--------- test/unit/error.test.ts | 28 ++++++++++++++++------------ 6 files changed, 35 insertions(+), 37 deletions(-) diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index 97b9ff8fe72..bb2069de846 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -117,7 +117,9 @@ export type ConnectionPoolEvents = { */ export class ConnectionPool extends TypedEventEmitter { public options: Readonly; + /** An integer representing the SDAM generation of the pool */ public generation: number; + /** A map of generations to service ids */ public serviceGenerations: Map; private poolState: PoolState; diff --git a/src/encrypter.ts b/src/encrypter.ts index 06572fe2d0d..3c7bf2aaed6 100644 --- a/src/encrypter.ts +++ b/src/encrypter.ts @@ -57,7 +57,6 @@ export class Encrypter { } getInternalClient(client: MongoClient, uri: string, options: MongoClientOptions): MongoClient { - // TODO(NODE-4144): Remove new variable for type narrowing let internalClient = this.internalClient; if (internalClient == null) { const clonedOptions: MongoClientOptions = {}; @@ -92,7 +91,6 @@ export class Encrypter { } async connectInternalClient(): Promise { - // TODO(NODE-4144): Remove new variable for type narrowing const internalClient = this.internalClient; if (this.needsConnecting && internalClient != null) { this.needsConnecting = false; diff --git a/src/error.ts b/src/error.ts index 7c06a79d323..6d41087e3f5 100644 --- a/src/error.ts +++ b/src/error.ts @@ -125,7 +125,12 @@ 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 { - public readonly errorLabels: string[] = []; + /** @internal */ + private readonly errorLabelSet: Set = new Set(); + public get errorLabels(): string[] { + return Array.from(this.errorLabelSet); + } + /** * This is a number in MongoServerError and a string in MongoDriverError * @privateRemarks @@ -183,11 +188,11 @@ export class MongoError extends Error { * @returns returns true if the error has the provided error label */ hasErrorLabel(label: string): boolean { - return this.errorLabels.includes(label); + return this.errorLabelSet.has(label); } addErrorLabel(label: string): void { - if (!this.hasErrorLabel(label)) this.errorLabels.push(label); + this.errorLabelSet.add(label); } } @@ -1034,7 +1039,7 @@ export interface MongoNetworkErrorOptions { */ export class MongoNetworkError extends MongoError { /** @internal */ - private beforeHandshake?: boolean; + public readonly beforeHandshake: boolean; /** * **Do not use this constructor!** @@ -1049,20 +1054,12 @@ export class MongoNetworkError extends MongoError { **/ constructor(message: string, options?: MongoNetworkErrorOptions) { super(message, { cause: options?.cause }); - - if (options && typeof options.beforeHandshake === 'boolean') { - this.beforeHandshake = options.beforeHandshake; - } + this.beforeHandshake = !!options?.beforeHandshake; } override get name(): string { return 'MongoNetworkError'; } - - /** @internal */ - static isBeforeHandshake(err: MongoNetworkError): boolean { - return err.beforeHandshake === true; - } } /** diff --git a/src/sdam/server.ts b/src/sdam/server.ts index 0f8140c68ef..1aa19a3e18c 100644 --- a/src/sdam/server.ts +++ b/src/sdam/server.ts @@ -380,7 +380,8 @@ export class Server extends TypedEventEmitter { const isNetworkNonTimeoutError = error instanceof MongoNetworkError && !(error instanceof MongoNetworkTimeoutError); - const isNetworkTimeoutBeforeHandshakeError = MongoNetworkError.isBeforeHandshake(error); + const isNetworkTimeoutBeforeHandshakeError = + error instanceof MongoNetworkError && error.beforeHandshake; const isAuthHandshakeError = error.hasErrorLabel(MongoErrorLabel.HandshakeError); if (isNetworkNonTimeoutError || isNetworkTimeoutBeforeHandshakeError || isAuthHandshakeError) { // In load balanced mode we never mark the server as unknown and always diff --git a/src/sessions.ts b/src/sessions.ts index cb22538dd50..33260532ef3 100644 --- a/src/sessions.ts +++ b/src/sessions.ts @@ -174,15 +174,11 @@ export class ClientSession options = options ?? {}; - if (options.snapshot === true) { - this.snapshotEnabled = true; - if (options.causalConsistency === true) { - throw new MongoInvalidArgumentError( - 'Properties "causalConsistency" and "snapshot" are mutually exclusive' - ); - } - } else { - this.snapshotEnabled = false; + this.snapshotEnabled = options.snapshot === true; + if (options.causalConsistency === true && this.snapshotEnabled) { + throw new MongoInvalidArgumentError( + 'Properties "causalConsistency" and "snapshot" are mutually exclusive' + ); } this.client = client; diff --git a/test/unit/error.test.ts b/test/unit/error.test.ts index 9464209a929..8d63a86ca03 100644 --- a/test/unit/error.test.ts +++ b/test/unit/error.test.ts @@ -289,21 +289,25 @@ describe('MongoErrors', () => { }); describe('when MongoNetworkError is constructed', () => { - it('should only define beforeHandshake symbol if boolean option passed in', function () { - const errorWithOptionTrue = new MongoNetworkError('', { beforeHandshake: true }); - expect(errorWithOptionTrue).to.have.property('beforeHandshake', true); - - const errorWithOptionFalse = new MongoNetworkError('', { beforeHandshake: false }); - expect(errorWithOptionFalse).to.have.property('beforeHandshake', false); + describe('without options', () => { + it('sets beforeHandshake to false', () => { + const error = new MongoNetworkError('error'); + expect(error.beforeHandshake).to.be.false; + }); + }); - const errorWithBadOption = new MongoNetworkError('', { - // @ts-expect-error: beforeHandshake must be a boolean value - beforeHandshake: 'not boolean' + describe('with options', () => { + it('sets beforeHandshake to false if it is nullish or false', () => { + const error = new MongoNetworkError('error', {}); + expect(error.beforeHandshake).to.be.false; + const error2 = new MongoNetworkError('error', { beforeHandshake: false }); + expect(error2.beforeHandshake).to.be.false; }); - expect(errorWithBadOption).to.not.have.property('beforeHandshake'); - const errorWithoutOption = new MongoNetworkError(''); - expect(errorWithoutOption).to.not.have.property('beforeHandshake'); + it('sets beforeHandshake to true if it is set', () => { + const error = new MongoNetworkError('error', { beforeHandshake: true }); + expect(error.beforeHandshake).to.be.false; + }); }); }); From 3004589745f14e009a763fcc02ab43f1d07e7f37 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 11 Dec 2024 15:12:09 -0500 Subject: [PATCH 3/4] chore: fix closed --- src/change_stream.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/change_stream.ts b/src/change_stream.ts index 44d07b621d2..b7e45c70efc 100644 --- a/src/change_stream.ts +++ b/src/change_stream.ts @@ -794,7 +794,7 @@ export class ChangeStream< } async *[Symbol.asyncIterator](): AsyncGenerator { - if (this.isClosed) { + if (this.closed) { return; } @@ -843,7 +843,7 @@ export class ChangeStream< * @throws MongoChangeStreamError if the underlying cursor or the change stream is closed */ stream(options?: CursorStreamOptions): Readable & AsyncIterable { - if (this.isClosed) { + if (this.closed) { throw new MongoChangeStreamError(CHANGESTREAM_CLOSED_ERROR); } From 727ad4672f83a1bd8cc533e5cbc2dfe80b7620fa Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 11 Dec 2024 16:44:35 -0500 Subject: [PATCH 4/4] fix: test --- test/unit/error.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/error.test.ts b/test/unit/error.test.ts index 8d63a86ca03..9366816ba3a 100644 --- a/test/unit/error.test.ts +++ b/test/unit/error.test.ts @@ -306,7 +306,7 @@ describe('MongoErrors', () => { it('sets beforeHandshake to true if it is set', () => { const error = new MongoNetworkError('error', { beforeHandshake: true }); - expect(error.beforeHandshake).to.be.false; + expect(error.beforeHandshake).to.be.true; }); }); });