From a8c717fc58032ee53104f75d82eb01328c3b0351 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Mon, 16 May 2016 17:36:11 -0700 Subject: [PATCH 1/9] Strip Raven's `wrapped` try/catch call from frames --- src/raven.js | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/raven.js b/src/raven.js index 0cad3335395e..24c8cafc92c1 100644 --- a/src/raven.js +++ b/src/raven.js @@ -278,7 +278,7 @@ Raven.prototype = { return func.apply(this, args); } catch(e) { self._ignoreNextOnError(); - self.captureException(e, options); + self.captureException(e, options, 1); throw e; } } @@ -323,9 +323,11 @@ Raven.prototype = { * @param {object} options A specific set of options for this error [optional] * @return {Raven} */ - captureException: function(ex, options) { + captureException: function(ex, options, skipframes) { + skipframes = skipframes || 0; + // If not an Error is passed through, recall as a message instead - if (!isError(ex)) return this.captureMessage(ex, options); + if (!isError(ex)) return this.captureMessage(ex, options, skipframes + 1); // Store the raw exception object for potential debugging and introspection this._lastCapturedException = ex; @@ -337,7 +339,7 @@ Raven.prototype = { // report on. try { var stack = TraceKit.computeStackTrace(ex); - this._handleStackInfo(stack, options); + this._handleStackInfo(stack, options, skipframes); } catch(ex1) { if(ex !== ex1) { throw ex1; @@ -354,7 +356,7 @@ Raven.prototype = { * @param {object} options A specific set of options for this message [optional] * @return {Raven} */ - captureMessage: function(msg, options) { + captureMessage: function(msg, options, skipframes) { // config() automagically converts ignoreErrors from a list to a RegExp so we need to test for an // early call; we'll error on the side of logging anything called before configuration since it's // probably something you should see: @@ -1064,7 +1066,7 @@ Raven.prototype = { } }, - _handleStackInfo: function(stackInfo, options) { + _handleStackInfo: function(stackInfo, options, skipframes) { var self = this; var frames = []; @@ -1075,6 +1077,10 @@ Raven.prototype = { frames.push(frame); } }); + + if (skipframes) { + frames = frames.slice(0, skipframes); + } } this._triggerEvent('handle', { From 6dc5dcb5050d437af01716233bbc17dc0ed63273 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Mon, 16 May 2016 21:02:27 -0700 Subject: [PATCH 2/9] captureMessage can attach stack trace via `stacktrace: true` --- src/raven.js | 59 +++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 49 insertions(+), 10 deletions(-) diff --git a/src/raven.js b/src/raven.js index 24c8cafc92c1..96c642562b64 100644 --- a/src/raven.js +++ b/src/raven.js @@ -278,7 +278,9 @@ Raven.prototype = { return func.apply(this, args); } catch(e) { self._ignoreNextOnError(); - self.captureException(e, options, 1); + self.captureException(e, objectMerge({ + trimTailFrames: 1 + }, options)); throw e; } } @@ -323,11 +325,14 @@ Raven.prototype = { * @param {object} options A specific set of options for this error [optional] * @return {Raven} */ - captureException: function(ex, options, skipframes) { - skipframes = skipframes || 0; - + captureException: function(ex, options) { // If not an Error is passed through, recall as a message instead - if (!isError(ex)) return this.captureMessage(ex, options, skipframes + 1); + if (!isError(ex)) { + return this.captureMessage(ex, objectMerge({ + trimHeadFrames: 1, + stacktrace: true // if we fall back to captureMessage, default to attempting a new trace + }, options)); + } // Store the raw exception object for potential debugging and introspection this._lastCapturedException = ex; @@ -339,7 +344,7 @@ Raven.prototype = { // report on. try { var stack = TraceKit.computeStackTrace(ex); - this._handleStackInfo(stack, options, skipframes); + this._handleStackInfo(stack, options); } catch(ex1) { if(ex !== ex1) { throw ex1; @@ -356,7 +361,36 @@ Raven.prototype = { * @param {object} options A specific set of options for this message [optional] * @return {Raven} */ - captureMessage: function(msg, options, skipframes) { + captureMessage: function(msg, options) { + if (options.stacktrace) { + var ex; + // create a stack trace from this point; just trim + // off extra frames so they don't include this function call (or + // earlier Raven.js library fn calls) + try { + throw new Error(msg); + } catch (ex1) { + ex = ex1; + } + + // null exception name so `Error` isn't prefixed to msg + ex.name = null; + + options = objectMerge({ + // fingerprint on msg, not stack trace + // NOTE: need also to do this because stack could include Raven.wrap, + // which may create inconsistent traces if only using window.onerror + fingerprint: msg, + trimTailFrames: (options.trimHeadFrames || 0) + 1 + }, options); + + // infinite loop if ex *somehow* returns false for isError + // is that possible when it is result of try/catch? + this.captureException(ex, options); + + return this; + } + // config() automagically converts ignoreErrors from a list to a RegExp so we need to test for an // early call; we'll error on the side of logging anything called before configuration since it's // probably something you should see: @@ -1066,7 +1100,7 @@ Raven.prototype = { } }, - _handleStackInfo: function(stackInfo, options, skipframes) { + _handleStackInfo: function(stackInfo, options) { var self = this; var frames = []; @@ -1078,8 +1112,13 @@ Raven.prototype = { } }); - if (skipframes) { - frames = frames.slice(0, skipframes); + // e.g. frames captured via captureMessage throw + if (options.trimHeadFrames) { + frames = frames.slice(options.trimHeadFrames); + } + // e.g. try/catch (wrapper) frames + if (options.trimTailFrames) { + frames = frames.slice(0, options.trimTailFrames); } } From ae422e9e299fe09c558114ae75f376c3d84b8872 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Fri, 1 Jul 2016 15:46:20 -0700 Subject: [PATCH 3/9] Pass stacktrace via captureMessage as stacktrace interface --- Gruntfile.js | 2 +- src/raven.js | 88 ++++++++++++++++++++++++++-------------------- test/raven.test.js | 9 +++-- 3 files changed, 58 insertions(+), 41 deletions(-) diff --git a/Gruntfile.js b/Gruntfile.js index adcf3d982c4c..407dfee6c6b1 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -132,7 +132,7 @@ module.exports = function(grunt) { dest: 'build/raven.test.js', options: { browserifyOptions: { - debug: true // source maps + debug: false// source maps }, ignore: ['react-native'], plugin: [proxyquire.plugin] diff --git a/src/raven.js b/src/raven.js index 96c642562b64..fa619a278d42 100644 --- a/src/raven.js +++ b/src/raven.js @@ -362,7 +362,18 @@ Raven.prototype = { * @return {Raven} */ captureMessage: function(msg, options) { - if (options.stacktrace) { + // config() automagically converts ignoreErrors from a list to a RegExp so we need to test for an + // early call; we'll error on the side of logging anything called before configuration since it's + // probably something you should see: + if (!!this._globalOptions.ignoreErrors.test && this._globalOptions.ignoreErrors.test(msg)) { + return; + } + + var data = objectMerge({ + message: msg + '' // Make sure it's actually a string + }, options); + + if (options && options.stacktrace) { var ex; // create a stack trace from this point; just trim // off extra frames so they don't include this function call (or @@ -384,26 +395,16 @@ Raven.prototype = { trimTailFrames: (options.trimHeadFrames || 0) + 1 }, options); - // infinite loop if ex *somehow* returns false for isError - // is that possible when it is result of try/catch? - this.captureException(ex, options); - - return this; - } - - // config() automagically converts ignoreErrors from a list to a RegExp so we need to test for an - // early call; we'll error on the side of logging anything called before configuration since it's - // probably something you should see: - if (!!this._globalOptions.ignoreErrors.test && this._globalOptions.ignoreErrors.test(msg)) { - return; + var stack = TraceKit.computeStackTrace(ex); + var frames = this._buildNormalizedFrames(stack, options); + data.stacktrace = { + // Sentry expects frames oldest to newest + frames: frames.reverse() + } } // Fire away! - this._send( - objectMerge({ - message: msg + '' // Make sure it's actually a string - }, options) - ); + this._send(data); return this; }, @@ -1101,9 +1102,26 @@ Raven.prototype = { }, _handleStackInfo: function(stackInfo, options) { + var frames = this._prepareFrames(stackInfo, options); + + this._triggerEvent('handle', { + stackInfo: stackInfo, + options: options + }); + + this._processException( + stackInfo.name, + stackInfo.message, + stackInfo.url, + stackInfo.lineno, + frames, + options + ); + }, + + _prepareFrames: function(stackInfo, options) { var self = this; var frames = []; - if (stackInfo.stack && stackInfo.stack.length) { each(stackInfo.stack, function(i, stack) { var frame = self._normalizeFrame(stack); @@ -1113,30 +1131,25 @@ Raven.prototype = { }); // e.g. frames captured via captureMessage throw - if (options.trimHeadFrames) { - frames = frames.slice(options.trimHeadFrames); + var j; + if (options && options.trimHeadFrames) { + for (j = 0; j < options.trimHeadFrames && j < frames.length; j++) { + frames[j].in_app = true; + } } + // e.g. try/catch (wrapper) frames - if (options.trimTailFrames) { - frames = frames.slice(0, options.trimTailFrames); + if (options && options.trimTailFrames) { + for (j = options.trimTailFrames; j < frames.length; j++) { + frames[j].in_app = true; + } } } - - this._triggerEvent('handle', { - stackInfo: stackInfo, - options: options - }); - - this._processException( - stackInfo.name, - stackInfo.message, - stackInfo.url, - stackInfo.lineno, - frames.slice(0, this._globalOptions.stackTraceLimit), - options - ); + frames = frames.slice(0, this._globalOptions.stackTraceLimit); + return frames; }, + _normalizeFrame: function(frame) { if (!frame.url) return; @@ -1162,7 +1175,6 @@ Raven.prototype = { _processException: function(type, message, fileurl, lineno, frames, options) { var stacktrace; - if (!!this._globalOptions.ignoreErrors.test && this._globalOptions.ignoreErrors.test(message)) return; message += ''; diff --git a/test/raven.test.js b/test/raven.test.js index 868bdf6b30ee..ab1155275a1e 100644 --- a/test/raven.test.js +++ b/test/raven.test.js @@ -1757,7 +1757,10 @@ describe('Raven (public API)', function() { Raven.context({foo: 'bar'}, broken); }, error); assert.isTrue(Raven.captureException.called); - assert.deepEqual(Raven.captureException.lastCall.args, [error, {'foo': 'bar'}]); + assert.deepEqual(Raven.captureException.lastCall.args, [error, { + 'foo': 'bar', + trimTailFrames: 1 // because wrap + }]); }); it('should capture the exception without options', function() { @@ -1768,7 +1771,9 @@ describe('Raven (public API)', function() { Raven.context(broken); }, error); assert.isTrue(Raven.captureException.called); - assert.deepEqual(Raven.captureException.lastCall.args, [error, undefined]); + assert.deepEqual(Raven.captureException.lastCall.args, [error, { + trimTailFrames: 1 // because wrap + }]); }); it('should execute the callback without arguments', function() { From df508d45723fc9ae80d865af20987ae072d65cda Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Fri, 1 Jul 2016 17:42:21 -0700 Subject: [PATCH 4/9] Bad method name --- src/raven.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/raven.js b/src/raven.js index fa619a278d42..98c89127e062 100644 --- a/src/raven.js +++ b/src/raven.js @@ -396,7 +396,7 @@ Raven.prototype = { }, options); var stack = TraceKit.computeStackTrace(ex); - var frames = this._buildNormalizedFrames(stack, options); + var frames = this._prepareFrames(stack, options); data.stacktrace = { // Sentry expects frames oldest to newest frames: frames.reverse() @@ -1134,14 +1134,14 @@ Raven.prototype = { var j; if (options && options.trimHeadFrames) { for (j = 0; j < options.trimHeadFrames && j < frames.length; j++) { - frames[j].in_app = true; + frames[j].in_app = false; } } // e.g. try/catch (wrapper) frames if (options && options.trimTailFrames) { for (j = options.trimTailFrames; j < frames.length; j++) { - frames[j].in_app = true; + frames[j].in_app = false; } } } From 1df1c9401bccef12ff19e1193e8fc21e8902925c Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Wed, 6 Jul 2016 14:41:24 -0700 Subject: [PATCH 5/9] First pass at tests --- src/raven.js | 10 ++++++---- test/raven.test.js | 23 +++++++++++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/raven.js b/src/raven.js index 98c89127e062..170ba7de27f9 100644 --- a/src/raven.js +++ b/src/raven.js @@ -384,6 +384,8 @@ Raven.prototype = { ex = ex1; } + console.log(ex.stack); + // null exception name so `Error` isn't prefixed to msg ex.name = null; @@ -1132,15 +1134,15 @@ Raven.prototype = { // e.g. frames captured via captureMessage throw var j; - if (options && options.trimHeadFrames) { - for (j = 0; j < options.trimHeadFrames && j < frames.length; j++) { + if (options && options.trimTailFrames) { + for (j = 0; j < options.trimTailFrames && j < frames.length; j++) { frames[j].in_app = false; } } // e.g. try/catch (wrapper) frames - if (options && options.trimTailFrames) { - for (j = options.trimTailFrames; j < frames.length; j++) { + if (options && options.trimHeadFrames) { + for (j = frames.length - options.trimHeadFrames; j < frames.length; j++) { frames[j].in_app = false; } } diff --git a/test/raven.test.js b/test/raven.test.js index ab1155275a1e..0d4ccb833459 100644 --- a/test/raven.test.js +++ b/test/raven.test.js @@ -2027,6 +2027,29 @@ describe('Raven (public API)', function() { }); }); + it('should include a synthetic stacktrace if stacktrace:true is passed', function () { + this.sinon.stub(Raven, 'isSetup').returns(true); + this.sinon.stub(Raven, '_send'); + + function foo() { + Raven.captureMessage('foo', { + stacktrace: true + }); + } + + foo(); + var frames = Raven._send.lastCall.args[0].stacktrace.frames; + + // Raven.captureMessage + var last = frames[frames.length - 1]; + console.log(last.function); + assert.isTrue(/captureMessage/.test(last.function)); // loose equality check because differs per-browser + assert.equal(last.in_app, false); + + var secondLast = frames[frames.length - 2]; + assert.equal(secondLast.function, 'foo'); + assert.equal(secondLast.in_app, true); + }); }); describe('.captureException', function() { From 22bebbdbb619183978761dbb68fcb59ae8b3b099 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Wed, 6 Jul 2016 19:02:54 -0700 Subject: [PATCH 6/9] Fix test for PhantomJS --- src/raven.js | 2 -- test/raven.test.js | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/raven.js b/src/raven.js index 170ba7de27f9..41096897af5e 100644 --- a/src/raven.js +++ b/src/raven.js @@ -384,8 +384,6 @@ Raven.prototype = { ex = ex1; } - console.log(ex.stack); - // null exception name so `Error` isn't prefixed to msg ex.name = null; diff --git a/test/raven.test.js b/test/raven.test.js index 0d4ccb833459..431e993d40b3 100644 --- a/test/raven.test.js +++ b/test/raven.test.js @@ -2042,10 +2042,10 @@ describe('Raven (public API)', function() { // Raven.captureMessage var last = frames[frames.length - 1]; - console.log(last.function); - assert.isTrue(/captureMessage/.test(last.function)); // loose equality check because differs per-browser + assert.isTrue(/(captureMessage|^\?)$/.test(last.function)); // loose equality check because differs per-browser assert.equal(last.in_app, false); + // foo var secondLast = frames[frames.length - 2]; assert.equal(secondLast.function, 'foo'); assert.equal(secondLast.in_app, true); From 612fe9561fd5f9f3793060d6e2cd2a17a194c6c9 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Fri, 26 Aug 2016 14:57:27 -0700 Subject: [PATCH 7/9] Remove tail frame trimming (too many edge cases atm), add synthetic example --- example/index.html | 1 + example/scratch.js | 6 ++++++ src/raven.js | 18 +++++------------- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/example/index.html b/example/index.html index 76b237e41d67..11ff74e3511a 100644 --- a/example/index.html +++ b/example/index.html @@ -36,6 +36,7 @@ + diff --git a/example/scratch.js b/example/scratch.js index 794511ed4b2c..48507f9c5d1f 100644 --- a/example/scratch.js +++ b/example/scratch.js @@ -46,6 +46,12 @@ function showDialog() { Raven.showReportDialog(); } +function testSynthetic() { + Raven.captureMessage('synthetic', { + stacktrace: true + }); +} + function blobExample() { var xhr = new XMLHttpRequest(); xhr.open('GET', 'stack.js'); diff --git a/src/raven.js b/src/raven.js index 41096897af5e..dd1d2e2f692b 100644 --- a/src/raven.js +++ b/src/raven.js @@ -278,9 +278,7 @@ Raven.prototype = { return func.apply(this, args); } catch(e) { self._ignoreNextOnError(); - self.captureException(e, objectMerge({ - trimTailFrames: 1 - }, options)); + self.captureException(e, options); throw e; } } @@ -392,7 +390,7 @@ Raven.prototype = { // NOTE: need also to do this because stack could include Raven.wrap, // which may create inconsistent traces if only using window.onerror fingerprint: msg, - trimTailFrames: (options.trimHeadFrames || 0) + 1 + trimHeadFrames: (options.trimHeadFrames || 0) + 1 }, options); var stack = TraceKit.computeStackTrace(ex); @@ -1131,18 +1129,12 @@ Raven.prototype = { }); // e.g. frames captured via captureMessage throw - var j; - if (options && options.trimTailFrames) { - for (j = 0; j < options.trimTailFrames && j < frames.length; j++) { - frames[j].in_app = false; - } - } - - // e.g. try/catch (wrapper) frames if (options && options.trimHeadFrames) { - for (j = frames.length - options.trimHeadFrames; j < frames.length; j++) { + for (var j = frames.length - options.trimHeadFrames; j < frames.length; j++) { frames[j].in_app = false; } + // ... delete to prevent from appearing in outbound payload + delete options.trimHeadFrames; } } frames = frames.slice(0, this._globalOptions.stackTraceLimit); From 31bedfe0ec021c551471418aad409dad70aae6ab Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Fri, 26 Aug 2016 15:47:16 -0700 Subject: [PATCH 8/9] Update comment re: msg fingerprint --- src/raven.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/raven.js b/src/raven.js index dd1d2e2f692b..4e9da5fd415d 100644 --- a/src/raven.js +++ b/src/raven.js @@ -386,9 +386,8 @@ Raven.prototype = { ex.name = null; options = objectMerge({ - // fingerprint on msg, not stack trace - // NOTE: need also to do this because stack could include Raven.wrap, - // which may create inconsistent traces if only using window.onerror + // fingerprint on msg, not stack trace (legacy behavior, could be + // revisited) fingerprint: msg, trimHeadFrames: (options.trimHeadFrames || 0) + 1 }, options); From 9cc1a71901ca3fa083795b26e8a34b6b63415a20 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Fri, 26 Aug 2016 16:29:58 -0700 Subject: [PATCH 9/9] Wrong logic for trimHeadFrames, fix tests --- src/raven.js | 2 +- test/raven.test.js | 7 ++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/raven.js b/src/raven.js index 4e9da5fd415d..c8aa18fefc40 100644 --- a/src/raven.js +++ b/src/raven.js @@ -1129,7 +1129,7 @@ Raven.prototype = { // e.g. frames captured via captureMessage throw if (options && options.trimHeadFrames) { - for (var j = frames.length - options.trimHeadFrames; j < frames.length; j++) { + for (var j = 0; j < options.trimHeadFrames && j < frames.length; j++) { frames[j].in_app = false; } // ... delete to prevent from appearing in outbound payload diff --git a/test/raven.test.js b/test/raven.test.js index 431e993d40b3..c8ad6bb0ccf3 100644 --- a/test/raven.test.js +++ b/test/raven.test.js @@ -1758,8 +1758,7 @@ describe('Raven (public API)', function() { }, error); assert.isTrue(Raven.captureException.called); assert.deepEqual(Raven.captureException.lastCall.args, [error, { - 'foo': 'bar', - trimTailFrames: 1 // because wrap + 'foo': 'bar' }]); }); @@ -1771,9 +1770,7 @@ describe('Raven (public API)', function() { Raven.context(broken); }, error); assert.isTrue(Raven.captureException.called); - assert.deepEqual(Raven.captureException.lastCall.args, [error, { - trimTailFrames: 1 // because wrap - }]); + assert.deepEqual(Raven.captureException.lastCall.args, [error, undefined]); }); it('should execute the callback without arguments', function() {