Skip to content

Don't choke if accessing evt props trigger exceptions ... #857

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

Merged
merged 2 commits into from
Feb 17, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 24 additions & 9 deletions src/raven.js
Original file line number Diff line number Diff line change
Expand Up @@ -744,14 +744,14 @@ Raven.prototype = {
return;

self._lastCapturedEvent = evt;
var elem = evt.target;

// try/catch both:
// - accessing evt.target (see getsentry/raven-js#838, #768)
// - `htmlTreeAsString` because it's complex, and just accessing the DOM incorrectly
// can throw an exception in some circumstances.
var target;

// try/catch htmlTreeAsString because it's particularly complicated, and
// just accessing the DOM incorrectly can throw an exception in some circumstances.
try {
target = htmlTreeAsString(elem);
target = htmlTreeAsString(evt.target);
} catch (e) {
target = '<unknown>';
}
Expand All @@ -776,8 +776,15 @@ Raven.prototype = {
// debounce timeout is triggered, we will only capture
// a single breadcrumb from the FIRST target (acceptable?)
return function (evt) {
var target = evt.target,
tagName = target && target.tagName;
var target;
try {
target = evt.target;
} catch (e) {
// just accessing event properties can throw an exception in some rare circumstances
// see: https://github.com/getsentry/raven-js/issues/838
return;
}
var tagName = target && target.tagName;

// only consider keypress events on actual input elements
// this will disregard keypresses targeting body (e.g. tabbing
Expand Down Expand Up @@ -894,9 +901,17 @@ Raven.prototype = {
// see #724
if (!evt) return;

if (evt.type === 'click')
var eventType;
try {
eventType = evt.type
} catch (e) {
// just accessing event properties can throw an exception in some rare circumstances
// see: https://github.com/getsentry/raven-js/issues/838
return;
}
if (eventType === 'click')
return clickHandler(evt);
else if (evt.type === 'keypress')
else if (eventType === 'keypress')
return keypressHandler(evt);
};
}
Expand Down
46 changes: 46 additions & 0 deletions test/integration/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,52 @@ describe('integration', function () {
);
});

// doesn't work in PhantomJS
if (!/PhantomJS/.test(window.navigator.userAgent)) {
it('should bail out if accessing the `type` and `target` properties of an event throw an exception', function (done) {
// see: https://github.com/getsentry/raven-js/issues/768
var iframe = this.iframe;

iframeExecute(iframe, done,
function () {
setTimeout(done);

// some browsers trigger onpopstate for load / reset breadcrumb state
Raven._breadcrumbs = [];

// click <input/>
var evt = document.createEvent('MouseEvent');
evt.initMouseEvent(evt,
"click",
true /* bubble */,
true /* cancelable */,
window,
null,
0, 0, 0, 0, /* coordinates */
false, false, false, false, /* modifier keys */
0 /*left*/,
null
);

function kaboom() { throw new Error('lol'); };
Object.defineProperty(evt, 'type', { get: kaboom });
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately in Phantom (and Phantom 2) just calling Object.defineProperty on this MouseEvent instance triggers an error. I've tried __defineGetter__ as well – no luck.

Object.defineProperty(evt, 'target', { get: kaboom });

var input = document.querySelector('.a'); // leaf node
input.dispatchEvent(evt);
},
function () {
var Raven = iframe.contentWindow.Raven,
breadcrumbs = Raven._breadcrumbs;

assert.equal(breadcrumbs.length, 1);
assert.equal(breadcrumbs[0].category, 'ui.click');
assert.equal(breadcrumbs[0].message, '<unknown>');
}
);
});
} // if PhantomJS

it('should record consecutive keypress events into a single "input" breadcrumb', function (done) {
var iframe = this.iframe;

Expand Down