From 05b5974376a0c83a250491b9df3b635ff5752c8b Mon Sep 17 00:00:00 2001 From: Sergei Cherniaev Date: Wed, 28 Dec 2022 15:56:19 +0400 Subject: [PATCH 1/3] fix(logger): fix typeError when a log item has BigInt value, add tests --- packages/logger/src/Logger.ts | 60 ++++++++-------- packages/logger/tests/unit/Logger.test.ts | 83 ++++++++++++++++------- 2 files changed, 92 insertions(+), 51 deletions(-) diff --git a/packages/logger/src/Logger.ts b/packages/logger/src/Logger.ts index 030cb2627b..a8ef03c8b5 100644 --- a/packages/logger/src/Logger.ts +++ b/packages/logger/src/Logger.ts @@ -568,6 +568,37 @@ class Logger extends Utility implements ClassThatLogs { return this.powertoolLogData; } + /** + * When the data added in the log item contains object references or BigInt values, + * `JSON.stringify()` can't handle them and instead throws errors: + * `TypeError: cyclic object value` or `TypeError: Do not know how to serialize a BigInt`. + * To mitigate these issues, this method will find and remove all cyclic references and convert BigInt values to strings. + * + * @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#exceptions + * @private + */ + private getReplacer(): (key: string, value: LogAttributes | Error | bigint) => void { + const references = new WeakSet(); + + return (key, value) => { + let item = value; + if (item instanceof Error) { + item = this.getLogFormatter().formatError(item); + } + if (typeof item === 'bigint') { + return item.toString(); + } + if (typeof item === 'object' && value !== null) { + if (references.has(item)) { + return; + } + references.add(item); + } + + return item; + }; + } + /** * It returns the numeric sample rate value. * @@ -605,7 +636,7 @@ class Logger extends Utility implements ClassThatLogs { const consoleMethod = logLevel.toLowerCase() as keyof ClassThatLogs; - this.console[consoleMethod](JSON.stringify(log.getAttributes(), this.removeCircularDependencies(), this.logIndentation)); + this.console[consoleMethod](JSON.stringify(log.getAttributes(), this.getReplacer(), this.logIndentation)); } /** @@ -622,33 +653,6 @@ class Logger extends Utility implements ClassThatLogs { this.printLog(logLevel, this.createAndPopulateLogItem(logLevel, input, extraInput)); } - /** - * When the data added in the log item contains object references, - * JSON.stringify() doesn't try to solve them and instead throws an error: TypeError: cyclic object value. - * To mitigate this issue, this method will find and remove all cyclic references. - * - * @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Cyclic_object_value - * @private - */ - private removeCircularDependencies(): (key: string, value: LogAttributes | Error) => void { - const references = new WeakSet(); - - return (key, value) => { - let item = value; - if (item instanceof Error) { - item = this.getLogFormatter().formatError(item); - } - if (typeof item === 'object' && value !== null) { - if (references.has(item)) { - return; - } - references.add(item); - } - - return item; - }; - } - /** * It initializes console property as an instance of the internal version of Console() class (PR #748) * or as the global node console if the `POWERTOOLS_DEV' env variable is set and has truthy value. diff --git a/packages/logger/tests/unit/Logger.test.ts b/packages/logger/tests/unit/Logger.test.ts index 03d7c7ae9b..7dc813c048 100644 --- a/packages/logger/tests/unit/Logger.test.ts +++ b/packages/logger/tests/unit/Logger.test.ts @@ -533,6 +533,44 @@ describe('Class: Logger', () => { }); + test('when a logged item has BigInt value, it doen\'t throw TypeError', () => { + + // Prepare + const logger = new Logger(); + jest.spyOn(logger['console'], methodOfLogger).mockImplementation(); + const message = `This is an ${methodOfLogger} log with BigInt value`; + const logItem = { value: BigInt(42) }; + const errorMessage = 'Do not know how to serialize a BigInt'; + + // Act & Assess + expect(() => { logger[methodOfLogger](message, logItem); }).not.toThrow(errorMessage); + + }); + + test('when a logged item has a BigInt value, it prints the log with value as a string', () => { + + // Prepare + const logger = new Logger(); + const consoleSpy = jest.spyOn(logger['console'], methodOfLogger).mockImplementation(); + const message = `This is an ${methodOfLogger} log with BigInt value`; + const logItem = { value: BigInt(42) }; + + // Act + logger[methodOfLogger](message, logItem); + + // Assess + expect(consoleSpy).toBeCalledTimes(1); + expect(consoleSpy).toHaveBeenNthCalledWith(1, JSON.stringify({ + level: methodOfLogger.toUpperCase(), + message: message, + service: 'hello-world', + timestamp: '2016-06-20T12:08:10.000Z', + xray_trace_id: '1-5759e988-bd862e3fe1be46a994272793', + value: '42', + })); + + }); + }); }); @@ -765,34 +803,33 @@ describe('Class: Logger', () => { })); }); - }); - - test('when called multiple times with the same keys, the outcome is the same', () => { - - // Prepare - const logger = new Logger(); - logger.appendKeys({ - aws_account_id: '123456789012', - aws_region: 'eu-west-1', - logger: { - name: 'aws-lambda-powertool-typescript', - version: '0.2.4', - }, - }); - - // Act - logger.removeKeys([ 'aws_account_id', 'aws_region' ]); - logger.removeKeys([ 'aws_account_id', 'aws_region' ]); + test('when called multiple times with the same keys, the outcome is the same', () => { - // Assess - expect(logger).toEqual(expect.objectContaining({ - persistentLogAttributes: { + // Prepare + const logger = new Logger(); + logger.appendKeys({ + aws_account_id: '123456789012', + aws_region: 'eu-west-1', logger: { name: 'aws-lambda-powertool-typescript', version: '0.2.4', }, - }, - })); + }); + + // Act + logger.removeKeys([ 'aws_account_id', 'aws_region' ]); + logger.removeKeys([ 'aws_account_id', 'aws_region' ]); + + // Assess + expect(logger).toEqual(expect.objectContaining({ + persistentLogAttributes: { + logger: { + name: 'aws-lambda-powertool-typescript', + version: '0.2.4', + }, + }, + })); + }); }); From 1041c4ed13527bbfc47a75db5affb82bb6e9b41a Mon Sep 17 00:00:00 2001 From: Sergei Cherniaev Date: Wed, 28 Dec 2022 16:13:53 +0400 Subject: [PATCH 2/3] refactor(logger): add a test and rewrite removeEmptyKeys without pickby dependency --- packages/logger/package.json | 5 ++--- packages/logger/src/log/LogItem.ts | 10 ++++++++-- packages/logger/tests/unit/Logger.test.ts | 24 +++++++++++++++++++++++ 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/packages/logger/package.json b/packages/logger/package.json index eaab9995ae..302aaa20a4 100644 --- a/packages/logger/package.json +++ b/packages/logger/package.json @@ -46,8 +46,7 @@ }, "dependencies": { "@aws-lambda-powertools/commons": "^1.5.0", - "lodash.merge": "^4.6.2", - "lodash.pickby": "^4.6.0" + "lodash.merge": "^4.6.2" }, "keywords": [ "aws", @@ -58,4 +57,4 @@ "serverless", "nodejs" ] -} \ No newline at end of file +} diff --git a/packages/logger/src/log/LogItem.ts b/packages/logger/src/log/LogItem.ts index bc35272c5e..68b27f1f86 100644 --- a/packages/logger/src/log/LogItem.ts +++ b/packages/logger/src/log/LogItem.ts @@ -1,4 +1,3 @@ -import pickBy from 'lodash.pickby'; import merge from 'lodash.merge'; import { LogItemInterface } from '.'; import { LogAttributes } from '../types'; @@ -31,7 +30,14 @@ class LogItem implements LogItemInterface { } public removeEmptyKeys(attributes: LogAttributes): LogAttributes { - return pickBy(attributes, (value) => value !== undefined && value !== '' && value !== null); + const newAttributes: LogAttributes = {}; + for (const key in attributes) { + if (attributes[key] !== undefined && attributes[key] !== '' && attributes[key] !== null) { + newAttributes[key] = attributes[key]; + } + } + + return newAttributes; } public setAttributes(attributes: LogAttributes): void { diff --git a/packages/logger/tests/unit/Logger.test.ts b/packages/logger/tests/unit/Logger.test.ts index 7dc813c048..42cb21062e 100644 --- a/packages/logger/tests/unit/Logger.test.ts +++ b/packages/logger/tests/unit/Logger.test.ts @@ -571,6 +571,30 @@ describe('Class: Logger', () => { }); + test('when a logged item has empty string, null, or undefined values, it removes it', () => { + + // Prepare + const logger = new Logger(); + const consoleSpy = jest.spyOn(logger['console'], methodOfLogger).mockImplementation(); + const message = `This is an ${methodOfLogger} log with empty, null, and undefined values`; + const logItem = { value: 42, emptyValue: '', undefinedValue: undefined, nullValue: null }; + + // Act + logger[methodOfLogger](message, logItem); + + // Assess + expect(consoleSpy).toBeCalledTimes(1); + expect(consoleSpy).toHaveBeenNthCalledWith(1, JSON.stringify({ + level: methodOfLogger.toUpperCase(), + message: message, + service: 'hello-world', + timestamp: '2016-06-20T12:08:10.000Z', + xray_trace_id: '1-5759e988-bd862e3fe1be46a994272793', + value: 42, + })); + + }); + }); }); From 49b4d733b97ade8d4b91a9c225eb0013a67b9a1d Mon Sep 17 00:00:00 2001 From: Sergei Cherniaev Date: Wed, 28 Dec 2022 17:48:07 +0400 Subject: [PATCH 3/3] remove dependencies --- package-lock.json | 40 ++++-------------------------------- packages/logger/package.json | 3 +-- 2 files changed, 5 insertions(+), 38 deletions(-) diff --git a/package-lock.json b/package-lock.json index d1c0efa359..deacb076cc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4625,15 +4625,6 @@ "@types/lodash": "*" } }, - "node_modules/@types/lodash.pickby": { - "version": "4.6.7", - "resolved": "https://registry.npmjs.org/@types/lodash.pickby/-/lodash.pickby-4.6.7.tgz", - "integrity": "sha512-4ebXRusuLflfscbD0PUX4eVknDHD9Yf+uMtBIvA/hrnTqeAzbuHuDjvnYriLjUrI9YrhCPVKUf4wkRSXJQ6gig==", - "dev": true, - "dependencies": { - "@types/lodash": "*" - } - }, "node_modules/@types/minimatch": { "version": "3.0.5", "resolved": "https://registry.npmjs.org/@types/minimatch/-/minimatch-3.0.5.tgz", @@ -11858,11 +11849,6 @@ "integrity": "sha512-GK3g5RPZWTRSeLSpgP8Xhra+pnjBC56q9FZYe1d5RN3TJ35dbkGy3YqBSMbyCrlbi+CM9Z3Jk5yTL7RCsqboyQ==", "dev": true }, - "node_modules/lodash.pickby": { - "version": "4.6.0", - "resolved": "https://registry.npmjs.org/lodash.pickby/-/lodash.pickby-4.6.0.tgz", - "integrity": "sha512-AZV+GsS/6ckvPOVQPXSiFFacKvKB4kOQu6ynt9wz0F3LO4R9Ij4K1ddYsIytDpSgLz88JHd9P+oaLeej5/Sl7Q==" - }, "node_modules/lodash.snakecase": { "version": "4.1.1", "resolved": "https://registry.npmjs.org/lodash.snakecase/-/lodash.snakecase-4.1.1.tgz", @@ -16462,12 +16448,10 @@ "license": "MIT", "dependencies": { "@aws-lambda-powertools/commons": "^1.5.0", - "lodash.merge": "^4.6.2", - "lodash.pickby": "^4.6.0" + "lodash.merge": "^4.6.2" }, "devDependencies": { - "@types/lodash.merge": "^4.6.7", - "@types/lodash.pickby": "^4.6.7" + "@types/lodash.merge": "^4.6.7" } }, "packages/metrics": { @@ -16754,9 +16738,7 @@ "requires": { "@aws-lambda-powertools/commons": "^1.5.0", "@types/lodash.merge": "^4.6.7", - "@types/lodash.pickby": "^4.6.7", - "lodash.merge": "^4.6.2", - "lodash.pickby": "^4.6.0" + "lodash.merge": "^4.6.2" } }, "@aws-lambda-powertools/metrics": { @@ -16788,7 +16770,7 @@ "@aws-lambda-powertools/commons": "^1.5.0", "@aws-sdk/client-dynamodb": "^3.231.0", "@types/promise-retry": "^1.1.3", - "aws-sdk": "*", + "aws-sdk": "^2.1276.0", "aws-xray-sdk-core": "^3.4.0", "axios": "^1.2.1", "promise-retry": "^2.0.1" @@ -20165,15 +20147,6 @@ "@types/lodash": "*" } }, - "@types/lodash.pickby": { - "version": "4.6.7", - "resolved": "https://registry.npmjs.org/@types/lodash.pickby/-/lodash.pickby-4.6.7.tgz", - "integrity": "sha512-4ebXRusuLflfscbD0PUX4eVknDHD9Yf+uMtBIvA/hrnTqeAzbuHuDjvnYriLjUrI9YrhCPVKUf4wkRSXJQ6gig==", - "dev": true, - "requires": { - "@types/lodash": "*" - } - }, "@types/minimatch": { "version": "3.0.5", "resolved": "https://registry.npmjs.org/@types/minimatch/-/minimatch-3.0.5.tgz", @@ -25924,11 +25897,6 @@ "integrity": "sha512-GK3g5RPZWTRSeLSpgP8Xhra+pnjBC56q9FZYe1d5RN3TJ35dbkGy3YqBSMbyCrlbi+CM9Z3Jk5yTL7RCsqboyQ==", "dev": true }, - "lodash.pickby": { - "version": "4.6.0", - "resolved": "https://registry.npmjs.org/lodash.pickby/-/lodash.pickby-4.6.0.tgz", - "integrity": "sha512-AZV+GsS/6ckvPOVQPXSiFFacKvKB4kOQu6ynt9wz0F3LO4R9Ij4K1ddYsIytDpSgLz88JHd9P+oaLeej5/Sl7Q==" - }, "lodash.snakecase": { "version": "4.1.1", "resolved": "https://registry.npmjs.org/lodash.snakecase/-/lodash.snakecase-4.1.1.tgz", diff --git a/packages/logger/package.json b/packages/logger/package.json index 6514ade194..aabdbc14ee 100644 --- a/packages/logger/package.json +++ b/packages/logger/package.json @@ -32,8 +32,7 @@ "types": "./lib/index.d.ts", "typedocMain": "src/index.ts", "devDependencies": { - "@types/lodash.merge": "^4.6.7", - "@types/lodash.pickby": "^4.6.7" + "@types/lodash.merge": "^4.6.7" }, "files": [ "lib"