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

Reject back-to-back duplicate errors/messages #861

Merged
merged 5 commits into from
Feb 24, 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
6 changes: 6 additions & 0 deletions docs/config.rst
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,12 @@ Those configuration options are documented below:
onError
Callback to be invoked upon a failed request.

.. describe:: allowDuplicates

By default, Raven.js attempts to suppress duplicate captured errors and messages that occur back-to-back. Such events are often triggered by rogue code (e.g. from a `setInterval` callback in a browser extension), are not actionable, and eat up your event quota.

To disable this behavior (for example, when testing), set ``allowDuplicates: true`` during configuration.

.. describe:: allowSecretKey

By default, Raven.js will throw an error if configured with a Sentry DSN that contains a secret key.
Expand Down
7 changes: 2 additions & 5 deletions example/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,9 @@
whitelistUrls: [
/localhost/,
/127\.0\.0\.1/
],
dataCallback: function(data) {
console.log(data);
return data;
}
]
}).install();
Raven.debug = true;

Raven.setUserContext({
email: 'me@example.com',
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,13 @@
"grunt-contrib-uglify": "^0.11.0",
"grunt-eslint": "^17.3.1",
"grunt-gitinfo": "^0.1.7",
"grunt-mocha": "1.0.2",
"grunt-mocha": "1.0.4",
"grunt-release": "^0.13.0",
"grunt-s3": "0.2.0-alpha.3",
"grunt-sri": "mattrobenolt/grunt-sri#pretty",
"jquery": "^2.1.4",
"lodash": "^3.10.1",
"mocha": "^1.21.5",
"mocha": "2.5.3",
"proxyquireify": "^3.0.2",
"sinon": "1.7.3",
"through2": "^2.0.0",
Expand Down
93 changes: 93 additions & 0 deletions src/raven.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ function Raven() {
this._hasDocument = !isUndefined(_document);
this._hasNavigator = !isUndefined(_navigator);
this._lastCapturedException = null;
this._lastData = null;
this._lastEventId = null;
this._globalServer = null;
this._globalKey = null;
Expand Down Expand Up @@ -1334,6 +1335,35 @@ Raven.prototype = {
return this._backoffDuration && now() - this._backoffStart < this._backoffDuration;
},

/**
* Returns true if the in-process data payload matches the signature
* of the previously-sent data
*
* NOTE: This has to be done at this level because TraceKit can generate
* data from window.onerror WITHOUT an exception object (IE8, IE9,
* other old browsers). This can take the form of an "exception"
* data object with a single frame (derived from the onerror args).
*/
_isRepeatData: function (current) {
var last = this._lastData;

if (!last ||
current.message !== last.message || // defined for captureMessage
current.culprit !== last.culprit) // defined for captureException/onerror
return false;

// Stacktrace interface (i.e. from captureMessage)
if (current.stacktrace || last.stacktrace) {
return isSameStacktrace(current.stacktrace, last.stacktrace);
}
// Exception interface (i.e. from captureException/onerror)
else if (current.exception || last.exception) {
return isSameException(current.exception, last.exception);
}

return true;
},

_setBackoffState: function(request) {
// If we are already in a backoff state, don't change anything
if (this._shouldBackoff()) {
Expand Down Expand Up @@ -1460,6 +1490,17 @@ Raven.prototype = {
// Try and clean up the packet before sending by truncating long values
data = this._trimPacket(data);

// ideally duplicate error testing should occur *before* dataCallback/shouldSendCallback,
// but this would require copying an un-truncated copy of the data packet, which can be
// arbitrarily deep (extra_data) -- could be worthwhile? will revisit
if (!this._globalOptions.allowDuplicates && this._isRepeatData(data)) {
this._logDebug('warn', 'Raven dropped repeat event: ', data);
return;
}

// Store outbound payload after trim
this._lastData = data;

this._logDebug('debug', 'Raven about to send:', data);

var auth = {
Expand Down Expand Up @@ -1821,6 +1862,58 @@ function htmlElementAsString(elem) {
return out.join('');
}

/**
* Returns true if either a OR b is truthy, but not both
*/
function isOnlyOneTruthy(a, b) {
return !!(!!a ^ !!b);
}

/**
* Returns true if the two input exception interfaces have the same content
*/
function isSameException(ex1, ex2) {
if (isOnlyOneTruthy(ex1, ex2))
return false;

ex1 = ex1.values[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

note to self/being paranoid - do we want to nullcheck values here first?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We assemble these objects ourselves, and other pieces of the code make the same assumption so I feel it's safe:

e.g. here's _trimPacket:

    _trimPacket: function(data) {
        // For now, we only want to truncate the two different messages
        // but this could/should be expanded to just trim everything
        var max = this._globalOptions.maxMessageLength;
        if (data.message) {
            data.message = truncate(data.message, max);
        }
        if (data.exception) {
            var exception = data.exception.values[0];
            exception.value = truncate(exception.value, max);
        }

        return data;
    },

ex2 = ex2.values[0];

if (ex1.type !== ex2.type ||
ex1.value !== ex2.value)
return false;

return isSameStacktrace(ex1.stacktrace, ex2.stacktrace);
}

/**
* Returns true if the two input stack trace interfaces have the same content
*/
function isSameStacktrace(stack1, stack2) {
if (isOnlyOneTruthy(stack1, stack2))
return false;

var frames1 = stack1.frames;
var frames2 = stack2.frames;

// Exit early if frame count differs
if (frames1.length !== frames2.length)
return false;

// Iterate through every frame; bail out if anything differs
var a, b;
for (var i = 0; i < frames1.length; i++) {
a = frames1[i];
b = frames2[i];
if (a.filename !== b.filename ||
a.lineno !== b.lineno ||
a.colno !== b.colno ||
a['function'] !== b['function'])
return false;
}
return true;
}

/**
* Polyfill a method
* @param obj object e.g. `document`
Expand Down
6 changes: 6 additions & 0 deletions test/integration/frame.html
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,12 @@
function foo() {
bar();
}

function foo2() {
// identical to foo, but meant for testing
// different stack frame fns w/ same stack length
bar();
}
</script>
</head>
<body>
Expand Down
119 changes: 118 additions & 1 deletion test/integration/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ describe('integration', function () {
function () {
setTimeout(done);


Raven.captureException({foo:'bar'});
},
function () {
Expand Down Expand Up @@ -120,6 +119,124 @@ describe('integration', function () {
}
);
});

it('should reject duplicate, back-to-back errors from captureError', function (done) {
var iframe = this.iframe;
iframeExecute(iframe, done,
function () {
Raven._breadcrumbs = [];

var count = 5;
setTimeout(function invoke() {
// use setTimeout to capture new error objects that have
// identical stack traces (can't call sequentially or callsite
// line number will change)
//
// order:
// Error: foo
// Error: foo (suppressed)
// Error: foo (suppressed)
// Error: bar
// Error: foo
if (count === 2) {
Raven.captureException(new Error('bar'));
}
else {
Raven.captureException(new Error('foo'));
}

if (count-- === 0) return void done();
else setTimeout(invoke);
});
},
function () {
var breadcrumbs = iframe.contentWindow.Raven._breadcrumbs;
// use breadcrumbs to evaluate which errors were sent
// NOTE: can't use ravenData because duplicate error suppression occurs
// AFTER dataCallback/shouldSendCallback (dataCallback will record
// duplicates but they ultimately won't be sent)
assert.equal(breadcrumbs.length, 3);
assert.equal(breadcrumbs[0].message, 'Error: foo');
assert.equal(breadcrumbs[1].message, 'Error: bar');
assert.equal(breadcrumbs[2].message, 'Error: foo');
}
);
});

it('should not reject back-to-back errors with different stack traces', function (done) {
var iframe = this.iframe;
iframeExecute(iframe, done,
function () {
setTimeout(done);
Raven._breadcrumbs = [];

// same error message, but different stacks means that these are considered
// different errors
// NOTE: PhantomJS can't derive function/lineno/colno from evaled frames, must
// use frames declared in frame.html (foo(), bar())

// stack:
// bar
try {
bar(); // declared in frame.html
} catch (e) {
Raven.captureException(e);
}

// stack (different # frames):
// bar
// foo
try {
foo(); // declared in frame.html
} catch (e) {
Raven.captureException(e);
}

// stack (same # frames, different frames):
// bar
// foo2
try {
foo2(); // declared in frame.html
} catch (e) {
Raven.captureException(e);
}
},
function () {
var breadcrumbs = iframe.contentWindow.Raven._breadcrumbs;
assert.equal(breadcrumbs.length, 3);
// NOTE: regex because exact error message differs per-browser
assert.match(breadcrumbs[0].message, /^ReferenceError.*baz/);
assert.match(breadcrumbs[1].message, /^ReferenceError.*baz/);
assert.match(breadcrumbs[2].message, /^ReferenceError.*baz/);
}
);
});

it('should reject duplicate, back-to-back messages from captureMessage', function (done) {
var iframe = this.iframe;
iframeExecute(iframe, done,
function () {
setTimeout(done);

Raven._breadcrumbs = [];

Raven.captureMessage('this is fine');
Raven.captureMessage('this is fine'); // suppressed
Raven.captureMessage('this is fine', { stacktrace: true });
Raven.captureMessage('i\'m okay with the events that are unfolding currently');
Raven.captureMessage('that\'s okay, things are going to be okay');
},
function () {
var breadcrumbs = iframe.contentWindow.Raven._breadcrumbs;

assert.equal(breadcrumbs.length, 4);
assert.equal(breadcrumbs[0].message, 'this is fine');
assert.equal(breadcrumbs[1].message, 'this is fine'); // with stacktrace
assert.equal(breadcrumbs[2].message, 'i\'m okay with the events that are unfolding currently');
assert.equal(breadcrumbs[3].message, 'that\'s okay, things are going to be okay');
}
);
});
});

describe('window.onerror', function () {
Expand Down
Loading