Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rollbar.log calls do not gracefully handle cyclic references in custom object argument #955

Closed
AndrewJDR opened this issue Jun 11, 2021 · 3 comments · Fixed by #990
Closed
Assignees

Comments

@AndrewJDR
Copy link

Repro:

  1. Paste the code at the end of this issue into a js file
  2. Ensure rollbar is installed (npm install rollbar)
  3. run ROLLBAR_ACCESS_TOKEN=<token> node <file>.js

Result: The error is not reported to rollbar. Instead, a new exception is thrown within rollbar's merge function, which is currently not capable of dealing with cyclic references in the custom object passed to it. This error message is shown on the console:

Uncaught Exception thrown RangeError: Maximum call stack size exceeded
    at merge (/home/dev/Documents/src/rollbar-cyclic-object-crash/node_modules/rollbar/src/merge.js:1:1)
    at merge (/home/dev/Documents/src/rollbar-cyclic-object-crash/node_modules/rollbar/src/merge.js:44:26)
    at merge (/home/dev/Documents/src/rollbar-cyclic-object-crash/node_modules/rollbar/src/merge.js:44:26)
    at merge (/home/dev/Documents/src/rollbar-cyclic-object-crash/node_modules/rollbar/src/merge.js:44:26)
    at merge (/home/dev/Documents/src/rollbar-cyclic-object-crash/node_modules/rollbar/src/merge.js:44:26)
    at merge (/home/dev/Documents/src/rollbar-cyclic-object-crash/node_modules/rollbar/src/merge.js:44:26)
    at merge (/home/dev/Documents/src/rollbar-cyclic-object-crash/node_modules/rollbar/src/merge.js:44:26)
    at merge (/home/dev/Documents/src/rollbar-cyclic-object-crash/node_modules/rollbar/src/merge.js:44:26)
    at merge (/home/dev/Documents/src/rollbar-cyclic-object-crash/node_modules/rollbar/src/merge.js:44:26)
    at merge (/home/dev/Documents/src/rollbar-cyclic-object-crash/node_modules/rollbar/src/merge.js:44:26)

Expected result:
Rollbar's merge() function used on the custom object paramter should probably deal with cyclic references. I believe Rollbar's scrubbing is already set up to handle cyclic references, so similar logic can probably be used here on the custom object?

code:

const Rollbar = require("rollbar");

const rollbar = new Rollbar({
  accessToken: process.env.ROLLBAR_ACCESS_TOKEN,
  enabled: true,
  environment: "production",
});

process.on('uncaughtException', err => {
  console.error('Uncaught Exception thrown', err);
})

const testError = new Error("test error");

// Prepare a custom object to log to rollbar, and include a cyclic reference
const objA = {}
const objB = {}
objA.objB = objB;
objB.objA = objA;

// TODO: This causes rollbar itself to throw an error, as rollbar.js doesn't have anything
//       in place to handle cyclic references in the custom object passed to it.
//       Perhaps it should?

rollbar.error("error!", testError, objA); // Report an error to rollbar, and include a custom object with it.
@saulogt
Copy link

saulogt commented Aug 26, 2021

I just had the same issue

@waltjones
Copy link
Contributor

We'll address this in the next release. I'll update here with status.

@waltjones
Copy link
Contributor

This is released in v2.24.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants