Skip to content

Commit d2ad938

Browse files
Fix propagation of options in Raven.context and Raven.captureException.
Options passed to Raven.context or Raven.captureException weren't getting propagated (there are unit tests for that, but they only test that the options get passed to TraceKit.report). This means that options like 'tags' or 'extra' never actually got passed on to Sentry. The root of the issue was in TraceKit: - Parameters passed to TraceKit.report() didn't get passed to notification handlers (unless the exception was swallowed and window.onerror didn't get called). - Notification handlers for TraceKit.report() didn't get called at all when TraceKit.collectWindowErrors was set to false (unless the exception was swallowed, window.onerror didn't get called, and a second call to TraceKit.report() was made).
1 parent 0356851 commit d2ad938

File tree

2 files changed

+103
-18
lines changed

2 files changed

+103
-18
lines changed

test/raven.test.js

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,91 @@ function generateUUID4() {
4040
return 'abc123';
4141
}
4242

43+
describe('TraceKit', function(){
44+
describe('error notifications', function(){
45+
var testMessage = "__mocha_ignore__";
46+
var subscriptionHandler;
47+
// TraceKit waits 2000ms for window.onerror to fire, so give the tests
48+
// some extra time.
49+
this.timeout(3000);
50+
51+
before(function() {
52+
// Prevent the onerror call that's part of our tests from getting to
53+
// mocha's handler, which would treat it as a test failure.
54+
//
55+
// We set this up here and don't ever restore the old handler, because
56+
// we can't do that without clobbering TraceKit's handler, which can only
57+
// be installed once.
58+
var oldOnError = window.onerror;
59+
window.onerror = function(message) {
60+
if (message == testMessage) {
61+
return true;
62+
}
63+
return oldOnError.apply(this, arguments);
64+
};
65+
});
66+
67+
afterEach(function() {
68+
if (subscriptionHandler) {
69+
TraceKit.report.unsubscribe(subscriptionHandler);
70+
subscriptionHandler = null;
71+
}
72+
});
73+
74+
function testErrorNotification(collectWindowErrors, callOnError, numReports, done) {
75+
var extraVal = "foo";
76+
var numDone = 0;
77+
// TraceKit's collectWindowErrors flag shouldn't affect direct calls
78+
// to TraceKit.report, so we parameterize it for the tests.
79+
TraceKit.collectWindowErrors = collectWindowErrors;
80+
81+
subscriptionHandler = function(stackInfo, extra) {
82+
assert.equal(extra, extraVal);
83+
numDone++;
84+
if (numDone == numReports) {
85+
done();
86+
}
87+
}
88+
TraceKit.report.subscribe(subscriptionHandler);
89+
90+
// TraceKit.report always throws an exception in order to trigger
91+
// window.onerror so it can gather more stack data. Mocha treats
92+
// uncaught exceptions as errors, so we catch it via assert.throws
93+
// here (and manually call window.onerror later if appropriate).
94+
//
95+
// We test multiple reports because TraceKit has special logic for when
96+
// report() is called a second time before either a timeout elapses or
97+
// window.onerror is called (which is why we always call window.onerror
98+
// only once below, after all calls to report()).
99+
for (var i=0; i < numReports; i++) {
100+
var e = new Error('testing');
101+
assert.throws(function() {
102+
TraceKit.report(e, extraVal);
103+
}, e);
104+
}
105+
// The call to report should work whether or not window.onerror is
106+
// triggered, so we parameterize it for the tests. We only call it
107+
// once, regardless of numReports, because the case we want to test for
108+
// multiple reports is when window.onerror is *not* called between them.
109+
if (callOnError) {
110+
window.onerror(testMessage);
111+
}
112+
}
113+
114+
Mocha.utils.forEach([false, true], function(collectWindowErrors) {
115+
Mocha.utils.forEach([false, true], function(callOnError) {
116+
Mocha.utils.forEach([1, 2], function(numReports) {
117+
it('it should receive arguments from report() when' +
118+
' collectWindowErrors is ' + collectWindowErrors +
119+
' and callOnError is ' + callOnError +
120+
' and numReports is ' + numReports, function(done) {
121+
testErrorNotification(collectWindowErrors, callOnError, numReports, done);
122+
});
123+
});
124+
});
125+
});
126+
});
127+
});
43128

44129
describe('globals', function() {
45130
beforeEach(function() {

vendor/TraceKit/tracekit.js

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ TraceKit.wrap = function traceKitWrapper(func) {
7575
*/
7676
TraceKit.report = (function reportModuleWrapper() {
7777
var handlers = [],
78+
lastArgs = null,
7879
lastException = null,
7980
lastExceptionStack = null;
8081

@@ -111,9 +112,9 @@ TraceKit.report = (function reportModuleWrapper() {
111112
* Dispatch stack information to all handlers.
112113
* @param {Object.<string, *>} stack
113114
*/
114-
function notifyHandlers(stack, windowError) {
115+
function notifyHandlers(stack, isWindowError) {
115116
var exception = null;
116-
if (windowError && !TraceKit.collectWindowErrors) {
117+
if (isWindowError && !TraceKit.collectWindowErrors) {
117118
return;
118119
}
119120
for (var i in handlers) {
@@ -145,19 +146,16 @@ TraceKit.report = (function reportModuleWrapper() {
145146
* @param {?Error} ex The actual Error object.
146147
*/
147148
function traceKitWindowOnError(message, url, lineNo, colNo, ex) {
148-
var stack = null, skipNotify = false;
149+
var stack = null;
149150

150151
if (!isUndefined(ex)) {
151152
// New chrome and blink send along a real error object
152153
// Let's just report that like a normal error.
153154
// See: https://mikewest.org/2013/08/debugging-runtime-errors-with-window-onerror
154155
report(ex, false);
155-
skipNotify = true;
156156
} else if (lastExceptionStack) {
157157
TraceKit.computeStackTrace.augmentStackTraceWithInitialElement(lastExceptionStack, url, lineNo, message);
158-
stack = lastExceptionStack;
159-
lastExceptionStack = null;
160-
lastException = null;
158+
processLastException();
161159
} else {
162160
var location = {
163161
'url': url,
@@ -173,10 +171,7 @@ TraceKit.report = (function reportModuleWrapper() {
173171
'stack': [location],
174172
'useragent': navigator.userAgent
175173
};
176-
}
177-
178-
if (!skipNotify) {
179-
notifyHandlers(stack, 'from window.onerror');
174+
notifyHandlers(stack, true);
180175
}
181176

182177
if (_oldOnerrorHandler) {
@@ -206,6 +201,15 @@ TraceKit.report = (function reportModuleWrapper() {
206201
_oldOnerrorHandler = undefined;
207202
}
208203

204+
function processLastException() {
205+
var _lastExceptionStack = lastExceptionStack,
206+
_lastArgs = lastArgs;
207+
lastArgs = null;
208+
lastExceptionStack = null;
209+
lastException = null;
210+
notifyHandlers.apply(null, [_lastExceptionStack, false].concat(_lastArgs));
211+
}
212+
209213
/**
210214
* Reports an unhandled Error to TraceKit.
211215
* @param {Error} ex
@@ -219,26 +223,22 @@ TraceKit.report = (function reportModuleWrapper() {
219223
if (lastException === ex) {
220224
return; // already caught by an inner catch block, ignore
221225
} else {
222-
var s = lastExceptionStack;
223-
lastExceptionStack = null;
224-
lastException = null;
225-
notifyHandlers.apply(null, [s, null].concat(args));
226+
processLastException();
226227
}
227228
}
228229

229230
var stack = TraceKit.computeStackTrace(ex);
230231
lastExceptionStack = stack;
231232
lastException = ex;
233+
lastArgs = args;
232234

233235
// If the stack trace is incomplete, wait for 2 seconds for
234236
// slow slow IE to see if onerror occurs or not before reporting
235237
// this exception; otherwise, we will end up with an incomplete
236238
// stack trace
237239
window.setTimeout(function () {
238240
if (lastException === ex) {
239-
lastExceptionStack = null;
240-
lastException = null;
241-
notifyHandlers.apply(null, [stack, null].concat(args));
241+
processLastException();
242242
}
243243
}, (stack.incomplete ? 2000 : 0));
244244

0 commit comments

Comments
 (0)