Skip to content

feat(NODE-4817)!: remove legacy logger #3518

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 23 commits into from
Jan 23, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
baea3b2
fix(NODE-4817): Remove legacy logger
W-A-James Jan 10, 2023
bf2e5b8
test(NODE-4817): Fix
W-A-James Jan 10, 2023
1888844
fix(NODE-4817): remove reference to logger
W-A-James Jan 11, 2023
06fe95a
fix(NODE-4817): Remove logger.ts
W-A-James Jan 13, 2023
b61d313
fix(NODE-4817): Remove reference to logger module
W-A-James Jan 13, 2023
f99a285
fix(NODE-4817): Add TODOS
W-A-James Jan 19, 2023
9ca43db
Merge branch 'main' into NODE-4817/remove-legacy-logger
W-A-James Jan 19, 2023
e4b7694
test(NODE-4817): Remove reference to LoggerLevel
W-A-James Jan 19, 2023
77a6d36
Merge branch 'NODE-4817/remove-legacy-logger' of github.com:mongodb/n…
W-A-James Jan 19, 2023
390b4c9
test(NODE-4817): Add todo comments
W-A-James Jan 19, 2023
cdc6c08
fix(NODE-4817): Remove unneeded code
W-A-James Jan 19, 2023
f84d4b3
Merge branch 'main' into NODE-4817/remove-legacy-logger
durran Jan 20, 2023
0799e26
fix(NODE-4817): Remove unneeded comments and code
W-A-James Jan 20, 2023
2ac3dfe
fix(NODE-4817): remove reference to deleted method
W-A-James Jan 20, 2023
8f79254
test(NODE-4817): remove reference to deleted method
W-A-James Jan 20, 2023
5a1d8fe
Merge branch 'main' into NODE-4817/remove-legacy-logger
W-A-James Jan 20, 2023
b0616e8
fix(NODE-4817): Remove unused code
W-A-James Jan 23, 2023
531f004
fix(NODE-4817): Change promise handler to unconditionally return null
W-A-James Jan 23, 2023
c8b865d
Merge branch 'main' into NODE-4817/remove-legacy-logger
W-A-James Jan 23, 2023
dd51e0f
fix(NODE-4817): remove unneeded argument in dummy promise error handler
W-A-James Jan 23, 2023
42c2daf
Merge branch 'NODE-4817/remove-legacy-logger' of github.com:mongodb/n…
W-A-James Jan 23, 2023
e64c821
style(NODE-4817): Remove TODO comment
W-A-James Jan 23, 2023
4f0a43c
style(NODE-4817): link to new ticket
W-A-James Jan 23, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix(NODE-4817): Remove unneeded code
  • Loading branch information
W-A-James committed Jan 19, 2023
commit cdc6c089bad5b501267411dbe19b86e2d938d887
71 changes: 0 additions & 71 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
38 changes: 1 addition & 37 deletions test/tools/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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<string, any[]>;
private _timeout: number;
Expand Down
221 changes: 0 additions & 221 deletions test/unit/assorted/deprecate_warning.test.js

This file was deleted.

24 changes: 0 additions & 24 deletions test/unit/sdam/srv_polling.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 () {
Expand Down