Sentry-javascript: Circular references causing browser crash when collecting errors
Important Details
How are you running Sentry?
"@sentry/browser": "^4.1.1"
Description
When error happens, the browser crash because Sentry tries to collect errors and there is a loop somewhere.
Steps to Reproduce
Using chrome:
1) An error happened during the use of my app (a crash happened in a React View, which is catched by ErrorBounding of React => Sentry is creating a report).
2) The app crash because of lack of memory
I don't really know how to reproduce it with a "normal use" of Sentry because I don't know how is collected "breadthumbs" (cf. following explanations) but you should be able to reproduce the issue with explanations and following code.
What happened ?
Synthesis:
This is caused by circular references in objects captured by Sentry when sending an error report.
When this kind of object is captured if you do JSON.stringify(objectWithCircular), the console will tell you that there are circular methods. But when using JSON.stringify(object, serializer()) (as used in sentry libs), I don't know why but it crashes.
Details:
I've investigated to understand why the crash happened and it's due to the following line :
"return JSON.stringify(object, serializer());" in @senty/browser/dist/index.
The object attribute is an object produced by Sentry to send the report. In "breadcrumbs.data.extra.arguments", you can find some object which are captured by Sentry.
When calling JSON.stringify(object, serializer()), it will progressively call the method "toJSON" of all object contained in the object. Sentry has a mecanism to prevent circular references but this doesn't work in this case.
For instance, in my case the problem was caused by a google map object which was captured inside breadcrumbs[].data.extra.arguments[0].
To reproduce it you can create a google map:
<html>
<head>
<meta charset="utf-8" />
<script src="https://maps.googleapis.com/maps/api/js?callback=initMap"
async defer></script>
</head>
<body>
<div id="map" ></div>
</body>
<script>
function initMap(map) {
var map;
map = new google.maps.Map(document.getElementById('map'), {
center: {lat: -34.397, lng: 150.644},
zoom: 8
});
console.log(JSON.stringify(map))
}
</script>
</html>
In this code, you can see that "console.log(JSON.stringify({gm: map}))" will write in console "circular references".
If you include the code in "@sentry/browser/dist", and use the function
"JSON.stringify({gm: map}, serializer()) memory will inscrease until the browser crash.
What you expected to happen
A mecanism to prevent circular references to cause the crash.
Possible Solution
?
The workaround I used in my code was to prevent google map object from being stringify like this:
function serializer() {
var stack = [];
var keys = [];
var cycleReplacer = function (_, value) {
if (stack[0] === value) {
return '[Circular ~]';
}
return "[Circular ~." + keys.slice(0, stack.indexOf(value)).join('.') + "]";
};
return function (key, value) {
if(value && value.gm_bindings_) return "[google map]"; //cause une r茅f茅rence circulaire
This could not be a perennial one because you never know which objects are collected and which ones will cause a crash.
dorianrod
All 14 comments
FYI, after looking at doc, I see a better workaround without modifying sources:
When init Sentry, I remove arguments.
init({
beforeBreadcrumb(breadcrumb, hint) {
breadcrumb.error.data.extra.arguments = [];
return breadcrumb;
}
});
dorianrod
on 30 Oct 2018
I can reproduce the issue locally. We can fix it by using smaller and way faster https://github.com/WebReflection/flatted instead, which doesn't choke on stringifying a google maps object (already tested it locally and it works).
kamilogorek
on 31 Oct 2018
Yes, this is messing up WordPress Gutenberg React-app big time, crashing people's browsers daily. Problem appears when "block" grammar validation fails.
Started when our WP integration upgraded from Sentry JS v3 -> v4.
lkraav
on 1 Nov 2018
Yup, I just installed Sentry for first time on my React app, and this is happening. I'm using google-maps-react for Google Maps integration, so suspect that's cause as per above.
ineffyble
on 4 Nov 2018
I am having I think the same issue. With Sentry 4.1.1 embedded (and inited with dsn) at some point my React app freezes apparently without an error (same flow without Sentry doesn't generate an error), perhaps Sentry is just trying to send a warning and there is a circular reference. I had to remove Sentry. @kamilogorek how do you use Flatted with Sentry?
nuthinking
on 5 Nov 2018
Apparently, it's not that easy to just swap them around, as we have to preserve information about NaN, undefined and have a custom Error serializer.
Is anyone able to provide a dataset that our SDK chokes and I could use it in test cases?
kamilogorek
on 15 Nov 2018
Does my case in #1717 not reproduce the error for you?
I fixed it like this for us atm
// Turn dom node into string
var crumb = function(node) {
var idOrClass = (node.id && "#"+node.id) || (""+node.classList && (""+node.classList).replace(/ /g, "."));
return node.tagName.toLowerCase() + idOrClass;
};
var crumbPath = function(node) {return node.parentNode ? crumbPath(node.parentNode).concat(crumb(node)) : [];};
// in the Sentry config:
beforeBreadcrumb: function (breadcrumb) {
if (breadcrumb.category === 'console' && breadcrumb.data && breadcrumb.data.extra && breadcrumb.data.extra.arguments && breadcrumb.data.extra.arguments.length > 0) {
breadcrumb.data.extra.arguments = breadcrumb.data.extra.arguments.map(function (argument) {
if (argument instanceof Element || argument instanceof Node || argument instanceof HTMLElement) {
try { argument = crumbPath(argument).join(' > '); } catch (_) {};
}
return argument;
})
}
return breadcrumb;
}
AdriVanHoudt
on 15 Nov 2018
@AdriVanHoudt I tried with no luck.
<!doctype html>
<html>
<head>
<script src="https://code.jquery.com/jquery-3.3.1.min.js"></script>
<script src="https://browser.sentry-cdn.com/4.3.0/bundle.js"></script>
</head>
<body>
<button>Hello</button>
<script>
Sentry.init({
debug: true,
dsn: "https://[email protected]/297378",
});
console.warn('foo', 1, $('button'));
Sentry.captureException(new Error('f00'));
</script>
</body>
</html>
We decided that in the 4.3.1 by default, we will serialize all argument objects just to 2 levels deep. This means that something like:
console.log(
'a',
1,
['b', 2, { foo: 'foo' }],
{ bar: 42 },
{ baz: [1, 2, 3] }
)
Will become:
console.log(
'a',
1,
['b', 2, '[Object]'],
{ bar: 42 },
{ baz: '[Array]' }
)
Breadcrumbs meant to be a very lightweight hints for developers what went wrong, and not be a fully-fledged data provider.
If someone wishes to get more data and know what he's doing (keep in mind that there's a payload limit), one can always use addEventProcessor and/or beforeBreadcrumb where it's hints have access to all raw arguments, eg.
beforeBreadcrumb (crumb, hint) {
if (crumb.type === 'console') {
// hint.input === raw arguments passed to console calls
// do anything to them, parse, stringify, whatever
}
return crumb;
}
Closed by https://github.com/getsentry/sentry-javascript/pull/1741 release to come Today
kamilogorek
on 16 Nov 2018
I think this works at minimum for restoring safety across the board and we can go further later :+1:
lkraav
on 16 Nov 2018
That works for me 馃憣
@kamilogorek hmm maybe you need an element deep in DOM tree with a lot of parent elements to trigger the fault, but again the proposed solution works for me as well.
AdriVanHoudt
on 19 Nov 2018
@AdriVanHoudt released in the newest version, would you mind giving it a try?
kamilogorek
on 19 Nov 2018
I had a very odd error but now seems to be fine. Thanks!
nuthinking
on 27 Nov 2018
Related issues
jaylinski
路
3Comments
nicolezhu
路
3Comments
Taewa
路
3Comments
mattkrick
路
3Comments
mogelbrod
路
3Comments