Merge pull request #861 from getsentry/less-dupes

Reject back-to-back duplicate errors/messages

Author: benvinegar

docs/config.rst

@@ -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.

example/index.html

@@ -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',

package.json

@@ -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",

src/raven.js

@@ -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;
@@ -1349,6 +1350,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()) {
@@ -1475,6 +1505,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 = {
@@ -1836,6 +1877,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];
+    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`

test/integration/frame.html

@@ -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>

test/integration/test.js

@@ -85,7 +85,6 @@ describe('integration', function () {
                 function () {
                     setTimeout(done);
 
-
                     Raven.captureException({foo:'bar'});
                 },
                 function () {
@@ -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 () {