From baea3b22f7231c66e26108265c7198c16c150a4c Mon Sep 17 00:00:00 2001 From: Warren James Date: Tue, 10 Jan 2023 14:36:11 -0500 Subject: [PATCH 01/17] fix(NODE-4817): Remove legacy logger --- src/cmap/connection_pool.ts | 6 - src/collection.ts | 15 +- src/connection_string.ts | 19 -- src/db.ts | 15 +- src/gridfs/index.ts | 6 - src/index.ts | 4 - src/logger.ts | 243 ------------------ src/mongo_client.ts | 19 -- src/operations/command.ts | 6 - src/sdam/server.ts | 4 - src/sdam/srv_polling.ts | 22 +- test/integration/crud/bulk.test.ts | 2 + test/tools/utils.ts | 17 +- .../types/community/changes_from_36.test-d.ts | 9 +- test/types/community/client.test-d.ts | 1 - test/unit/assorted/deprecate_warning.test.js | 17 -- test/unit/index.test.ts | 2 - test/unit/mongo_client.test.js | 4 - test/unit/sdam/srv_polling.test.ts | 5 +- 19 files changed, 15 insertions(+), 401 deletions(-) diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index d6e19a7e3dc..ca3270db5c6 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -22,7 +22,6 @@ import { MongoRuntimeError, MongoServerError } from '../error'; -import { Logger } from '../logger'; import { CancellationToken, TypedEventEmitter } from '../mongo_types'; import type { Server } from '../sdam/server'; import { Callback, eachAsync, List, makeCounter } from '../utils'; @@ -52,8 +51,6 @@ import { ConnectionPoolMetrics } from './metrics'; /** @internal */ const kServer = Symbol('server'); /** @internal */ -const kLogger = Symbol('logger'); -/** @internal */ const kConnections = Symbol('connections'); /** @internal */ const kPending = Symbol('pending'); @@ -140,7 +137,6 @@ export class ConnectionPool extends TypedEventEmitter { options: Readonly; [kPoolState]: typeof PoolState[keyof typeof PoolState]; [kServer]: Server; - [kLogger]: Logger; [kConnections]: List; [kPending]: number; [kCheckedOut]: Set; @@ -239,7 +235,6 @@ export class ConnectionPool extends TypedEventEmitter { this[kPoolState] = PoolState.paused; this[kServer] = server; - this[kLogger] = new Logger('ConnectionPool'); this[kConnections] = new List(); this[kPending] = 0; this[kCheckedOut] = new Set(); @@ -642,7 +637,6 @@ export class ConnectionPool extends TypedEventEmitter { connect(connectOptions, (err, connection) => { if (err || !connection) { - this[kLogger].debug(`connection attempt failed with error [${JSON.stringify(err)}]`); this[kPending]--; this.emit( ConnectionPool.CONNECTION_CLOSED, diff --git a/src/collection.ts b/src/collection.ts index 7c6a37e6df1..23031d8900b 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -8,7 +8,6 @@ import { FindCursor } from './cursor/find_cursor'; import { ListIndexesCursor } from './cursor/list_indexes_cursor'; import type { Db } from './db'; import { MongoInvalidArgumentError } from './error'; -import type { Logger, LoggerOptions } from './logger'; import type { PkFactory } from './mongo_client'; import type { Filter, @@ -105,10 +104,7 @@ export interface ModifyResult { } /** @public */ -export interface CollectionOptions - extends BSONSerializeOptions, - WriteConcernOptions, - LoggerOptions { +export interface CollectionOptions extends BSONSerializeOptions, WriteConcernOptions { /** Specify a read concern for the collection. (only MongoDB 3.2 or higher supported) */ readConcern?: ReadConcernLike; /** The preferred read preference (ReadPreference.PRIMARY, ReadPreference.PRIMARY_PREFERRED, ReadPreference.SECONDARY, ReadPreference.SECONDARY_PREFERRED, ReadPreference.NEAREST). */ @@ -1535,15 +1531,6 @@ export class Collection { return new OrderedBulkOperation(this as TODO_NODE_3286, resolveOptions(this, options)); } - /** Get the db scoped logger */ - getLogger(): Logger { - return this.s.db.s.logger; - } - - get logger(): Logger { - return this.s.db.s.logger; - } - /** * An estimated count of matching documents in the db to a filter. * diff --git a/src/connection_string.ts b/src/connection_string.ts index 807457ee61b..d18bbf0eb6b 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -14,7 +14,6 @@ import { MongoMissingCredentialsError, MongoParseError } from './error'; -import { Logger as LegacyLogger, LoggerLevel as LegacyLoggerLevel } from './logger'; import { DriverInfo, MongoClient, @@ -870,24 +869,6 @@ export const OPTIONS = { default: 15, type: 'uint' }, - logger: { - default: new LegacyLogger('MongoClient'), - transform({ values: [value] }) { - if (value instanceof LegacyLogger) { - return value; - } - emitWarning('Alternative loggers might not be supported'); - // TODO: make Logger an interface that others can implement, make usage consistent in driver - // DRIVERS-1204 - return; - } - }, - loggerLevel: { - target: 'logger', - transform({ values: [value] }) { - return new LegacyLogger('MongoClient', { loggerLevel: value as LegacyLoggerLevel }); - } - }, maxConnecting: { default: 2, transform({ name, values: [value] }): number { diff --git a/src/db.ts b/src/db.ts index 29f97fb7723..c99e9def96d 100644 --- a/src/db.ts +++ b/src/db.ts @@ -6,7 +6,6 @@ import * as CONSTANTS from './constants'; import { AggregationCursor } from './cursor/aggregation_cursor'; import { ListCollectionsCursor } from './cursor/list_collections_cursor'; import { MongoAPIError, MongoInvalidArgumentError } from './error'; -import { Logger, LoggerOptions } from './logger'; import type { MongoClient, PkFactory } from './mongo_client'; import type { TODO_NODE_3286 } from './mongo_types'; import { AddUserOperation, AddUserOptions } from './operations/add_user'; @@ -79,7 +78,6 @@ const DB_OPTIONS_ALLOW_LIST = [ export interface DbPrivate { client: MongoClient; options?: DbOptions; - logger: Logger; readPreference?: ReadPreference; pkFactory: PkFactory; readConcern?: ReadConcern; @@ -89,7 +87,7 @@ export interface DbPrivate { } /** @public */ -export interface DbOptions extends BSONSerializeOptions, WriteConcernOptions, LoggerOptions { +export interface DbOptions extends BSONSerializeOptions, WriteConcernOptions { /** If the database authentication is dependent on another databaseName. */ authSource?: string; /** Force server to assign _id values instead of driver. */ @@ -159,8 +157,6 @@ export class Db { client, // Options options, - // Logger instance - logger: new Logger('Db', options), // Unpack read preference readPreference: ReadPreference.fromOptions(options), // Merge bson options @@ -756,15 +752,6 @@ export class Db { return new ChangeStream(this, pipeline, resolveOptions(this, options)); } - - /** Return the db logger */ - getLogger(): Logger { - return this.s.logger; - } - - get logger(): Logger { - return this.s.logger; - } } // TODO(NODE-3484): Refactor into MongoDBNamespace diff --git a/src/gridfs/index.ts b/src/gridfs/index.ts index 874762b0132..301df170fb5 100644 --- a/src/gridfs/index.ts +++ b/src/gridfs/index.ts @@ -3,7 +3,6 @@ import type { Collection } from '../collection'; import type { FindCursor } from '../cursor/find_cursor'; import type { Db } from '../db'; import { MongoRuntimeError } from '../error'; -import type { Logger } from '../logger'; import { Filter, TypedEventEmitter } from '../mongo_types'; import type { ReadPreference } from '../read_preference'; import type { Sort } from '../sort'; @@ -225,9 +224,4 @@ export class GridFSBucket extends TypedEventEmitter { await this.s._chunksCollection.drop(); }, callback); } - - /** Get the Db scoped logger. */ - getLogger(): Logger { - return this.s.db.s.logger; - } } diff --git a/src/index.ts b/src/index.ts index 0d0718b43b6..a3174b55fb4 100644 --- a/src/index.ts +++ b/src/index.ts @@ -12,7 +12,6 @@ import { Db } from './db'; import { GridFSBucket } from './gridfs'; import { GridFSBucketReadStream } from './gridfs/download'; import { GridFSBucketWriteStream } from './gridfs/upload'; -import { Logger } from './logger'; import { MongoClient } from './mongo_client'; import { CancellationToken } from './mongo_types'; import { ClientSession } from './sessions'; @@ -86,7 +85,6 @@ export { GridFSBucketWriteStream, ListCollectionsCursor, ListIndexesCursor, - Logger, MongoClient, OrderedBulkOperation, UnorderedBulkOperation @@ -101,7 +99,6 @@ export { CURSOR_FLAGS } from './cursor/abstract_cursor'; export { AutoEncryptionLoggerLevel } from './deps'; export { MongoErrorLabel } from './error'; export { ExplainVerbosity } from './explain'; -export { LoggerLevel } from './logger'; export { ServerApiVersion } from './mongo_client'; export { BSONType } from './mongo_types'; export { ReturnDocument } from './operations/find_and_modify'; @@ -270,7 +267,6 @@ export type { } from './gridfs/download'; export type { GridFSBucketEvents, GridFSBucketOptions, GridFSBucketPrivate } from './gridfs/index'; export type { GridFSBucketWriteStreamOptions, GridFSChunk } from './gridfs/upload'; -export type { LoggerFunction, LoggerOptions } from './logger'; export type { Auth, DriverInfo, diff --git a/src/logger.ts b/src/logger.ts index ecbbe9cbfb7..6902b1fbaf5 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -1,20 +1,3 @@ -import { format } from 'util'; - -import { MongoInvalidArgumentError } from './error'; -import { enumToString } from './utils'; - -// Filters for classes -const classFilters: any = {}; -let filteredClasses: any = {}; -let level: LoggerLevel; - -// Save the process id -const pid = process.pid; - -// current logger -// eslint-disable-next-line no-console -let currentLogger: LoggerFunction = console.warn; - /** @public */ export const LoggerLevel = Object.freeze({ ERROR: 'error', @@ -38,229 +21,3 @@ export interface LoggerOptions { logger?: LoggerFunction; loggerLevel?: LoggerLevel; } - -/** - * @public - * @deprecated This logger is unused and will be removed in the next major version. - */ -export class Logger { - className: string; - - /** - * Creates a new Logger instance - * - * @param className - The Class name associated with the logging instance - * @param options - Optional logging settings - */ - constructor(className: string, options?: LoggerOptions) { - options = options ?? {}; - - // Current reference - this.className = className; - - // Current logger - if (!(options.logger instanceof Logger) && typeof options.logger === 'function') { - currentLogger = options.logger; - } - - // Set level of logging, default is error - if (options.loggerLevel) { - level = options.loggerLevel || LoggerLevel.ERROR; - } - - // Add all class names - if (filteredClasses[this.className] == null) { - classFilters[this.className] = true; - } - } - - /** - * Log a message at the debug level - * - * @param message - The message to log - * @param object - Additional meta data to log - */ - debug(message: string, object?: unknown): void { - if ( - this.isDebug() && - ((Object.keys(filteredClasses).length > 0 && filteredClasses[this.className]) || - (Object.keys(filteredClasses).length === 0 && classFilters[this.className])) - ) { - const dateTime = new Date().getTime(); - const msg = format('[%s-%s:%s] %s %s', 'DEBUG', this.className, pid, dateTime, message); - const state = { - type: LoggerLevel.DEBUG, - message, - className: this.className, - pid, - date: dateTime - } as any; - - if (object) state.meta = object; - currentLogger(msg, state); - } - } - - /** - * Log a message at the warn level - * - * @param message - The message to log - * @param object - Additional meta data to log - */ - warn(message: string, object?: unknown): void { - if ( - this.isWarn() && - ((Object.keys(filteredClasses).length > 0 && filteredClasses[this.className]) || - (Object.keys(filteredClasses).length === 0 && classFilters[this.className])) - ) { - const dateTime = new Date().getTime(); - const msg = format('[%s-%s:%s] %s %s', 'WARN', this.className, pid, dateTime, message); - const state = { - type: LoggerLevel.WARN, - message, - className: this.className, - pid, - date: dateTime - } as any; - - if (object) state.meta = object; - currentLogger(msg, state); - } - } - - /** - * Log a message at the info level - * - * @param message - The message to log - * @param object - Additional meta data to log - */ - info(message: string, object?: unknown): void { - if ( - this.isInfo() && - ((Object.keys(filteredClasses).length > 0 && filteredClasses[this.className]) || - (Object.keys(filteredClasses).length === 0 && classFilters[this.className])) - ) { - const dateTime = new Date().getTime(); - const msg = format('[%s-%s:%s] %s %s', 'INFO', this.className, pid, dateTime, message); - const state = { - type: LoggerLevel.INFO, - message, - className: this.className, - pid, - date: dateTime - } as any; - - if (object) state.meta = object; - currentLogger(msg, state); - } - } - - /** - * Log a message at the error level - * - * @param message - The message to log - * @param object - Additional meta data to log - */ - error(message: string, object?: unknown): void { - if ( - this.isError() && - ((Object.keys(filteredClasses).length > 0 && filteredClasses[this.className]) || - (Object.keys(filteredClasses).length === 0 && classFilters[this.className])) - ) { - const dateTime = new Date().getTime(); - const msg = format('[%s-%s:%s] %s %s', 'ERROR', this.className, pid, dateTime, message); - const state = { - type: LoggerLevel.ERROR, - message, - className: this.className, - pid, - date: dateTime - } as any; - - if (object) state.meta = object; - currentLogger(msg, state); - } - } - - /** Is the logger set at info level */ - isInfo(): boolean { - return level === LoggerLevel.INFO || level === LoggerLevel.DEBUG; - } - - /** Is the logger set at error level */ - isError(): boolean { - return level === LoggerLevel.ERROR || level === LoggerLevel.INFO || level === LoggerLevel.DEBUG; - } - - /** Is the logger set at error level */ - isWarn(): boolean { - return ( - level === LoggerLevel.ERROR || - level === LoggerLevel.WARN || - level === LoggerLevel.INFO || - level === LoggerLevel.DEBUG - ); - } - - /** Is the logger set at debug level */ - isDebug(): boolean { - return level === LoggerLevel.DEBUG; - } - - /** Resets the logger to default settings, error and no filtered classes */ - static reset(): void { - level = LoggerLevel.ERROR; - filteredClasses = {}; - } - - /** Get the current logger function */ - static currentLogger(): LoggerFunction { - return currentLogger; - } - - /** - * Set the current logger function - * - * @param logger - Custom logging function - */ - static setCurrentLogger(logger: LoggerFunction): void { - if (typeof logger !== 'function') { - throw new MongoInvalidArgumentError('Current logger must be a function'); - } - - currentLogger = logger; - } - - /** - * Filter log messages for a particular class - * - * @param type - The type of filter (currently only class) - * @param values - The filters to apply - */ - static filter(type: string, values: string[]): void { - if (type === 'class' && Array.isArray(values)) { - filteredClasses = {}; - values.forEach(x => (filteredClasses[x] = true)); - } - } - - /** - * Set the current log level - * - * @param newLevel - Set current log level (debug, warn, info, error) - */ - static setLevel(newLevel: LoggerLevel): void { - if ( - newLevel !== LoggerLevel.INFO && - newLevel !== LoggerLevel.ERROR && - newLevel !== LoggerLevel.DEBUG && - newLevel !== LoggerLevel.WARN - ) { - throw new MongoInvalidArgumentError( - `Argument "newLevel" should be one of ${enumToString(LoggerLevel)}` - ); - } - - level = newLevel; - } -} diff --git a/src/mongo_client.ts b/src/mongo_client.ts index 63b46cf9ecb..a34767758e3 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -15,7 +15,6 @@ import { Db, DbOptions } from './db'; import type { AutoEncrypter, AutoEncryptionOptions } from './deps'; import type { Encrypter } from './encrypter'; import { MongoInvalidArgumentError } from './error'; -import type { Logger as LegacyLogger, LoggerLevel as LegacyLoggerLevel } from './logger'; import { MongoLogger, MongoLoggerOptions } from './mongo_logger'; import { TypedEventEmitter } from './mongo_types'; import type { ReadConcern, ReadConcernLevel, ReadConcernLike } from './read_concern'; @@ -228,10 +227,6 @@ export interface MongoClientOptions extends BSONSerializeOptions, SupportedNodeC forceServerObjectId?: boolean; /** A primary key factory function for generation of custom `_id` keys */ pkFactory?: PkFactory; - /** The logging level */ - loggerLevel?: LegacyLoggerLevel; - /** Custom logger object */ - logger?: LegacyLogger; /** Enable command monitoring for this client */ monitorCommands?: boolean; /** Server API version */ @@ -292,7 +287,6 @@ export interface MongoClientPrivate { readonly readConcern?: ReadConcern; readonly writeConcern?: WriteConcern; readonly readPreference: ReadPreference; - readonly logger: LegacyLogger; readonly isMongoClient: true; } @@ -369,9 +363,6 @@ export class MongoClient extends TypedEventEmitter { get readPreference() { return client[kOptions].readPreference; }, - get logger() { - return client[kOptions].logger; - }, get isMongoClient(): true { return true; } @@ -416,10 +407,6 @@ export class MongoClient extends TypedEventEmitter { return this.s.bsonOptions; } - get logger(): LegacyLogger { - return this.s.logger; - } - /** * Connect to MongoDB using a url * @@ -705,11 +692,6 @@ export class MongoClient extends TypedEventEmitter { return new ChangeStream(this, pipeline, resolveOptions(this, options)); } - - /** Return the mongo client logger */ - getLogger(): LegacyLogger { - return this.s.logger; - } } /** @@ -730,7 +712,6 @@ export interface MongoOptions | 'keepAlive' | 'keepAliveInitialDelay' | 'localThresholdMS' - | 'logger' | 'maxConnecting' | 'maxIdleTimeMS' | 'maxPoolSize' diff --git a/src/operations/command.ts b/src/operations/command.ts index 2464ccedd4a..ce2ab29e606 100644 --- a/src/operations/command.ts +++ b/src/operations/command.ts @@ -1,7 +1,6 @@ import type { BSONSerializeOptions, Document } from '../bson'; import { MongoInvalidArgumentError } from '../error'; import { Explain, ExplainOptions } from '../explain'; -import type { Logger } from '../logger'; import { ReadConcern } from '../read_concern'; import type { ReadPreference } from '../read_preference'; import type { Server } from '../sdam/server'; @@ -65,7 +64,6 @@ export interface OperationParent { readConcern?: ReadConcern; writeConcern?: WriteConcern; readPreference?: ReadPreference; - logger?: Logger; bsonOptions?: BSONSerializeOptions; } @@ -75,7 +73,6 @@ export abstract class CommandOperation extends AbstractOperation { readConcern?: ReadConcern; writeConcern?: WriteConcern; explain?: Explain; - logger?: Logger; constructor(parent?: OperationParent, options?: CommandOperationOptions) { super(options); @@ -97,9 +94,6 @@ export abstract class CommandOperation extends AbstractOperation { this.writeConcern = WriteConcern.fromOptions(options); // TODO(NODE-2056): make logger another "inheritable" property - if (parent && parent.logger) { - this.logger = parent.logger; - } if (this.hasAspect(Aspect.EXPLAINABLE)) { this.explain = Explain.fromOptions(options); diff --git a/src/sdam/server.ts b/src/sdam/server.ts index 21dcc23f97d..adb41b47653 100644 --- a/src/sdam/server.ts +++ b/src/sdam/server.ts @@ -35,7 +35,6 @@ import { MongoUnexpectedServerResponseError, needsRetryableWriteLabel } from '../error'; -import { Logger } from '../logger'; import type { ServerApi } from '../mongo_client'; import { TypedEventEmitter } from '../mongo_types'; import type { GetMoreOptions } from '../operations/get_more'; @@ -86,8 +85,6 @@ export interface ServerPrivate { description: ServerDescription; /** A copy of the options used to construct this instance */ options: ServerOptions; - /** A logger instance */ - logger: Logger; /** The current state of the Server */ state: string; /** The topology this server is a part of */ @@ -149,7 +146,6 @@ export class Server extends TypedEventEmitter { this.s = { description, options, - logger: new Logger('Server'), state: STATE_CLOSED, topology, pool: new ConnectionPool(this, poolOptions), diff --git a/src/sdam/srv_polling.ts b/src/sdam/srv_polling.ts index f9fc60193e3..edf49b9d7a3 100644 --- a/src/sdam/srv_polling.ts +++ b/src/sdam/srv_polling.ts @@ -2,7 +2,6 @@ import * as dns from 'dns'; import { clearTimeout, setTimeout } from 'timers'; import { MongoRuntimeError } from '../error'; -import { Logger, LoggerOptions } from '../logger'; import { TypedEventEmitter } from '../mongo_types'; import { HostAddress } from '../utils'; @@ -37,7 +36,7 @@ export class SrvPollingEvent { } /** @internal */ -export interface SrvPollerOptions extends LoggerOptions { +export interface SrvPollerOptions { srvServiceName: string; srvMaxHosts: number; srvHost: string; @@ -54,7 +53,6 @@ export class SrvPoller extends TypedEventEmitter { srvHost: string; rescanSrvIntervalMS: number; heartbeatFrequencyMS: number; - logger: Logger; haMode: boolean; generation: number; srvMaxHosts: number; @@ -76,7 +74,6 @@ export class SrvPoller extends TypedEventEmitter { this.srvServiceName = options.srvServiceName ?? 'mongodb'; this.rescanSrvIntervalMS = 60000; this.heartbeatFrequencyMS = options.heartbeatFrequencyMS ?? 10000; - this.logger = new Logger('srvPoller', options); this.haMode = false; this.generation = 0; @@ -112,9 +109,8 @@ export class SrvPoller extends TypedEventEmitter { } this._timeout = setTimeout(() => { - this._poll().catch(unexpectedRuntimeError => { - this.logger.error(`Unexpected ${new MongoRuntimeError(unexpectedRuntimeError).stack}`); - }); + // eslint-disable-next-line @typescript-eslint/no-empty-function, @typescript-eslint/no-unused-vars + this._poll().catch(_unexpectedRuntimeError => {}); }, this.intervalMS); } @@ -124,18 +120,14 @@ export class SrvPoller extends TypedEventEmitter { this.emit(SrvPoller.SRV_RECORD_DISCOVERY, new SrvPollingEvent(srvRecords)); } - failure(message: string, obj?: NodeJS.ErrnoException): void { - this.logger.warn(message, obj); + // eslint-disable-next-line @typescript-eslint/no-unused-vars + failure(_message: string, _obj?: NodeJS.ErrnoException): void { this.haMode = true; this.schedule(); } - parentDomainMismatch(srvRecord: dns.SrvRecord): void { - this.logger.warn( - `parent domain mismatch on SRV record (${srvRecord.name}:${srvRecord.port})`, - srvRecord - ); - } + // eslint-disable-next-line @typescript-eslint/no-empty-function, @typescript-eslint/no-unused-vars + parentDomainMismatch(_srvRecord: dns.SrvRecord): void {} async _poll(): Promise { const generation = this.generation; diff --git a/test/integration/crud/bulk.test.ts b/test/integration/crud/bulk.test.ts index 4e09d91fd13..064992a08c5 100644 --- a/test/integration/crud/bulk.test.ts +++ b/test/integration/crud/bulk.test.ts @@ -1846,6 +1846,8 @@ describe('Bulk', function () { let bulk = undefined; + let bulk = undefined; + bulk = collection.initializeOrderedBulkOp({ session }); bulk.insert({ answer: 42 }); await bulk.execute(); diff --git a/test/tools/utils.ts b/test/tools/utils.ts index 75be56c9f3b..d8deac20bf9 100644 --- a/test/tools/utils.ts +++ b/test/tools/utils.ts @@ -5,7 +5,7 @@ import { Readable } from 'stream'; import { setTimeout } from 'timers'; import { inspect, promisify } from 'util'; -import { deprecateOptions, DeprecateOptionsConfig, Document, Logger, OP_MSG } from '../mongodb'; +import { deprecateOptions, DeprecateOptionsConfig, Document, OP_MSG } from '../mongodb'; import { runUnifiedSuite } from './unified-spec-runner/runner'; import { CollectionData, @@ -28,21 +28,6 @@ export function ensureCalledWith(stub: any, args: any[]) { args.forEach((m: any) => expect(stub).to.have.been.calledWith(m)); } -// creation of class with a logger -export function ClassWithLogger() { - this.logger = new Logger('ClassWithLogger'); -} - -ClassWithLogger.prototype.f = makeTestFunction({ - name: 'f', - deprecatedOptions: ['maxScan', 'snapshot', 'fields'], - optionsIndex: 0 -}); - -ClassWithLogger.prototype.getLogger = function () { - return this.logger; -}; - // creation of class without a logger export function ClassWithoutLogger() { // empty function for class diff --git a/test/types/community/changes_from_36.test-d.ts b/test/types/community/changes_from_36.test-d.ts index d43c313097e..83f1e48358c 100644 --- a/test/types/community/changes_from_36.test-d.ts +++ b/test/types/community/changes_from_36.test-d.ts @@ -2,13 +2,7 @@ import type { PeerCertificate } from 'tls'; import { expectAssignable, expectError, expectNotType, expectType } from 'tsd'; -import { - LoggerLevel, - MongoClient, - MongoClientOptions, - ReadPreference, - ReadPreferenceMode -} from '../../../src'; +import { MongoClient, MongoClientOptions, ReadPreference, ReadPreferenceMode } from '../../../src'; import type { PropExists } from '../utility_types'; type MongoDBImport = typeof import('../../../src'); @@ -57,7 +51,6 @@ expectType>(false); expectType>(false); expectType(options.authSource); -expectType(options.loggerLevel); expectType(options.readPreference); expectType(options.promoteValues); expectType(options.family); diff --git a/test/types/community/client.test-d.ts b/test/types/community/client.test-d.ts index 5a067616f97..613bf6095db 100644 --- a/test/types/community/client.test-d.ts +++ b/test/types/community/client.test-d.ts @@ -18,7 +18,6 @@ export const connectionString = 'mongodb://127.0.0.1:27017/test'; const options: MongoClientOptions = { authSource: ' ', - loggerLevel: 'debug', w: 1, wtimeoutMS: 300, journal: true, diff --git a/test/unit/assorted/deprecate_warning.test.js b/test/unit/assorted/deprecate_warning.test.js index 7e8df0f1cf7..8345764e8fa 100644 --- a/test/unit/assorted/deprecate_warning.test.js +++ b/test/unit/assorted/deprecate_warning.test.js @@ -1,17 +1,14 @@ 'use strict'; const { deprecateOptions } = require('../../mongodb'); const { - ClassWithLogger, ClassWithoutLogger, ClassWithUndefinedLogger, - ensureCalledWith, makeTestFunction } = require('../../tools/utils'); const chai = require('chai'); const expect = chai.expect; -const sinon = require('sinon'); const sinonChai = require('sinon-chai'); require('mocha-sinon'); @@ -200,20 +197,6 @@ describe('Deprecation Warnings', function () { this.sinon.stub(console, 'error'); }); - it('test behavior for classes with an associated logger', function () { - const fakeClass = new ClassWithLogger(); - const logger = fakeClass.getLogger(); - const stub = sinon.stub(logger, 'warn'); - - fakeClass.f({ maxScan: 5, snapshot: true }); - fakeClass.f({ maxScan: 5, snapshot: true }); - expect(stub).to.have.been.calledTwice; - ensureCalledWith(stub, [ - 'f option [maxScan] is deprecated and will be removed in a later version.', - 'f option [snapshot] is deprecated and will be removed in a later version.' - ]); - }); - it('test behavior for classes without an associated logger', function () { const fakeClass = new ClassWithoutLogger(); diff --git a/test/unit/index.test.ts b/test/unit/index.test.ts index 197aaad6896..2aa6a9d8ac0 100644 --- a/test/unit/index.test.ts +++ b/test/unit/index.test.ts @@ -58,8 +58,6 @@ const EXPECTED_EXPORTS = [ 'Int32', 'ListCollectionsCursor', 'ListIndexesCursor', - 'Logger', - 'LoggerLevel', 'Long', 'Map', 'MaxKey', diff --git a/test/unit/mongo_client.test.js b/test/unit/mongo_client.test.js index 9256a761e67..b8978ad1541 100644 --- a/test/unit/mongo_client.test.js +++ b/test/unit/mongo_client.test.js @@ -7,7 +7,6 @@ const { parseOptions, resolveSRVRecord } = require('../mongodb'); const { ReadConcern } = require('../mongodb'); const { WriteConcern } = require('../mongodb'); const { ReadPreference } = require('../mongodb'); -const { Logger } = require('../mongodb'); const { MongoCredentials } = require('../mongodb'); const { MongoClient, MongoParseError, ServerApiVersion } = require('../../src'); const { MongoLogger } = require('../mongodb'); @@ -97,8 +96,6 @@ describe('MongoOptions', function () { keepAlive: true, keepAliveInitialDelay: 3, localThresholdMS: 3, - logger: new Logger('Testing!'), - loggerLevel: 'info', maxConnecting: 5, maxIdleTimeMS: 3, maxPoolSize: 2, @@ -615,7 +612,6 @@ describe('MongoOptions', function () { ['keepalive', true], ['keepaliveinitialdelay', 120000], ['localthresholdms', 15], - ['logger', doNotCheckEq], ['maxidletimems', 0], ['maxpoolsize', 100], ['minpoolsize', 0], diff --git a/test/unit/sdam/srv_polling.test.ts b/test/unit/sdam/srv_polling.test.ts index 55053af47ca..657707b774d 100644 --- a/test/unit/sdam/srv_polling.test.ts +++ b/test/unit/sdam/srv_polling.test.ts @@ -88,10 +88,9 @@ describe('Mongos SRV Polling', function () { expect(poller).to.have.property('haMode', true); }); - it('should do something if dns API breaks', async function () { + it.skip('should do something if dns API breaks', async function () { const poller = new SrvPoller({ srvHost: SRV_HOST, - loggerLevel: 'error', heartbeatFrequencyMS: 100 }); @@ -110,7 +109,7 @@ describe('Mongos SRV Polling', function () { expect(loggerError).to.have.been.calledOnceWith( sinon.match(/Unexpected MongoRuntimeError/) ); - }); + }).skipReason = 'TODO(NODE-4817): determine whether or not to keep this here'; }); describe('poll', function () { From bf2e5b8fb2b31fa166701152d59cd68d6f24d82c Mon Sep 17 00:00:00 2001 From: Warren James Date: Tue, 10 Jan 2023 15:37:29 -0500 Subject: [PATCH 02/17] test(NODE-4817): Fix --- test/integration/crud/bulk.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/integration/crud/bulk.test.ts b/test/integration/crud/bulk.test.ts index 064992a08c5..4e09d91fd13 100644 --- a/test/integration/crud/bulk.test.ts +++ b/test/integration/crud/bulk.test.ts @@ -1846,8 +1846,6 @@ describe('Bulk', function () { let bulk = undefined; - let bulk = undefined; - bulk = collection.initializeOrderedBulkOp({ session }); bulk.insert({ answer: 42 }); await bulk.execute(); From 1888844fd3418ac92c444dd1dfc71f1328166f4b Mon Sep 17 00:00:00 2001 From: Warren James Date: Wed, 11 Jan 2023 09:15:38 -0500 Subject: [PATCH 03/17] fix(NODE-4817): remove reference to logger --- src/db.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/db.ts b/src/db.ts index c99e9def96d..4133d491c5c 100644 --- a/src/db.ts +++ b/src/db.ts @@ -63,8 +63,6 @@ const DB_OPTIONS_ALLOW_LIST = [ 'readConcern', 'retryMiliSeconds', 'numberOfRetries', - 'loggerLevel', - 'logger', 'promoteBuffers', 'promoteLongs', 'bsonRegExp', From 06fe95ad73729338861c6f38f1bb7eaef798a63f Mon Sep 17 00:00:00 2001 From: Warren James Date: Fri, 13 Jan 2023 15:39:05 -0500 Subject: [PATCH 04/17] fix(NODE-4817): Remove logger.ts --- src/logger.ts | 23 ----------------------- 1 file changed, 23 deletions(-) delete mode 100644 src/logger.ts diff --git a/src/logger.ts b/src/logger.ts deleted file mode 100644 index 6902b1fbaf5..00000000000 --- a/src/logger.ts +++ /dev/null @@ -1,23 +0,0 @@ -/** @public */ -export const LoggerLevel = Object.freeze({ - ERROR: 'error', - WARN: 'warn', - INFO: 'info', - DEBUG: 'debug', - error: 'error', - warn: 'warn', - info: 'info', - debug: 'debug' -} as const); - -/** @public */ -export type LoggerLevel = typeof LoggerLevel[keyof typeof LoggerLevel]; - -/** @public */ -export type LoggerFunction = (message?: any, ...optionalParams: any[]) => void; - -/** @public */ -export interface LoggerOptions { - logger?: LoggerFunction; - loggerLevel?: LoggerLevel; -} From b61d31354f272310ba81c4df680ad01bbebf2ea3 Mon Sep 17 00:00:00 2001 From: Warren James Date: Fri, 13 Jan 2023 16:11:36 -0500 Subject: [PATCH 05/17] fix(NODE-4817): Remove reference to logger module --- test/mongodb.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/mongodb.ts b/test/mongodb.ts index cc037bb625a..9639586d9bb 100644 --- a/test/mongodb.ts +++ b/test/mongodb.ts @@ -67,7 +67,6 @@ export * from '../src/explain'; export * from '../src/gridfs/download'; export * from '../src/gridfs/index'; export * from '../src/gridfs/upload'; -export * from '../src/logger'; export * from '../src/mongo_client'; export * from '../src/mongo_logger'; export * from '../src/mongo_types'; From f99a285d81cb47fcb489a09fb40a3c349b223d88 Mon Sep 17 00:00:00 2001 From: Warren James Date: Thu, 19 Jan 2023 15:02:56 -0500 Subject: [PATCH 06/17] fix(NODE-4817): Add TODOS --- src/sdam/srv_polling.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/sdam/srv_polling.ts b/src/sdam/srv_polling.ts index edf49b9d7a3..3c3dec84f41 100644 --- a/src/sdam/srv_polling.ts +++ b/src/sdam/srv_polling.ts @@ -103,6 +103,7 @@ export class SrvPoller extends TypedEventEmitter { } } + // TODO(NODE-4817): This method has been left to allow for refactoring with the new logger schedule(): void { if (this._timeout) { clearTimeout(this._timeout); @@ -120,12 +121,16 @@ export class SrvPoller extends TypedEventEmitter { this.emit(SrvPoller.SRV_RECORD_DISCOVERY, new SrvPollingEvent(srvRecords)); } + // TODO(NODE-4817): This method has been left here to allow for refactoring with + // the new logger // eslint-disable-next-line @typescript-eslint/no-unused-vars failure(_message: string, _obj?: NodeJS.ErrnoException): void { this.haMode = true; this.schedule(); } + // TODO(NODE-4817): This method has been left here to allow for refactoring with + // the new logger // eslint-disable-next-line @typescript-eslint/no-empty-function, @typescript-eslint/no-unused-vars parentDomainMismatch(_srvRecord: dns.SrvRecord): void {} From e4b76941c6b2f60a7bfb2225953d980b9320bbf3 Mon Sep 17 00:00:00 2001 From: Warren James Date: Thu, 19 Jan 2023 15:32:57 -0500 Subject: [PATCH 07/17] test(NODE-4817): Remove reference to LoggerLevel --- test/types/enum.test-d.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/types/enum.test-d.ts b/test/types/enum.test-d.ts index aed57a509aa..108406b7c5d 100644 --- a/test/types/enum.test-d.ts +++ b/test/types/enum.test-d.ts @@ -10,7 +10,6 @@ import { CursorFlag, ExplainVerbosity, GSSAPICanonicalizationValue, - LoggerLevel, MongoErrorLabel, ProfilingLevel, ReadConcernLevel, @@ -37,7 +36,6 @@ expectType(Object.values(BatchType)[num]); expectType(Object.values(BSONType)[num]); expectType(Object.values(Compressor)[num]); expectType(Object.values(GSSAPICanonicalizationValue)[num]); -expectType(Object.values(LoggerLevel)[num]); expectType(Object.values(ProfilingLevel)[num]); expectType(Object.values(ReadConcernLevel)[num]); expectType(Object.values(ReadPreferenceMode)[num]); From 390b4c942061ccdd07085d8e5aaad0e9b22431e4 Mon Sep 17 00:00:00 2001 From: Warren James Date: Thu, 19 Jan 2023 15:45:09 -0500 Subject: [PATCH 08/17] test(NODE-4817): Add todo comments --- test/tools/utils.ts | 3 +++ test/unit/assorted/deprecate_warning.test.js | 1 + 2 files changed, 4 insertions(+) diff --git a/test/tools/utils.ts b/test/tools/utils.ts index d8deac20bf9..24793722505 100644 --- a/test/tools/utils.ts +++ b/test/tools/utils.ts @@ -28,6 +28,9 @@ export function ensureCalledWith(stub: any, args: any[]) { args.forEach((m: any) => expect(stub).to.have.been.calledWith(m)); } +// TODO(NODE-4817): Dependent on whehter or not we need the tests that use these in +// test/unit/assorted/deprecate_warning_test.js, we may be able to delete these classes + // creation of class without a logger export function ClassWithoutLogger() { // empty function for class diff --git a/test/unit/assorted/deprecate_warning.test.js b/test/unit/assorted/deprecate_warning.test.js index 8345764e8fa..49ca22770d8 100644 --- a/test/unit/assorted/deprecate_warning.test.js +++ b/test/unit/assorted/deprecate_warning.test.js @@ -192,6 +192,7 @@ describe('Deprecation Warnings', function () { }); }); + // TODO(NODE-4817): Do we need these tests? describe('Deprecation Warnings - functional', function () { beforeEach(function () { this.sinon.stub(console, 'error'); From cdc6c089bad5b501267411dbe19b86e2d938d887 Mon Sep 17 00:00:00 2001 From: Warren James Date: Thu, 19 Jan 2023 16:57:51 -0500 Subject: [PATCH 09/17] fix(NODE-4817): Remove unneeded code --- src/utils.ts | 71 ------ test/tools/utils.ts | 38 +--- test/unit/assorted/deprecate_warning.test.js | 221 ------------------- test/unit/sdam/srv_polling.test.ts | 24 -- 4 files changed, 1 insertion(+), 353 deletions(-) delete mode 100644 test/unit/assorted/deprecate_warning.test.js diff --git a/src/utils.ts b/src/utils.ts index 61fc08ef7a0..ada668027f9 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -311,77 +311,6 @@ export function defaultMsgHandler(name: string, option: string): string { return `${name} option [${option}] is deprecated and will be removed in a later version.`; } -export interface DeprecateOptionsConfig { - /** function name */ - name: string; - /** options to deprecate */ - deprecatedOptions: string[]; - /** index of options object in function arguments array */ - optionsIndex: number; - /** optional custom message handler to generate warnings */ - msgHandler?(this: void, name: string, option: string): string; -} - -/** - * Deprecates a given function's options. - * @internal - * - * @param this - the bound class if this is a method - * @param config - configuration for deprecation - * @param fn - the target function of deprecation - * @returns modified function that warns once per deprecated option, and executes original function - */ -export function deprecateOptions( - this: unknown, - config: DeprecateOptionsConfig, - fn: (...args: any[]) => any -): any { - if ((process as any).noDeprecation === true) { - return fn; - } - - const msgHandler = config.msgHandler ? config.msgHandler : defaultMsgHandler; - - const optionsWarned = new Set(); - function deprecated(this: any, ...args: any[]) { - const options = args[config.optionsIndex] as AnyOptions; - - // ensure options is a valid, non-empty object, otherwise short-circuit - if (!isObject(options) || Object.keys(options).length === 0) { - return fn.bind(this)(...args); // call the function, no change - } - - // interrupt the function call with a warning - for (const deprecatedOption of config.deprecatedOptions) { - if (deprecatedOption in options && !optionsWarned.has(deprecatedOption)) { - optionsWarned.add(deprecatedOption); - const msg = msgHandler(config.name, deprecatedOption); - emitWarning(msg); - if (this && 'getLogger' in this) { - const logger = this.getLogger(); - if (logger) { - logger.warn(msg); - } - } - } - } - - return fn.bind(this)(...args); - } - - // These lines copied from https://github.com/nodejs/node/blob/25e5ae41688676a5fd29b2e2e7602168eee4ceb5/lib/internal/util.js#L73-L80 - // The wrapper will keep the same prototype as fn to maintain prototype chain - Object.setPrototypeOf(deprecated, fn); - if (fn.prototype) { - // Setting this (rather than using Object.setPrototype, as above) ensures - // that calling the unwrapped constructor gives an instanceof the wrapped - // constructor. - deprecated.prototype = fn.prototype; - } - - return deprecated; -} - /** @internal */ export function ns(ns: string): MongoDBNamespace { return MongoDBNamespace.fromString(ns); diff --git a/test/tools/utils.ts b/test/tools/utils.ts index 24793722505..5124627309d 100644 --- a/test/tools/utils.ts +++ b/test/tools/utils.ts @@ -5,7 +5,7 @@ import { Readable } from 'stream'; import { setTimeout } from 'timers'; import { inspect, promisify } from 'util'; -import { deprecateOptions, DeprecateOptionsConfig, Document, OP_MSG } from '../mongodb'; +import { Document, OP_MSG } from '../mongodb'; import { runUnifiedSuite } from './unified-spec-runner/runner'; import { CollectionData, @@ -17,46 +17,10 @@ import { UnifiedSuite } from './unified-spec-runner/schema'; -export function makeTestFunction(config: DeprecateOptionsConfig) { - const fn = (options: any) => { - if (options) options = null; - }; - return deprecateOptions(config, fn); -} - export function ensureCalledWith(stub: any, args: any[]) { args.forEach((m: any) => expect(stub).to.have.been.calledWith(m)); } -// TODO(NODE-4817): Dependent on whehter or not we need the tests that use these in -// test/unit/assorted/deprecate_warning_test.js, we may be able to delete these classes - -// creation of class without a logger -export function ClassWithoutLogger() { - // empty function for class -} - -ClassWithoutLogger.prototype.f = makeTestFunction({ - name: 'f', - deprecatedOptions: ['maxScan', 'snapshot', 'fields'], - optionsIndex: 0 -}); - -// creation of class where getLogger returns undefined -export function ClassWithUndefinedLogger() { - // empty function for class -} - -ClassWithUndefinedLogger.prototype.f = makeTestFunction({ - name: 'f', - deprecatedOptions: ['maxScan', 'snapshot', 'fields'], - optionsIndex: 0 -}); - -ClassWithUndefinedLogger.prototype.getLogger = function () { - return undefined; -}; - export class EventCollector { private _events: Record; private _timeout: number; diff --git a/test/unit/assorted/deprecate_warning.test.js b/test/unit/assorted/deprecate_warning.test.js deleted file mode 100644 index 49ca22770d8..00000000000 --- a/test/unit/assorted/deprecate_warning.test.js +++ /dev/null @@ -1,221 +0,0 @@ -'use strict'; -const { deprecateOptions } = require('../../mongodb'); -const { - ClassWithoutLogger, - ClassWithUndefinedLogger, - makeTestFunction -} = require('../../tools/utils'); - -const chai = require('chai'); - -const expect = chai.expect; -const sinonChai = require('sinon-chai'); -require('mocha-sinon'); - -chai.use(sinonChai); - -describe('Deprecation Warnings', function () { - describe('Deprecation Warnings - unit', function () { - let messages = []; - const deprecatedOptions = ['maxScan', 'snapshot', 'fields']; - const defaultMessage = ' is deprecated and will be removed in a later version.'; - - before(function () { - if (process.emitWarning) { - process.on('warning', warning => { - messages.push(warning.message); - }); - } - return; - }); - - beforeEach(function () { - this.sinon.stub(console, 'error'); - }); - - afterEach(function () { - messages.length = 0; - }); - - describe('Mult functions with same options', function () { - beforeEach(function () { - const f1 = makeTestFunction({ - name: 'f1', - deprecatedOptions: deprecatedOptions, - optionsIndex: 0 - }); - const f2 = makeTestFunction({ - name: 'f2', - deprecatedOptions: deprecatedOptions, - optionsIndex: 0 - }); - f1({ maxScan: 5 }); - f2({ maxScan: 5 }); - }); - - it('multiple functions with the same deprecated options should both warn', done => { - process.nextTick(() => { - expect(messages).to.deep.equal([ - 'f1 option [maxScan]' + defaultMessage, - 'f2 option [maxScan]' + defaultMessage - ]); - expect(messages).to.have.a.lengthOf(2); - done(); - }); - }); - }); - - describe('Empty options object', function () { - beforeEach(function () { - const f = makeTestFunction({ - name: 'f', - deprecatedOptions: deprecatedOptions, - optionsIndex: 0 - }); - f({}); - }); - - it('should not warn if empty options object passed in', done => { - process.nextTick(() => { - expect(messages).to.have.a.lengthOf(0); - done(); - }); - }); - }); - - describe('Custom Message Handler', function () { - beforeEach(function () { - const customMsgHandler = (name, option) => { - return 'custom msg for function ' + name + ' and option ' + option; - }; - - const f = makeTestFunction({ - name: 'f', - deprecatedOptions: deprecatedOptions, - optionsIndex: 0, - msgHandler: customMsgHandler - }); - - f({ maxScan: 5, snapshot: true, fields: 'hi' }); - }); - - it('should use user-specified message handler', done => { - process.nextTick(() => { - expect(messages).to.deep.equal([ - 'custom msg for function f and option maxScan', - 'custom msg for function f and option snapshot', - 'custom msg for function f and option fields' - ]); - expect(messages).to.have.a.lengthOf(3); - done(); - }); - }); - }); - - describe('Warn once', function () { - beforeEach(function () { - const f = makeTestFunction({ - name: 'f', - deprecatedOptions: deprecatedOptions, - optionsIndex: 0 - }); - f({ maxScan: 5, fields: 'hi' }); - f({ maxScan: 5, fields: 'hi' }); - }); - - it('each function should only warn once per deprecated option', done => { - process.nextTick(() => { - expect(messages).to.deep.equal([ - 'f option [maxScan]' + defaultMessage, - 'f option [fields]' + defaultMessage - ]); - expect(messages).to.have.a.lengthOf(2); - done(); - }); - }); - }); - - describe('Maintain functionality', function () { - beforeEach(function () { - const config = { - name: 'f', - deprecatedOptions: ['multiply', 'add'], - optionsIndex: 0 - }; - - const operateBy2 = (options, num) => { - if (options.multiply === true) { - return num * 2; - } - if (options.add === true) { - return num + 2; - } - }; - - const f = deprecateOptions(config, operateBy2); - - const mult = f({ multiply: true }, 5); - const add = f({ add: true }, 5); - - expect(mult).to.equal(10); - expect(add).to.equal(7); - }); - - it('wrapped functions should maintain original functionality', done => { - process.nextTick(() => { - expect(messages).to.deep.equal([ - 'f option [multiply]' + defaultMessage, - 'f option [add]' + defaultMessage - ]); - expect(messages).to.have.a.lengthOf(2); - done(); - }); - }); - }); - - it('optionsIndex pointing to undefined should not error', () => { - const f = makeTestFunction({ - name: 'f', - deprecatedOptions: deprecatedOptions, - optionsIndex: 0 - }); - expect(f).to.not.throw(); - }); - - it('optionsIndex not pointing to object should not error', () => { - const f = makeTestFunction({ - name: 'f', - deprecatedOptions: deprecatedOptions, - optionsIndex: 0 - }); - expect(() => f('not-an-object')).to.not.throw(); - }); - }); - - // TODO(NODE-4817): Do we need these tests? - describe('Deprecation Warnings - functional', function () { - beforeEach(function () { - this.sinon.stub(console, 'error'); - }); - - it('test behavior for classes without an associated logger', function () { - const fakeClass = new ClassWithoutLogger(); - - function func() { - fakeClass.f({ maxScan: 5, snapshot: true }); - } - - expect(func).to.not.throw(); - }); - - it('test behavior for classes with an undefined logger', function () { - const fakeClass = new ClassWithUndefinedLogger(); - - function func() { - fakeClass.f({ maxScan: 5, snapshot: true }); - } - - expect(func).to.not.throw(); - }); - }); -}); diff --git a/test/unit/sdam/srv_polling.test.ts b/test/unit/sdam/srv_polling.test.ts index 657707b774d..d6947411cdc 100644 --- a/test/unit/sdam/srv_polling.test.ts +++ b/test/unit/sdam/srv_polling.test.ts @@ -13,7 +13,6 @@ import { TopologyType } from '../../mongodb'; import * as sdamEvents from '../../mongodb'; -import { sleep } from '../../tools/utils'; describe('Mongos SRV Polling', function () { const SRV_HOST = 'darmok.tanagra.com'; @@ -87,29 +86,6 @@ describe('Mongos SRV Polling', function () { expect(poller.schedule).to.have.been.calledOnce; expect(poller).to.have.property('haMode', true); }); - - it.skip('should do something if dns API breaks', async function () { - const poller = new SrvPoller({ - srvHost: SRV_HOST, - heartbeatFrequencyMS: 100 - }); - - // set haMode to make the poller use the 100ms heartbeat, otherwise this test would take 60 secs - poller.haMode = true; - - // @ts-expect-error: Testing what happens if node breaks DNS API - sinon.stub(dns.promises, 'resolveSrv').resolves(null); - - const loggerError = sinon.stub(poller.logger, 'error').returns(); - - poller.schedule(); - await sleep(130); - clearTimeout(poller._timeout); - - expect(loggerError).to.have.been.calledOnceWith( - sinon.match(/Unexpected MongoRuntimeError/) - ); - }).skipReason = 'TODO(NODE-4817): determine whether or not to keep this here'; }); describe('poll', function () { From 0799e26d41db1b1668dfaaa5c7bbdc7e9a4bd26c Mon Sep 17 00:00:00 2001 From: Warren James Date: Fri, 20 Jan 2023 14:13:08 -0500 Subject: [PATCH 10/17] fix(NODE-4817): Remove unneeded comments and code --- src/operations/command.ts | 2 -- src/sdam/srv_polling.ts | 12 ++---------- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/src/operations/command.ts b/src/operations/command.ts index ce2ab29e606..0aa1a69e0da 100644 --- a/src/operations/command.ts +++ b/src/operations/command.ts @@ -93,8 +93,6 @@ export abstract class CommandOperation extends AbstractOperation { this.readConcern = ReadConcern.fromOptions(options); this.writeConcern = WriteConcern.fromOptions(options); - // TODO(NODE-2056): make logger another "inheritable" property - if (this.hasAspect(Aspect.EXPLAINABLE)) { this.explain = Explain.fromOptions(options); } else if (options?.explain != null) { diff --git a/src/sdam/srv_polling.ts b/src/sdam/srv_polling.ts index 3c3dec84f41..ab1fd58483c 100644 --- a/src/sdam/srv_polling.ts +++ b/src/sdam/srv_polling.ts @@ -121,19 +121,11 @@ export class SrvPoller extends TypedEventEmitter { this.emit(SrvPoller.SRV_RECORD_DISCOVERY, new SrvPollingEvent(srvRecords)); } - // TODO(NODE-4817): This method has been left here to allow for refactoring with - // the new logger - // eslint-disable-next-line @typescript-eslint/no-unused-vars - failure(_message: string, _obj?: NodeJS.ErrnoException): void { + failure(): void { this.haMode = true; this.schedule(); } - // TODO(NODE-4817): This method has been left here to allow for refactoring with - // the new logger - // eslint-disable-next-line @typescript-eslint/no-empty-function, @typescript-eslint/no-unused-vars - parentDomainMismatch(_srvRecord: dns.SrvRecord): void {} - async _poll(): Promise { const generation = this.generation; let srvRecords; @@ -141,7 +133,7 @@ export class SrvPoller extends TypedEventEmitter { try { srvRecords = await dns.promises.resolveSrv(this.srvAddress); } catch (dnsError) { - this.failure('DNS error', dnsError); + this.failure(); return; } From 2ac3dfeebd739aead42511ecf52367d1ffbda291 Mon Sep 17 00:00:00 2001 From: Warren James Date: Fri, 20 Jan 2023 14:45:15 -0500 Subject: [PATCH 11/17] fix(NODE-4817): remove reference to deleted method --- src/sdam/srv_polling.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/sdam/srv_polling.ts b/src/sdam/srv_polling.ts index ab1fd58483c..32c1d5a3f0d 100644 --- a/src/sdam/srv_polling.ts +++ b/src/sdam/srv_polling.ts @@ -145,13 +145,11 @@ export class SrvPoller extends TypedEventEmitter { for (const record of srvRecords) { if (matchesParentDomain(record.name, this.srvHost)) { finalAddresses.push(record); - } else { - this.parentDomainMismatch(record); } } if (!finalAddresses.length) { - this.failure('No valid addresses found at host'); + this.failure(); return; } From 8f7925409fbb60a625ab3875813f6ee47524a4b0 Mon Sep 17 00:00:00 2001 From: Warren James Date: Fri, 20 Jan 2023 15:08:56 -0500 Subject: [PATCH 12/17] test(NODE-4817): remove reference to deleted method --- test/unit/sdam/srv_polling.test.ts | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/test/unit/sdam/srv_polling.test.ts b/test/unit/sdam/srv_polling.test.ts index d6947411cdc..ee6f2656318 100644 --- a/test/unit/sdam/srv_polling.test.ts +++ b/test/unit/sdam/srv_polling.test.ts @@ -45,7 +45,6 @@ describe('Mongos SRV Polling', function () { function stubPoller(poller) { sinon.stub(poller, 'success'); sinon.stub(poller, 'failure'); - sinon.stub(poller, 'parentDomainMismatch'); } it('should always return a valid value for `intervalMS`', function () { @@ -120,7 +119,6 @@ describe('Mongos SRV Polling', function () { expect(poller.success).to.not.have.been.called; expect(poller.failure).to.not.have.been.called; - expect(poller.parentDomainMismatch).to.not.have.been.called; }); it('should fail if dns returns error', async () => { @@ -132,8 +130,7 @@ describe('Mongos SRV Polling', function () { await poller._poll(); expect(poller.success).to.not.have.been.called; - expect(poller.failure).to.have.been.calledOnce.and.calledWith('DNS error'); - expect(poller.parentDomainMismatch).to.not.have.been.called; + expect(poller.failure).to.have.been.calledOnce; }); it('should fail if dns returns no records', async () => { @@ -145,10 +142,7 @@ describe('Mongos SRV Polling', function () { await poller._poll(); expect(poller.success).to.not.have.been.called; - expect(poller.failure).to.have.been.calledOnce.and.calledWith( - 'No valid addresses found at host' - ); - expect(poller.parentDomainMismatch).to.not.have.been.called; + expect(poller.failure).to.have.been.calledOnce; }); it('should fail if dns returns no records that match parent domain', async () => { @@ -161,12 +155,7 @@ describe('Mongos SRV Polling', function () { await poller._poll(); expect(poller.success).to.not.have.been.called; - expect(poller.failure).to.have.been.calledOnce.and.calledWith( - 'No valid addresses found at host' - ); - expect(poller.parentDomainMismatch) - .to.have.been.calledTwice.and.calledWith(records[0]) - .and.calledWith(records[1]); + expect(poller.failure).to.have.been.calledOnce; }); it('should succeed when valid records are returned by dns', async () => { @@ -180,7 +169,6 @@ describe('Mongos SRV Polling', function () { expect(poller.success).to.have.been.calledOnce.and.calledWithMatch(records); expect(poller.failure).to.not.have.been.called; - expect(poller.parentDomainMismatch).to.not.have.been.called; }); it('should succeed when some valid records are returned and some do not match parent domain', async () => { @@ -194,7 +182,6 @@ describe('Mongos SRV Polling', function () { expect(poller.success).to.have.been.calledOnce.and.calledWithMatch([records[0]]); expect(poller.failure).to.not.have.been.called; - expect(poller.parentDomainMismatch).to.have.been.calledOnce.and.calledWith(records[1]); }); }); }); From b0616e89bed571e0345c147844b2a0908b4d65bd Mon Sep 17 00:00:00 2001 From: Warren James Date: Mon, 23 Jan 2023 09:32:05 -0500 Subject: [PATCH 13/17] fix(NODE-4817): Remove unused code --- src/utils.ts | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index ada668027f9..95284613cd6 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -299,18 +299,6 @@ export function getTopology(provider: TopologyProvider): Topology { throw new MongoNotConnectedError('MongoClient must be connected to perform this operation'); } -/** - * Default message handler for generating deprecation warnings. - * @internal - * - * @param name - function name - * @param option - option name - * @returns warning message - */ -export function defaultMsgHandler(name: string, option: string): string { - return `${name} option [${option}] is deprecated and will be removed in a later version.`; -} - /** @internal */ export function ns(ns: string): MongoDBNamespace { return MongoDBNamespace.fromString(ns); From 531f004ea41bd88d3b18fbeb17fb323a9ce96e76 Mon Sep 17 00:00:00 2001 From: Warren James Date: Mon, 23 Jan 2023 10:00:47 -0500 Subject: [PATCH 14/17] fix(NODE-4817): Change promise handler to unconditionally return null --- src/sdam/srv_polling.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sdam/srv_polling.ts b/src/sdam/srv_polling.ts index 32c1d5a3f0d..4fd7b29ddeb 100644 --- a/src/sdam/srv_polling.ts +++ b/src/sdam/srv_polling.ts @@ -110,8 +110,8 @@ export class SrvPoller extends TypedEventEmitter { } this._timeout = setTimeout(() => { - // eslint-disable-next-line @typescript-eslint/no-empty-function, @typescript-eslint/no-unused-vars - this._poll().catch(_unexpectedRuntimeError => {}); + // eslint-disable-next-line @typescript-eslint/no-unused-vars + this._poll().catch(_unexpectedRuntimeError => null); }, this.intervalMS); } From dd51e0fdc74e3a7eb0c37b10b322b2d59379c2d9 Mon Sep 17 00:00:00 2001 From: Warren James Date: Mon, 23 Jan 2023 10:22:37 -0500 Subject: [PATCH 15/17] fix(NODE-4817): remove unneeded argument in dummy promise error handler --- src/sdam/srv_polling.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/sdam/srv_polling.ts b/src/sdam/srv_polling.ts index 4fd7b29ddeb..69918402da1 100644 --- a/src/sdam/srv_polling.ts +++ b/src/sdam/srv_polling.ts @@ -110,8 +110,7 @@ export class SrvPoller extends TypedEventEmitter { } this._timeout = setTimeout(() => { - // eslint-disable-next-line @typescript-eslint/no-unused-vars - this._poll().catch(_unexpectedRuntimeError => null); + this._poll().catch(() => null); }, this.intervalMS); } From e64c8211587505621bb87d2efc10937913117b75 Mon Sep 17 00:00:00 2001 From: Warren James Date: Mon, 23 Jan 2023 11:48:51 -0500 Subject: [PATCH 16/17] style(NODE-4817): Remove TODO comment --- src/sdam/srv_polling.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/sdam/srv_polling.ts b/src/sdam/srv_polling.ts index 69918402da1..9fe01e57fab 100644 --- a/src/sdam/srv_polling.ts +++ b/src/sdam/srv_polling.ts @@ -103,7 +103,6 @@ export class SrvPoller extends TypedEventEmitter { } } - // TODO(NODE-4817): This method has been left to allow for refactoring with the new logger schedule(): void { if (this._timeout) { clearTimeout(this._timeout); From 4f0a43c70f967c2925914fe52c5bfb99bc2b2529 Mon Sep 17 00:00:00 2001 From: Warren James Date: Mon, 23 Jan 2023 13:01:24 -0500 Subject: [PATCH 17/17] style(NODE-4817): link to new ticket --- src/sdam/srv_polling.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/sdam/srv_polling.ts b/src/sdam/srv_polling.ts index 9fe01e57fab..253f3a74d23 100644 --- a/src/sdam/srv_polling.ts +++ b/src/sdam/srv_polling.ts @@ -103,6 +103,7 @@ export class SrvPoller extends TypedEventEmitter { } } + // TODO(NODE-4994): implement new logging logic for SrvPoller failures schedule(): void { if (this._timeout) { clearTimeout(this._timeout);