Fix bad message pulled from thrown strings and objects #872
Conversation
vendor/TraceKit/tracekit.js
Outdated
| @@ -26,7 +26,7 @@ var _slice = [].slice; | |||
| var UNKNOWN_FUNCTION = '?'; | |||
|
|
|||
| // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error#Error_types | |||
| var ERROR_TYPES_RE = /^(?:Uncaught (?:exception: )?)?((?:Eval|Internal|Range|Reference|Syntax|Type|URI)Error): ?(.*)$/; | |||
| var ERROR_TYPES_RE = /^(?:Uncaught (?:exception: )?)?((?:Eval|Internal|Range|Reference|Syntax|Type|URI|)Error): ?(.*)$/; | |||
There was a problem hiding this comment.
This was not matching on just Error, e.g. throw new Error('test').
There was a problem hiding this comment.
would be kind of nice to have unit tests for this regex as kinda-documentation, but i don't think its important and the other tests cover it
9c8bb49 to
538d1a5
Compare
vendor/TraceKit/tracekit.js
Outdated
| @@ -26,7 +26,7 @@ var _slice = [].slice; | |||
| var UNKNOWN_FUNCTION = '?'; | |||
|
|
|||
| // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error#Error_types | |||
| var ERROR_TYPES_RE = /^(?:Uncaught (?:exception: )?)?((?:Eval|Internal|Range|Reference|Syntax|Type|URI)Error): ?(.*)$/; | |||
| var ERROR_TYPES_RE = /^(?:Uncaught (?:exception: )?)?((?:Eval|Internal|Range|Reference|Syntax|Type|URI|)Error): ?(.*)$/; | |||
There was a problem hiding this comment.
would be kind of nice to have unit tests for this regex as kinda-documentation, but i don't think its important and the other tests cover it
It's partially covered here: https://github.com/getsentry/raven-js/blob/master/test/vendor/tracekit.test.js#L151 |
vendor/TraceKit/tracekit.js
Outdated
| @@ -152,7 +152,11 @@ TraceKit.report = (function reportModuleWrapper() { | |||
| if (lastExceptionStack) { | |||
| TraceKit.computeStackTrace.augmentStackTraceWithInitialElement(lastExceptionStack, url, lineNo, message); | |||
| processLastException(); | |||
| } else if (ex) { | |||
| } else if (ex && typeof ex === 'object') { | |||
There was a problem hiding this comment.
Couldn't we end up with a string (or even an error-like string) here? This prints "string":
window.onerror = function (message, filename, lineno, colno, ex) {
console.log(typeof ex);
};
var x = "hello";
x.stack = ['a'];
throw x;There was a problem hiding this comment.
Yes, but we didn't handle that case either.
There was a problem hiding this comment.
But you did remind me of one thing, which is that you could do throw new String('lol') that would evaluate as an object.
Gonna revisit.
|
Okay, the scope of this has increased once I realized there was a gap in coverage. It now covers both uncaught thrown strings AND objects. |
Fixes #871
At some point browsers must have started sending the 5th argument to window.onerror for non-errors (Safari 10+? Chrome X?), which is why this error popped up in the past year and not before.
cc @getsentry/javascript