Skip to content

Commit e9767a4

Browse files
EmilianoSanchezEmilianoSanchez
committed
Tests and polishing
1 parent fd8b306 commit e9767a4

File tree

6 files changed

+115
-33
lines changed

6 files changed

+115
-33
lines changed

‎src/logger/index.ts‎

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { objectAssign } from '../utils/lang/objectAssign';
22
import { ILoggerOptions, ILogger } from './types';
33
import { find, isObject } from '../utils/lang';
44
import SplitIO from '../../types/splitio';
5+
import { isLogger } from '../utils/settingsValidation/logger/commons';
56

67
export const LogLevels: SplitIO.ILoggerAPI['LogLevel'] = {
78
DEBUG: 'DEBUG',
@@ -43,61 +44,68 @@ const defaultOptions = {
4344
showLevel: true,
4445
};
4546

46-
const defaultConsoleLogger: SplitIO.Logger = {
47-
debug(message: string) { console.log(message); },
48-
info(message: string) { console.log(message); },
49-
warn(message: string) { console.log(message); },
50-
error(message: string) { console.log(message); }
51-
};
52-
5347
export class Logger implements ILogger {
5448

5549
private options: Required<ILoggerOptions>;
5650
private codes: Map<number, string>;
5751
private logLevel: number;
58-
private logger: SplitIO.Logger;
52+
private logger?: SplitIO.Logger;
5953

6054
constructor(options?: ILoggerOptions, codes?: Map<number, string>) {
6155
this.options = objectAssign({}, defaultOptions, options);
6256
this.codes = codes || new Map();
6357
this.logLevel = LogLevelIndexes[this.options.logLevel];
64-
this.logger = defaultConsoleLogger;
65-
}
66-
67-
setLogger(logger: SplitIO.Logger) {
68-
this.logger = logger;
6958
}
7059

7160
setLogLevel(logLevel: SplitIO.LogLevel) {
7261
this.options.logLevel = logLevel;
7362
this.logLevel = LogLevelIndexes[logLevel];
7463
}
7564

65+
setLogger(logger?: SplitIO.Logger) {
66+
if (!logger || isLogger(logger)) {
67+
this.logger = logger;
68+
} else {
69+
this._log(LogLevels.ERROR, 'Invalid `logger` instance. It must be an object with `debug`, `info`, `warn` and `error` methods. Defaulting to `console.log`');
70+
this.logger = undefined;
71+
}
72+
}
73+
7674
debug(msg: string | number, args?: any[]) {
77-
if (this._shouldLog(LogLevelIndexes.DEBUG)) this.logger.debug(this._log(LogLevels.DEBUG, msg, args));
75+
if (this._shouldLog(LogLevelIndexes.DEBUG)) this._log(LogLevels.DEBUG, msg, args);
7876
}
7977

8078
info(msg: string | number, args?: any[]) {
81-
if (this._shouldLog(LogLevelIndexes.INFO)) this.logger.info(this._log(LogLevels.INFO, msg, args));
79+
if (this._shouldLog(LogLevelIndexes.INFO)) this._log(LogLevels.INFO, msg, args);
8280
}
8381

8482
warn(msg: string | number, args?: any[]) {
85-
if (this._shouldLog(LogLevelIndexes.WARN)) this.logger.warn(this._log(LogLevels.WARN, msg, args));
83+
if (this._shouldLog(LogLevelIndexes.WARN)) this._log(LogLevels.WARN, msg, args);
8684
}
8785

8886
error(msg: string | number, args?: any[]) {
89-
if (this._shouldLog(LogLevelIndexes.ERROR)) this.logger.error(this._log(LogLevels.ERROR, msg, args));
87+
if (this._shouldLog(LogLevelIndexes.ERROR)) this._log(LogLevels.ERROR, msg, args);
9088
}
9189

92-
private _log(level: SplitIO.LogLevel, msg: string | number, args?: any[]): string {
90+
private _log(level: SplitIO.LogLevel, msg: string | number, args?: any[]) {
9391
if (typeof msg === 'number') {
9492
const format = this.codes.get(msg);
9593
msg = format ? _sprintf(format, args) : `Message code ${msg}${args ? ', with args: ' + args.toString() : ''}`;
9694
} else {
9795
if (args) msg = _sprintf(msg, args);
9896
}
9997

100-
return this._generateLogMessage(level, msg);
98+
const formattedText = this._generateLogMessage(level, msg);
99+
100+
// Do not break on custom logger errors
101+
if (this.logger) {
102+
try { // @ts-expect-error
103+
this.logger[level.toLowerCase()](formattedText);
104+
return;
105+
} catch (e) { /* empty */ }
106+
}
107+
108+
console.log(formattedText);
101109
}
102110

103111
private _generateLogMessage(level: SplitIO.LogLevel, text: string) {

‎src/logger/types.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ export interface ILoggerOptions {
77
}
88

99
export interface ILogger extends SplitIO.ILogger {
10-
setLogger(logger: SplitIO.Logger): void;
10+
setLogger(logger?: SplitIO.Logger): void;
1111

1212
debug(msg: any): void;
1313
debug(msg: string | number, args?: any[]): void;

‎src/utils/settingsValidation/logger/__tests__/index.spec.ts‎

Lines changed: 74 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ describe('logger validators', () => {
2020
const consoleLogSpy = jest.spyOn(global.console, 'log');
2121
afterEach(() => {
2222
consoleLogSpy.mockClear();
23+
customLogger.debug.mockClear();
24+
customLogger.info.mockClear();
25+
customLogger.warn.mockClear();
2326
customLogger.error.mockClear();
2427
});
2528

@@ -33,17 +36,16 @@ describe('logger validators', () => {
3336

3437
test.each(testTargets)('returns a NONE logger if `debug` property is invalid and logs the error', (validateLogger) => {
3538
expect(getLoggerLogLevel(validateLogger({ debug: null }))).toBe('NONE');
36-
expect(getLoggerLogLevel(validateLogger({ debug: 10 }))).toBe('NONE');
37-
expect(getLoggerLogLevel(validateLogger({ debug: {}, logger: customLogger }))).toBe('NONE');
39+
expect(getLoggerLogLevel(validateLogger({ debug: 10, logger: undefined }))).toBe('NONE'); // @ts-expect-error invalid `logger`, ignored because it's falsy
40+
expect(getLoggerLogLevel(validateLogger({ debug: {}, logger: false }))).toBe('NONE');
3841

3942
if (validateLogger === builtinValidateLogger) {
4043
// for builtinValidateLogger, a logger cannot be passed as `debug` property
4144
expect(getLoggerLogLevel(validateLogger({ debug: loggerMock }))).toBe('NONE');
42-
expect(consoleLogSpy).toBeCalledTimes(3);
45+
expect(consoleLogSpy).toBeCalledTimes(4);
4346
} else {
44-
expect(consoleLogSpy).toBeCalledTimes(2);
47+
expect(consoleLogSpy).toBeCalledTimes(3);
4548
}
46-
expect(customLogger.error).toBeCalledTimes(1);
4749
});
4850

4951
test.each(testTargets)('returns a logger with the provided log level if `debug` property is true or a string log level', (validateLogger) => {
@@ -63,4 +65,71 @@ describe('logger validators', () => {
6365
expect(consoleLogSpy).not.toBeCalled();
6466
});
6567

68+
test.each(testTargets)('uses the provided custom logger if it is valid', (validateLogger) => {
69+
const logger = validateLogger({ debug: true, logger: customLogger });
70+
71+
logger.debug('test debug');
72+
expect(customLogger.debug).toBeCalledWith('[DEBUG] splitio => test debug');
73+
74+
logger.info('test info');
75+
expect(customLogger.info).toBeCalledWith('[INFO] splitio => test info');
76+
77+
logger.warn('test warn');
78+
expect(customLogger.warn).toBeCalledWith('[WARN] splitio => test warn');
79+
80+
logger.error('test error');
81+
expect(customLogger.error).toBeCalledWith('[ERROR] splitio => test error');
82+
83+
expect(consoleLogSpy).not.toBeCalled();
84+
});
85+
86+
test.each(testTargets)('uses the default console.log method if the provided custom logger is not valid', (validateLogger) => {
87+
// @ts-expect-error `logger` property is not valid
88+
const logger = validateLogger({ debug: true, logger: {} });
89+
expect(consoleLogSpy).toBeCalledWith('[ERROR] splitio => Invalid `logger` instance. It must be an object with `debug`, `info`, `warn` and `error` methods. Defaulting to `console.log`');
90+
91+
logger.debug('test debug');
92+
expect(consoleLogSpy).toBeCalledWith('[DEBUG] splitio => test debug');
93+
94+
logger.info('test info');
95+
expect(consoleLogSpy).toBeCalledWith('[INFO] splitio => test info');
96+
97+
logger.warn('test warn');
98+
expect(consoleLogSpy).toBeCalledWith('[WARN] splitio => test warn');
99+
100+
logger.error('test error');
101+
expect(consoleLogSpy).toBeCalledWith('[ERROR] splitio => test error');
102+
103+
expect(consoleLogSpy).toBeCalledTimes(5);
104+
});
105+
106+
test.each(testTargets)('uses the default console.log method if the provided custom logger throws an error', (validateLogger) => {
107+
const customLoggerWithErrors = {
108+
debug: jest.fn(() => { throw new Error('debug error'); }),
109+
info: jest.fn(() => { throw new Error('info error'); }),
110+
warn: jest.fn(() => { throw new Error('warn error'); }),
111+
error: jest.fn(() => { throw new Error('error error'); })
112+
};
113+
114+
const logger = validateLogger({ debug: true, logger: customLoggerWithErrors });
115+
116+
logger.debug('test debug');
117+
expect(customLoggerWithErrors.debug).toBeCalledWith('[DEBUG] splitio => test debug');
118+
expect(consoleLogSpy).toBeCalledWith('[DEBUG] splitio => test debug');
119+
120+
logger.info('test info');
121+
expect(customLoggerWithErrors.info).toBeCalledWith('[INFO] splitio => test info');
122+
expect(consoleLogSpy).toBeCalledWith('[INFO] splitio => test info');
123+
124+
logger.warn('test warn');
125+
expect(customLoggerWithErrors.warn).toBeCalledWith('[WARN] splitio => test warn');
126+
expect(consoleLogSpy).toBeCalledWith('[WARN] splitio => test warn');
127+
128+
logger.error('test error');
129+
expect(customLoggerWithErrors.error).toBeCalledWith('[ERROR] splitio => test error');
130+
expect(consoleLogSpy).toBeCalledWith('[ERROR] splitio => test error');
131+
132+
expect(consoleLogSpy).toBeCalledTimes(4);
133+
});
134+
66135
});

‎src/utils/settingsValidation/logger/builtinLogger.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { ILogger } from '../../../logger/types';
33
import { isLocalStorageAvailable } from '../../env/isLocalStorageAvailable';
44
import { isNode } from '../../env/isNode';
55
import { codesDebug } from '../../../logger/messages/debug';
6-
import { getLogLevel, isLogger } from './commons';
6+
import { getLogLevel } from './commons';
77
import SplitIO from '../../../../types/splitio';
88

99
const allCodes = new Map(codesDebug);
@@ -40,16 +40,16 @@ if (/^(enabled?|on)/i.test(initialState)) {
4040
* @param settings - user config object, with an optional `debug` property of type boolean or string log level.
4141
* @returns a logger instance with the log level at `settings.debug`. If `settings.debug` is invalid or not provided, `initialLogLevel` is used.
4242
*/
43-
export function validateLogger(settings: { debug: unknown, logger?: unknown }): ILogger {
43+
export function validateLogger(settings: { debug: unknown, logger?: SplitIO.Logger }): ILogger {
4444
const { debug, logger } = settings;
4545

4646
const logLevel: SplitIO.LogLevel | undefined = debug !== undefined ? getLogLevel(debug) : initialLogLevel;
4747

4848
const log = new Logger({ logLevel: logLevel || initialLogLevel }, allCodes);
49-
if (isLogger(logger)) log.setLogger(logger);
49+
log.setLogger(logger);
5050

5151
// @ts-ignore // if logLevel is undefined at this point, it means that settings `debug` value is invalid
52-
if (!logLevel) log.logger.error(log._log(LogLevels.ERROR, 'Invalid Log Level - No changes to the logs will be applied.'));
52+
if (!logLevel) log._log(LogLevels.ERROR, 'Invalid Log Level - No changes to the logs will be applied.');
5353

5454
return log;
5555
}

‎src/utils/settingsValidation/logger/pluggableLogger.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,23 +17,23 @@ let initialLogLevel = LogLevels.NONE;
1717
* @returns a logger instance, that might be: the provided logger at `settings.debug`, or one with the given `debug` log level,
1818
* or one with NONE log level if `debug` is not defined or invalid.
1919
*/
20-
export function validateLogger(settings: { debug: unknown, logger?: unknown }): ILogger {
20+
export function validateLogger(settings: { debug: unknown, logger?: SplitIO.Logger }): ILogger {
2121
const { debug, logger } = settings;
2222
let logLevel: SplitIO.LogLevel | undefined = initialLogLevel;
2323

2424
if (debug !== undefined) {
2525
if (isILogger(debug)) {
26-
if (isLogger(logger)) debug.setLogger(logger);
26+
debug.setLogger(logger);
2727
return debug;
2828
}
2929
logLevel = getLogLevel(settings.debug);
3030
}
3131

3232
const log = new Logger({ logLevel: logLevel || initialLogLevel });
33-
if (isLogger(logger)) log.setLogger(logger);
33+
log.setLogger(logger);
3434

3535
// @ts-ignore // `debug` value is invalid if logLevel is undefined at this point
36-
if (!logLevel) log.logger.error(log._log(LogLevels.ERROR, 'Invalid `debug` value at config. Logs will be disabled.'));
36+
if (!logLevel) log._log(LogLevels.ERROR, 'Invalid `debug` value at config. Logs will be disabled.');
3737

3838
return log;
3939
}

‎types/splitio.d.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,9 @@ interface ISharedSettings {
9191
* Do not change these settings unless you're working an advanced use case, like connecting to the Split proxy.
9292
*/
9393
urls?: SplitIO.UrlSettings;
94+
/**
95+
* Custom logger object. If not provided, the SDK will use the default `console.log` method for all log levels.
96+
*/
9497
logger?: SplitIO.Logger;
9598
}
9699
/**
@@ -642,6 +645,8 @@ declare namespace SplitIO {
642645
disable(): void;
643646
/**
644647
* Sets a log level for the SDK logs.
648+
*
649+
* @param logLevel - The log level to set.
645650
*/
646651
setLogLevel(logLevel: LogLevel): void;
647652
/**

0 commit comments

Comments
 (0)