fix(logger): logger throws TypeError when log item has BigInt value (#…

…1201) * fix(logger): fix typeError when a log item has BigInt value, add tests * refactor(logger): add a test and rewrite removeEmptyKeys without pickby dependency * remove dependencies
aws-powertools · Dec 28, 2022 · a09e4df · a09e4df
1 parent b4cb3d9
commit a09e4df
Show file tree

Hide file tree

Showing 5 changed files with 131 additions and 94 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git 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"
@@ -47,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",
@@ -59,4 +57,4 @@
     "serverless",
     "nodejs"
   ]
-}
+}
diff --git 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/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
@@ -533,6 +533,68 @@ 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',
+          }));
+
+        });
+
+        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,
+          }));
+
+        });
+
       });
     });
 
@@ -765,34 +827,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',
+          },
+        },
+      }));
+    });
 
   });