fix: Do not override context when capturing non-error exceptions

Author: kamilogorek

lib/client.js

@@ -333,7 +333,8 @@ extend(Raven.prototype, {
       cb = kwargs;
       kwargs = {};
     } else {
-      kwargs = kwargs || {};
+      // Do not use reference, as we'll modify this object later on
+      kwargs = kwargs ? JSON.parse(stringify(kwargs)) : {};
     }
 
     var eventId = this.generateEventId();
@@ -363,25 +364,28 @@ extend(Raven.prototype, {
       cb = kwargs;
       kwargs = {};
     } else {
-      kwargs = kwargs || {};
+      var req = kwargs && kwargs.req;
+      // Do not use reference, as we'll modify this object later on
+      kwargs = kwargs ? JSON.parse(stringify(kwargs)) : {};
+      // Preserve `req` so we can use some framework's middleware methods
+      // `req` is the only thing that we pass through in `errorHandler`, so we need just that
+      if (req) kwargs.req = req;
     }
 
     if (!(err instanceof Error)) {
       if (utils.isPlainObject(err)) {
         // This will allow us to group events based on top-level keys
         // which is much better than creating new group when any key/value change
         var keys = Object.keys(err).sort();
-        var hash = md5(keys);
         var message =
           'Non-Error exception captured with keys: ' +
           utils.serializeKeysForMessage(keys);
-        var serializedException = utils.serializeException(err);
-
-        kwargs.message = message;
-        kwargs.fingerprint = [hash];
-        kwargs.extra = {
-          __serialized__: serializedException
-        };
+        kwargs = extend(kwargs, {
+          message: message,
+          fingerprint: [md5(keys)],
+          extra: kwargs.extra || {}
+        });
+        kwargs.extra.__serialized__ = utils.serializeException(err);
 
         err = new Error(message);
       } else {

test/raven.client.js

@@ -223,6 +223,22 @@ describe('raven.Client', function() {
         done();
       });
     });
+
+    it('should copy object with extra data instead of using its reference directly', function(done) {
+      var old = client.send;
+      var info = {
+        extra: {
+          hello: 'there'
+        }
+      };
+      client.send = function mockSend(kwargs) {
+        client.send = old;
+        kwargs.extra.should.have.property('hello', 'there');
+        kwargs.extra.should.not.equal(info);
+        done();
+      };
+      client.captureMessage('exception', info);
+    });
   });
 
   describe('#captureException()', function() {
@@ -259,13 +275,9 @@ describe('raven.Client', function() {
       var old = client.send;
       client.send = function mockSend(kwargs) {
         client.send = old;
-
         kwargs.message.should.equal(
           'Non-Error exception captured with keys: aKeyOne, bKeyTwo, cKeyThree, dKeyFour\u2026'
         );
-
-        // Remove superfluous node version data to simplify the test itself
-        delete kwargs.extra.node;
         kwargs.extra.should.have.property('__serialized__', {
           aKeyOne: 'a',
           bKeyTwo: 42,
@@ -385,6 +397,64 @@ describe('raven.Client', function() {
         done();
       });
     });
+
+    it('should use and merge provided extra data instead of overriding it', function(done) {
+      var old = client.send;
+      client.send = function mockSend(kwargs) {
+        client.send = old;
+        kwargs.extra.should.have.property('hello', 'there');
+        kwargs.tags.should.deepEqual({'0': 'whoop'});
+        done();
+      };
+      client.captureException(
+        {some: 'exception'},
+        {
+          extra: {
+            hello: 'there'
+          },
+          tags: ['whoop']
+        }
+      );
+    });
+
+    it('should copy object with extra data instead of using its reference directly', function(done) {
+      var old = client.send;
+      var info = {
+        extra: {
+          hello: 'there'
+        }
+      };
+      client.send = function mockSend(kwargs) {
+        client.send = old;
+        kwargs.extra.should.have.property('hello', 'there');
+        kwargs.extra.should.not.equal(info.extra);
+        done();
+      };
+      client.captureException({some: 'exception'}, info);
+    });
+
+    it('should preserve same reference to `req` attribute in kwargs', function(done) {
+      var old = client.process;
+      var info = {
+        extra: {
+          hello: 'there'
+        },
+        req: {
+          something: 'else'
+        }
+      };
+      // Use `process` instead of `send` as `req` is stripped from the final payload
+      client.process = function mockProcess(id, kwargs) {
+        client.process = old;
+        kwargs.extra.should.have.property('hello', 'there');
+        kwargs.extra.should.not.equal(info.extra);
+
+        kwargs.req.should.have.property('something', 'else');
+        kwargs.req.should.equal(info.req);
+        done();
+      };
+      client.captureException({some: 'exception'}, info);
+    });
   });
 
   describe('#install()', function() {