Merge pull request #1332 from log4js-node/fix/serde

fix(LoggingEvent): serde for `NaN`, `Infinity`, `-Infinity`, `undefined`
log4js-node · Oct 1, 2022 · 570ef53 · 570ef53
2 parents 916eef1 + ffa9e5e
commit 570ef53
Show file tree

Hide file tree

Showing 5 changed files with 100 additions and 25 deletions.
diff --git a/lib/LoggingEvent.js b/lib/LoggingEvent.js
@@ -1,6 +1,45 @@
+/* eslint max-classes-per-file: ["error", 2] */
+
 const flatted = require('flatted');
 const levels = require('./levels');
 
+class SerDe {
+  constructor() {
+    const deserialise = {
+      __LOG4JS_undefined__: undefined,
+      __LOG4JS_NaN__: Number('abc'),
+      __LOG4JS_Infinity__: 1 / 0,
+      '__LOG4JS_-Infinity__': -1 / 0,
+    };
+    this.deMap = deserialise;
+    this.serMap = {};
+    Object.keys(this.deMap).forEach((key) => {
+      const value = this.deMap[key];
+      this.serMap[value] = key;
+    });
+  }
+
+  canSerialise(key) {
+    if (typeof key === 'string') return false;
+    return key in this.serMap;
+  }
+
+  serialise(key) {
+    if (this.canSerialise(key)) return this.serMap[key];
+    return key;
+  }
+
+  canDeserialise(key) {
+    return key in this.deMap;
+  }
+
+  deserialise(key) {
+    if (this.canDeserialise(key)) return this.deMap[key];
+    return key;
+  }
+}
+const serde = new SerDe();
+
 /**
  * @name LoggingEvent
  * @namespace Log4js
@@ -47,20 +86,11 @@ class LoggingEvent {
           value
         );
       }
-      // JSON.stringify({a: parseInt('abc'), b: 1/0, c: -1/0}) returns {a: null, b: null, c: null}.
+      // JSON.stringify({a: Number('abc'), b: 1/0, c: -1/0}) returns {a: null, b: null, c: null}.
       // The following allows us to serialize to NaN, Infinity and -Infinity correctly.
-      else if (
-        typeof value === 'number' &&
-        (Number.isNaN(value) || !Number.isFinite(value))
-      ) {
-        value = value.toString();
-      }
       // JSON.stringify([undefined]) returns [null].
       // The following allows us to serialize to undefined correctly.
-      else if (typeof value === 'undefined') {
-        value = typeof value;
-      }
-      return value;
+      return serde.serialise(value);
     });
   }
 
@@ -75,7 +105,7 @@ class LoggingEvent {
           });
           value = fakeError;
         }
-        return value;
+        return serde.deserialise(value);
       });
       if (
         rehydratedEvent.fileName ||

diff --git a/lib/connect-logger.js b/lib/connect-logger.js
@@ -251,7 +251,7 @@ module.exports = function getLogger(logger4js, options) {
 
   return (req, res, next) => {
     // mount safety
-    if (req._logging !== undefined) return next();
+    if (typeof req._logging !== 'undefined') return next();
 
     // nologs
     if (typeof options.nolog !== 'function') {

diff --git a/lib/logger.js b/lib/logger.js
@@ -210,7 +210,7 @@ class Logger {
   setParseCallStackFunction(parseFunction) {
     if (typeof parseFunction === 'function') {
       this.parseCallStack = parseFunction;
-    } else if (parseFunction === undefined) {
+    } else if (typeof parseFunction === 'undefined') {
       this.parseCallStack = defaultParseCallStack;
     } else {
       throw new TypeError('Invalid type passed to setParseCallStackFunction');

diff --git a/test/tap/LoggingEvent-test.js b/test/tap/LoggingEvent-test.js
@@ -8,7 +8,17 @@ test('LoggingEvent', (batch) => {
     const event = new LoggingEvent(
       'cheese',
       levels.DEBUG,
-      ['log message', parseInt('abc', 10), 1 / 0, -1 / 0, undefined],
+      [
+        'log message',
+        Number('abc'),
+        'NaN',
+        1 / 0,
+        'Infinity',
+        -1 / 0,
+        '-Infinity',
+        undefined,
+        'undefined',
+      ],
       {
         user: 'bob',
       }
@@ -19,12 +29,16 @@ test('LoggingEvent', (batch) => {
     t.equal(rehydratedEvent.startTime, '2018-02-04T18:30:23.010Z');
     t.equal(rehydratedEvent.categoryName, 'cheese');
     t.equal(rehydratedEvent.level.levelStr, 'DEBUG');
-    t.equal(rehydratedEvent.data.length, 5);
+    t.equal(rehydratedEvent.data.length, 9);
     t.equal(rehydratedEvent.data[0], 'log message');
-    t.equal(rehydratedEvent.data[1], 'NaN');
-    t.equal(rehydratedEvent.data[2], 'Infinity');
-    t.equal(rehydratedEvent.data[3], '-Infinity');
-    t.equal(rehydratedEvent.data[4], 'undefined');
+    t.equal(rehydratedEvent.data[1], '__LOG4JS_NaN__');
+    t.equal(rehydratedEvent.data[2], 'NaN');
+    t.equal(rehydratedEvent.data[3], '__LOG4JS_Infinity__');
+    t.equal(rehydratedEvent.data[4], 'Infinity');
+    t.equal(rehydratedEvent.data[5], '__LOG4JS_-Infinity__');
+    t.equal(rehydratedEvent.data[6], '-Infinity');
+    t.equal(rehydratedEvent.data[7], '__LOG4JS_undefined__');
+    t.equal(rehydratedEvent.data[8], 'undefined');
     t.equal(rehydratedEvent.context.user, 'bob');
     t.end();
   });
@@ -36,7 +50,18 @@ test('LoggingEvent', (batch) => {
       level: {
         levelStr: 'INFO',
       },
-      data: ['some log message', { x: 1 }],
+      data: [
+        'some log message',
+        { x: 1 },
+        '__LOG4JS_NaN__',
+        'NaN',
+        '__LOG4JS_Infinity__',
+        'Infinity',
+        '__LOG4JS_-Infinity__',
+        '-Infinity',
+        '__LOG4JS_undefined__',
+        'undefined',
+      ],
       context: { thing: 'otherThing' },
       pid: '1234',
       functionName: 'bound',
@@ -50,8 +75,17 @@ test('LoggingEvent', (batch) => {
     t.same(event.startTime, new Date(Date.UTC(2018, 1, 4, 10, 25, 23, 10)));
     t.equal(event.categoryName, 'biscuits');
     t.same(event.level, levels.INFO);
+    t.equal(event.data.length, 10);
     t.equal(event.data[0], 'some log message');
     t.equal(event.data[1].x, 1);
+    t.ok(Number.isNaN(event.data[2]));
+    t.equal(event.data[3], 'NaN');
+    t.equal(event.data[4], 1 / 0);
+    t.equal(event.data[5], 'Infinity');
+    t.equal(event.data[6], -1 / 0);
+    t.equal(event.data[7], '-Infinity');
+    t.equal(event.data[8], undefined);
+    t.equal(event.data[9], 'undefined');
     t.equal(event.context.thing, 'otherThing');
     t.equal(event.pid, '1234');
     t.equal(event.functionName, 'bound');
@@ -167,7 +201,18 @@ test('LoggingEvent', (batch) => {
     const event = new LoggingEvent(
       'cheese',
       levels.DEBUG,
-      [error, 'log message'],
+      [
+        error,
+        'log message',
+        Number('abc'),
+        'NaN',
+        1 / 0,
+        'Infinity',
+        -1 / 0,
+        '-Infinity',
+        undefined,
+        'undefined',
+      ],
       {
         user: 'bob',
       },

diff --git a/test/tap/logLevelFilter-test.js b/test/tap/logLevelFilter-test.js
@@ -110,7 +110,7 @@ test('log4js logLevelFilter', (batch) => {
     logger.trace('trace');
     // wait for the file system to catch up
     setTimeout(() => {
-      t.test('tmp-tests.log should contain all log messages', (assert) => {
+      t.test('logLevelFilter.log should contain all log messages', (assert) => {
         fs.readFile(
           `${__dirname}/logLevelFilter.log`,
           'utf8',
@@ -129,7 +129,7 @@ test('log4js logLevelFilter', (batch) => {
         );
       });
       t.test(
-        'tmp-tests-warnings.log should contain only error and warning logs',
+        'logLevelFilter-warnings.log should contain only error and warning logs',
         (assert) => {
           fs.readFile(
             `${__dirname}/logLevelFilter-warnings.log`,
@@ -143,7 +143,7 @@ test('log4js logLevelFilter', (batch) => {
         }
       );
       t.test(
-        'tmp-tests-debugs.log should contain only trace and debug logs',
+        'logLevelFilter-debugs.log should contain only trace and debug logs',
         (assert) => {
           fs.readFile(
             `${__dirname}/logLevelFilter-debugs.log`,