Skip to content

Commit 3600a05

Browse files
benvinegarbenvinegar
authored
Merge pull request #872 from getsentry/fix-undefined
Fix bad message pulled from thrown strings and objects
2 parents b8d9670 + 7619d2d commit 3600a05

File tree

7 files changed

+132
-16
lines changed

7 files changed

+132
-16
lines changed

‎src/raven.js

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,11 @@
33

44
var TraceKit = require('../vendor/TraceKit/tracekit');
55
var RavenConfigError = require('./configError');
6+
var utils = require('./utils');
7+
8+
var isError = utils.isError,
9+
isObject = utils.isObject;
10+
611
var stringify = require('json-stringify-safe');
712

813
var wrapConsoleMethod = require('./console').wrapMethod;
@@ -1655,25 +1660,12 @@ function isString(what) {
16551660
return objectPrototype.toString.call(what) === '[object String]';
16561661
}
16571662

1658-
function isObject(what) {
1659-
return typeof what === 'object' && what !== null;
1660-
}
16611663

16621664
function isEmptyObject(what) {
16631665
for (var _ in what) return false; // eslint-disable-line guard-for-in, no-unused-vars
16641666
return true;
16651667
}
16661668

1667-
// Sorta yanked from https://github.com/joyent/node/blob/aa3b4b4/lib/util.js#L560
1668-
// with some tiny modifications
1669-
function isError(what) {
1670-
var toString = objectPrototype.toString.call(what);
1671-
return isObject(what) &&
1672-
toString === '[object Error]' ||
1673-
toString === '[object Exception]' || // Firefox NS_ERROR_FAILURE Exceptions
1674-
what instanceof Error;
1675-
}
1676-
16771669
function each(obj, callback) {
16781670
var i, j;
16791671

‎src/utils.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
'use strict';
2+
3+
function isObject(what) {
4+
return typeof what === 'object' && what !== null;
5+
}
6+
7+
// Sorta yanked from https://github.com/joyent/node/blob/aa3b4b4/lib/util.js#L560
8+
// with some tiny modifications
9+
function isError(what) {
10+
var toString = {}.toString.call(what);
11+
return isObject(what) &&
12+
toString === '[object Error]' ||
13+
toString === '[object Exception]' || // Firefox NS_ERROR_FAILURE Exceptions
14+
what instanceof Error;
15+
}
16+
17+
module.exports = {
18+
isObject: isObject,
19+
isError: isError
20+
};

‎test/integration/test.js

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,89 @@ describe('integration', function () {
256256
);
257257
});
258258

259+
it('should catch thrown strings', function (done) {
260+
var iframe = this.iframe;
261+
262+
iframeExecute(iframe, done,
263+
function () {
264+
// intentionally loading this error via a script file to make
265+
// sure it is 1) not caught by instrumentation 2) doesn't trigger
266+
// "Script error"
267+
var script = document.createElement('script');
268+
script.src = 'throw-string.js';
269+
script.onload = function () {
270+
done();
271+
};
272+
document.head.appendChild(script);
273+
},
274+
function () {
275+
var ravenData = iframe.contentWindow.ravenData[0];
276+
assert.match(ravenData.exception.values[0].value, /stringError$/);
277+
assert.equal(ravenData.exception.values[0].stacktrace.frames.length, 1); // always 1 because thrown strings can't provide > 1 frame
278+
279+
// some browsers extract proper url, line, and column for thrown strings
280+
// but not all - falls back to frame url
281+
assert.match(ravenData.exception.values[0].stacktrace.frames[0].filename, /\/test\/integration\//);
282+
assert.match(ravenData.exception.values[0].stacktrace.frames[0]['function'], /\?|global code/);
283+
}
284+
);
285+
});
286+
287+
it('should catch thrown objects', function (done) {
288+
var iframe = this.iframe;
289+
290+
iframeExecute(iframe, done,
291+
function () {
292+
// intentionally loading this error via a script file to make
293+
// sure it is 1) not caught by instrumentation 2) doesn't trigger
294+
// "Script error"
295+
var script = document.createElement('script');
296+
script.src = 'throw-object.js';
297+
script.onload = function () {
298+
done();
299+
};
300+
document.head.appendChild(script);
301+
},
302+
function () {
303+
var ravenData = iframe.contentWindow.ravenData[0];
304+
assert.equal(ravenData.exception.values[0].type, undefined);
305+
assert.equal(ravenData.exception.values[0].value, '[object Object]');
306+
assert.equal(ravenData.exception.values[0].stacktrace.frames.length, 1); // always 1 because thrown objects can't provide > 1 frame
307+
308+
// some browsers extract proper url, line, and column for thrown objects
309+
// but not all - falls back to frame url
310+
assert.match(ravenData.exception.values[0].stacktrace.frames[0].filename, /\/test\/integration\//);
311+
assert.match(ravenData.exception.values[0].stacktrace.frames[0]['function'], /\?|global code/);
312+
}
313+
);
314+
});
315+
316+
it('should catch thrown errors', function (done) {
317+
var iframe = this.iframe;
318+
319+
iframeExecute(iframe, done,
320+
function () {
321+
// intentionally loading this error via a script file to make
322+
// sure it is 1) not caught by instrumentation 2) doesn't trigger
323+
// "Script error"
324+
var script = document.createElement('script');
325+
script.src = 'throw-error.js';
326+
script.onload = function () {
327+
done();
328+
};
329+
document.head.appendChild(script);
330+
},
331+
function () {
332+
var ravenData = iframe.contentWindow.ravenData[0];
333+
assert.match(ravenData.exception.values[0].type, /^Error/);
334+
assert.match(ravenData.exception.values[0].value, /realError$/);
335+
assert.isAbove(ravenData.exception.values[0].stacktrace.frames.length, 0); // 1 or 2 depending on platform
336+
assert.match(ravenData.exception.values[0].stacktrace.frames[0].filename, /\/test\/integration\/throw-error\.js/)
337+
assert.match(ravenData.exception.values[0].stacktrace.frames[0]['function'], /\?|global code/);
338+
}
339+
);
340+
});
341+
259342
it('should NOT catch an exception already caught via Raven.wrap', function (done) {
260343
var iframe = this.iframe;
261344

‎test/integration/throw-error.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
function throwRealError() {
2+
throw new Error('realError');
3+
}
4+
5+
throwRealError();

‎test/integration/throw-object.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
function throwStringError() {
2+
// never do this; just making sure Raven.js handles this case
3+
// gracefully
4+
throw {error: 'stuff is broken'};
5+
}
6+
7+
throwStringError();

‎test/integration/throw-string.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
function throwStringError() {
2+
throw 'stringError';
3+
}
4+
5+
throwStringError();

‎vendor/TraceKit/tracekit.js

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
'use strict';
22

3+
var utils = require('../../src/utils');
4+
35
/*
46
TraceKit - Cross brower stack traces
57
@@ -26,7 +28,7 @@ var _slice = [].slice;
2628
var UNKNOWN_FUNCTION = '?';
2729

2830
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error#Error_types
29-
var ERROR_TYPES_RE = /^(?:Uncaught (?:exception: )?)?((?:Eval|Internal|Range|Reference|Syntax|Type|URI)Error): ?(.*)$/;
31+
var ERROR_TYPES_RE = /^(?:[Uu]ncaught (?:exception: )?)?(?:((?:Eval|Internal|Range|Reference|Syntax|Type|URI|)Error): )?(.*)$/;
3032

3133
function getLocationHref() {
3234
if (typeof document === 'undefined' || typeof document.location === 'undefined')
@@ -35,6 +37,7 @@ function getLocationHref() {
3537
return document.location.href;
3638
}
3739

40+
3841
/**
3942
* TraceKit.report: cross-browser processing of unhandled exceptions
4043
*
@@ -152,7 +155,9 @@ TraceKit.report = (function reportModuleWrapper() {
152155
if (lastExceptionStack) {
153156
TraceKit.computeStackTrace.augmentStackTraceWithInitialElement(lastExceptionStack, url, lineNo, message);
154157
processLastException();
155-
} else if (ex) {
158+
} else if (ex && utils.isError(ex)) {
159+
// non-string `ex` arg; attempt to extract stack trace
160+
156161
// New chrome and blink send along a real error object
157162
// Let's just report that like a normal error.
158163
// See: https://mikewest.org/2013/08/debugging-runtime-errors-with-window-onerror
@@ -595,7 +600,6 @@ TraceKit.computeStackTrace = (function computeStackTraceWrapper() {
595600
throw e;
596601
}
597602
}
598-
599603
return {
600604
'name': ex.name,
601605
'message': ex.message,

0 commit comments

Comments
 (0)