Handle circular references in custom data (#990)

* fix: handle circular references in custom data

* test: use deepStrictEqual to compare objects for equality

Author: waltjones

src/utility.js

@@ -398,6 +398,37 @@ function wrapCallback(logger, f) {
   };
 }
 
+function nonCircularClone(obj) {
+  var seen = [obj];
+
+  function clone(obj, seen) {
+    var value, name, newSeen, result = {};
+
+    try {
+      for (name in obj) {
+        value = obj[name];
+
+        if (value && (isType(value, 'object') || isType(value, 'array'))) {
+          if (seen.includes(value)) {
+            result[name] = 'Removed circular reference: ' + typeName(value);
+          } else {
+            newSeen = seen.slice();
+            newSeen.push(value);
+            result[name] = clone(value, newSeen);
+          }
+          continue;
+        }
+
+        result[name] = value;
+      }
+    } catch (e) {
+      result = 'Failed cloning custom data: ' + e.message;
+    }
+    return result;
+  }
+  return clone(obj, seen);
+}
+
 function createItem(args, logger, notifier, requestKeys, lambdaContext) {
   var message, err, custom, callback, request;
   var arg;
@@ -455,10 +486,12 @@ function createItem(args, logger, notifier, requestKeys, lambdaContext) {
     }
   }
 
+  // if custom is an array this turns it into an object with integer keys
+  if (custom) custom = nonCircularClone(custom);
+
   if (extraArgs.length > 0) {
-    // if custom is an array this turns it into an object with integer keys
-    custom = merge(custom);
-    custom.extraArgs = extraArgs;
+    if (!custom) custom = nonCircularClone({});
+    custom.extraArgs = nonCircularClone(extraArgs);
   }
 
   var item = {
@@ -503,7 +536,7 @@ function addErrorContext(item, errors) {
   try {
     for (var i = 0; i < errors.length; ++i) {
       if (errors[i].hasOwnProperty('rollbarContext')) {
-        custom = merge(custom, errors[i].rollbarContext);
+        custom = merge(custom, nonCircularClone(errors[i].rollbarContext));
         contextAdded = true;
       }
     }

test/browser.core.test.js

@@ -503,6 +503,58 @@ describe('log', function() {
     done();
   })
 
+  it('should remove circular references in custom data', function(done) {
+    var server = window.server;
+    stubResponse(server);
+    server.requests.length = 0;
+
+    var options = {
+      accessToken: 'POST_CLIENT_ITEM_TOKEN',
+      addErrorContext: true
+    };
+    var rollbar = window.rollbar = new Rollbar(options);
+
+    var err = new Error('test error');
+    var contextData = { extra: 'baz' }
+    contextData.data = contextData;
+    var context = { err: 'test', contextData: contextData };
+    err.rollbarContext = context;
+
+    var array = ['one', 'two'];
+    array.push(array);
+    var custom = { foo: 'bar', array: array };
+    var notCircular = { key: 'value' };
+    custom.notCircular1 = notCircular;
+    custom.notCircular2 = notCircular;
+    custom.self = custom;
+    rollbar.error(err, custom);
+
+    server.respond();
+
+    var body = JSON.parse(server.requests[0].requestBody);
+
+    expect(body.data.body.trace.exception.message).to.eql('test error');
+    expect(body.data.custom.foo).to.eql('bar');
+    expect(body.data.custom.err).to.eql('test');
+
+    // Duplicate objects are allowed when there is no circular reference.
+    expect(body.data.custom.notCircular1).to.eql(notCircular);
+    expect(body.data.custom.notCircular2).to.eql(notCircular);
+
+    expect(body.data.custom.self).to.eql(
+      'Removed circular reference: object'
+    );
+    expect(body.data.custom.array).to.eql([
+      'one', 'two', 'Removed circular reference: array'
+    ]);
+    expect(body.data.custom.contextData).to.eql({
+      extra: 'baz',
+      data: 'Removed circular reference: object'
+    });
+
+    done();
+  })
+
   it('should send message when called with only null arguments', function(done) {
     var server = window.server;
     stubResponse(server);

test/server.rollbar.test.js

@@ -278,7 +278,7 @@ vows.describe('rollbar')
             var item = r.client.logCalls[r.client.logCalls.length - 1].item;
             assert.equal(item.message, message);
             assert.equal(item.request, request);
-            assert.equal(item.custom, custom);
+            assert.deepStrictEqual(item.custom, custom);
             item.callback();
             assert.isTrue(callbackCalled);
           }
@@ -332,7 +332,7 @@ vows.describe('rollbar')
             var item = r.client.logCalls[r.client.logCalls.length - 1].item
             assert.equal(item.message, message)
             assert.equal(item.request, request)
-            assert.equal(item.custom, custom)
+            assert.deepStrictEqual(item.custom, custom)
           },
           'should work with request and custom and callback': function(r) {
             var message = 'hello'
@@ -346,7 +346,7 @@ vows.describe('rollbar')
             var item = r.client.logCalls[r.client.logCalls.length - 1].item
             assert.equal(item.message, message)
             assert.equal(item.request, request)
-            assert.equal(item.custom, custom)
+            assert.deepStrictEqual(item.custom, custom)
             item.callback();
             assert.isTrue(callbackCalled);
           }
@@ -399,7 +399,7 @@ vows.describe('rollbar')
             var item = r.client.logCalls[r.client.logCalls.length - 1].item
             assert.equal(item.err, err)
             assert.equal(item.request, request)
-            assert.equal(item.custom, custom)
+            assert.deepStrictEqual(item.custom, custom)
             item.callback();
             assert.isTrue(callbackCalled);
           }
@@ -453,7 +453,7 @@ vows.describe('rollbar')
             var item = r.client.logCalls[r.client.logCalls.length - 1].item
             assert.equal(item.err, err)
             assert.equal(item.request, request)
-            assert.equal(item.custom, custom)
+            assert.deepStrictEqual(item.custom, custom)
           },
           'should work with request and custom and callback': function(r) {
             var err = new Error('hello!')
@@ -467,7 +467,7 @@ vows.describe('rollbar')
             var item = r.client.logCalls[r.client.logCalls.length - 1].item
             assert.equal(item.err, err)
             assert.equal(item.request, request)
-            assert.equal(item.custom, custom)
+            assert.deepStrictEqual(item.custom, custom)
             item.callback();
             assert.isTrue(callbackCalled);
           }