Skip to content

Commit 8626b8b

Browse files
mattrobenoltmattrobenolt
committed
Merge pull request #189 from russelldavis/fix-notification-args
Fix propagation of options in Raven.context and Raven.wrap.
2 parents 0356851 + 7720af5 commit 8626b8b

File tree

3 files changed

+110
-18
lines changed

3 files changed

+110
-18
lines changed

‎example/index.html

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@
1919
id: 5
2020
})
2121

22+
function testOptions() {
23+
Raven.context({tags: {foo: 'bar'}}, function() {
24+
throw new Error('foo');
25+
});
26+
}
2227

2328
</script>
2429
<body>
@@ -27,5 +32,7 @@
2732

2833
<button onclick="divide(1, 0)">Sourcemap breakage</button>
2934
<button onclick="derp()">window.onerror</button>
35+
<button onclick="testOptions()">test options</button>
36+
3037
</body>
3138
</html>

‎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)