Do not modify Error or Date objects when logging. Fixes #610

lib/winston/common.js

@@ -80,17 +80,25 @@ exports.clone = function (obj) {
   //
   // We only need to clone reference types (Object)
   //
+  var copy = {};
+
   if (obj instanceof Error) {
-    return obj;
+    // With potential custom Error objects, this might not be exactly correct,
+    // but probably close-enough for purposes of this lib.
+    copy = new Error(obj.message);
+    for (var i in obj) {
+      copy[i] = obj[i];
+    }
+
+    return copy;
   }
   else if (!(obj instanceof Object)) {
     return obj;
   }
   else if (obj instanceof Date) {
-    return obj;
+    return new Date(obj.getTime());
   }
 
-  var copy = {};
   for (var i in obj) {
     if (Array.isArray(obj[i])) {
       copy[i] = obj[i].slice(0);

test/transports/file-test.js

@@ -91,6 +91,42 @@ vows.describe('winston/transports/file').addBatch({
       assert.isTrue(true);
     }
   }
+}).addBatch({
+  "Error object in metadata #610": {
+    topic: function () {
+      var myErr = new Error("foo");
+
+      fileTransport.log('info', 'test message', myErr, this.callback.bind(this, null, myErr));
+    },
+    "should not be modified": function (err, myErr) {
+      assert.equal(myErr.message, "foo");
+      // Not sure if this is the best possible way to check if additional props appeared
+      assert.deepEqual(Object.getOwnPropertyNames(myErr), Object.getOwnPropertyNames(new Error("foo")));
+    }
+  }
+}).addBatch({
+  "Date object in metadata": {
+    topic: function () {
+      var obj = new Date(1000);
+
+      fileTransport.log('info', 'test message', obj, this.callback.bind(this, null, obj));
+    },
+    "should not be modified": function (err, obj) {
+      // Not sure if this is the best possible way to check if additional props appeared
+      assert.deepEqual(Object.getOwnPropertyNames(obj), Object.getOwnPropertyNames(new Date()));
+    }
+  }
+}).addBatch({
+  "Plain object in metadata": {
+    topic: function () {
+      var obj = { message: "foo" };
+
+      fileTransport.log('info', 'test message', obj, this.callback.bind(this, null, obj));
+    },
+    "should not be modified": function (err, obj) {
+      assert.deepEqual(obj, { message: "foo" });
+    }
+  }
 }).addBatch({
   "An instance of the File Transport": transport(winston.transports.File, {
     filename: path.join(__dirname, '..', 'fixtures', 'logs', 'testfile.log')

test/winston-test.js

@@ -93,6 +93,36 @@ vows.describe('winston').addBatch({
             assert.isTrue(typeof winston[key] === 'undefined');
           });
       }
+    },
+    "the clone() method": {
+      "with Error object": {
+        topic: function () {
+          var original = new Error("foo");
+          original.name = "bar";
+
+          var copy = winston.clone(original);
+
+          return { original: original, copy: copy };
+        },
+        "should clone the value": function (result) {
+          assert.notEqual(result.original, result.copy);
+          assert.equal(result.original.message, result.copy.message);
+          assert.equal(result.original.name, result.copy.name);
+        }
+      },
+      "with Date object": {
+        topic: function () {
+          var original = new Date(1000);
+
+          var copy = winston.clone(original);
+
+          return { original: original, copy: copy };
+        },
+        "should clone the value": function (result) {
+          assert.notEqual(result.original, result.copy);
+          assert.equal(result.original.getTime(), result.copy.getTime());
+        }
+      }
     }
   }
 }).export(module);