fix(log4js-node#671): fixes serialisation of errors with properties

Kurea · Feb 27, 2018 · a901861 · a901861
1 parent 29368cf
commit a901861
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 6 deletions.
diff --git a/lib/log4js.js b/lib/log4js.js
@@ -93,8 +93,8 @@ function serialise(logEvent) {
   // Validate that we really are in this case
   try {
     const logData = logEvent.data.map((e) => {
-      if (e && e.stack && CircularJSON.stringify(e) === '{}') {
-        e = { message: e.message, stack: e.stack };
+      if (e && e.message && e.stack) {
+        e = Object.assign({ message: e.message, stack: e.stack }, e);
       }
       return e;
     });
@@ -116,9 +116,9 @@ function deserialise(serialised) {
     event.startTime = new Date(event.startTime);
     event.level = config.levels.getLevel(event.level.levelStr);
     event.data = event.data.map((e) => {
-      if (e && e.stack) {
-        const fakeError = new Error(e.message);
-        fakeError.stack = e.stack;
+      if (e && e.message && e.stack) {
+        const fakeError = new Error(e);
+        Object.keys(e).forEach((key) => { fakeError[key] = e[key]; });
         e = fakeError;
       }
       return e;

diff --git a/test/tap/cluster-test.js b/test/tap/cluster-test.js
@@ -36,17 +36,30 @@ if (cluster.isMaster) {
     test('cluster master', (batch) => {
       batch.test('events should be logged', (t) => {
         t.equal(logEvents.length, 3);
+
         t.equal(logEvents[0].categoryName, 'master');
         t.equal(logEvents[0].pid, masterPid);
+
         t.equal(logEvents[1].categoryName, 'worker');
         t.equal(logEvents[1].pid, workerPid);
+        // serialising errors with stacks intact
         t.type(logEvents[1].data[1], 'Error');
         t.contains(logEvents[1].data[1].stack, 'Error: oh dear');
+        // serialising circular references in objects
         t.type(logEvents[1].data[2], 'object');
         t.type(logEvents[1].data[2].me, 'object');
+        // serialising errors with custom properties
+        t.type(logEvents[1].data[3], 'Error');
+        t.contains(logEvents[1].data[3].stack, 'Error: wtf');
+        t.equal(logEvents[1].data[3].alert, 'chartreuse');
+        // serialising things that are not errors, but look a bit like them
+        t.type(logEvents[1].data[4], 'object');
+        t.equal(logEvents[1].data[4].stack, 'this is not a stack trace');
+
         t.equal(logEvents[2].categoryName, 'log4js');
         t.equal(logEvents[2].level.toString(), 'ERROR');
         t.equal(logEvents[2].data[0], 'Unable to parse log:');
+
         t.end();
       });
 
@@ -63,9 +76,15 @@ if (cluster.isMaster) {
   });
 } else {
   const workerLogger = log4js.getLogger('worker');
+  // test for serialising circular references
   const circle = {};
   circle.me = circle;
-  workerLogger.info('this is worker', new Error('oh dear'), circle);
+  // test for serialising errors with their own properties
+  const someError = new Error('wtf');
+  someError.alert = 'chartreuse';
+  // test for serialising things that look like errors but aren't.
+  const notAnError = { stack: 'this is not a stack trace' };
+  workerLogger.info('this is worker', new Error('oh dear'), circle, someError, notAnError);
   // can't run the test in the worker, things get weird
   process.send({
     type: '::testing',