From 538d1a57518187f3c746c5fb3e9ade5910592df7 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Mon, 27 Feb 2017 17:53:05 -0800 Subject: [PATCH 1/3] Fix bad message pulled from thrown strings Fixes #871 --- test/integration/test.js | 51 ++++++++++++++++++++++++++++++++ test/integration/throw-error.js | 5 ++++ test/integration/throw-string.js | 5 ++++ vendor/TraceKit/tracekit.js | 9 ++++-- 4 files changed, 67 insertions(+), 3 deletions(-) create mode 100644 test/integration/throw-error.js create mode 100644 test/integration/throw-string.js diff --git a/test/integration/test.js b/test/integration/test.js index 4d8a0a07f601..d56e481df229 100644 --- a/test/integration/test.js +++ b/test/integration/test.js @@ -256,6 +256,57 @@ describe('integration', function () { ); }); + it('should catch thrown strings', function (done) { + var iframe = this.iframe; + + iframeExecute(iframe, done, + function () { + // intentionally loading this error via a script file to make + // sure it is 1) not caught by instrumentation 2) doesn't trigger + // "Script error" + var script = document.createElement('script'); + script.src = 'throw-string.js'; + script.onload = function () { + done(); + }; + document.head.appendChild(script); + }, + function () { + var ravenData = iframe.contentWindow.ravenData[0]; + assert.match(ravenData.exception.values[0].value, /stringError$/); + assert.equal(ravenData.exception.values[0].stacktrace.frames.length, 1); // always 1 because thrown strings can't provide > 1 frame + assert.match(ravenData.exception.values[0].stacktrace.frames[0].filename, /\/test\/integration\/throw-string\.js/) + assert.match(ravenData.exception.values[0].stacktrace.frames[0]['function'], /\?|global code/); + } + ); + }); + + it('should catch thrown errors', function (done) { + var iframe = this.iframe; + + iframeExecute(iframe, done, + function () { + // intentionally loading this error via a script file to make + // sure it is 1) not caught by instrumentation 2) doesn't trigger + // "Script error" + var script = document.createElement('script'); + script.src = 'throw-error.js'; + script.onload = function () { + done(); + }; + document.head.appendChild(script); + }, + function () { + var ravenData = iframe.contentWindow.ravenData[0]; + assert.match(ravenData.exception.values[0].type, /^Error/); + assert.match(ravenData.exception.values[0].value, /realError$/); + assert.isAbove(ravenData.exception.values[0].stacktrace.frames.length, 0); // 1 or 2 depending on platform + assert.match(ravenData.exception.values[0].stacktrace.frames[0].filename, /\/test\/integration\/throw-error\.js/) + assert.match(ravenData.exception.values[0].stacktrace.frames[0]['function'], /\?|global code/); + } + ); + }); + it('should NOT catch an exception already caught via Raven.wrap', function (done) { var iframe = this.iframe; diff --git a/test/integration/throw-error.js b/test/integration/throw-error.js new file mode 100644 index 000000000000..c3cdc14ad3a6 --- /dev/null +++ b/test/integration/throw-error.js @@ -0,0 +1,5 @@ +function throwRealError() { + throw new Error('realError'); +} + +throwRealError(); diff --git a/test/integration/throw-string.js b/test/integration/throw-string.js new file mode 100644 index 000000000000..0904692d6c00 --- /dev/null +++ b/test/integration/throw-string.js @@ -0,0 +1,5 @@ +function throwStringError() { + throw 'stringError'; +} + +throwStringError(); diff --git a/vendor/TraceKit/tracekit.js b/vendor/TraceKit/tracekit.js index b602c9cc1b13..bae0170290f0 100644 --- a/vendor/TraceKit/tracekit.js +++ b/vendor/TraceKit/tracekit.js @@ -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): ?(.*)$/; function getLocationHref() { if (typeof document === 'undefined' || typeof document.location === 'undefined') @@ -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') { + // intentionally a "weak" object check here - want to accept + // Error-like objects just in case (previously *any* truthy ex value) + // triggered this branch + // New chrome and blink send along a real error object // Let's just report that like a normal error. // See: https://mikewest.org/2013/08/debugging-runtime-errors-with-window-onerror @@ -595,7 +599,6 @@ TraceKit.computeStackTrace = (function computeStackTraceWrapper() { throw e; } } - return { 'name': ex.name, 'message': ex.message, From 2710ccc4f44235a915aab2f181bbf6ba20c954b8 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Wed, 1 Mar 2017 13:32:57 -0800 Subject: [PATCH 2/3] Use non-string check instead of object check --- vendor/TraceKit/tracekit.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/vendor/TraceKit/tracekit.js b/vendor/TraceKit/tracekit.js index bae0170290f0..d642d70f0f4d 100644 --- a/vendor/TraceKit/tracekit.js +++ b/vendor/TraceKit/tracekit.js @@ -35,6 +35,7 @@ function getLocationHref() { return document.location.href; } + /** * TraceKit.report: cross-browser processing of unhandled exceptions * @@ -152,10 +153,8 @@ TraceKit.report = (function reportModuleWrapper() { if (lastExceptionStack) { TraceKit.computeStackTrace.augmentStackTraceWithInitialElement(lastExceptionStack, url, lineNo, message); processLastException(); - } else if (ex && typeof ex === 'object') { - // intentionally a "weak" object check here - want to accept - // Error-like objects just in case (previously *any* truthy ex value) - // triggered this branch + } else if (ex && {}.toString.call(ex) !== '[object String]') { + // non-string `ex` arg; attempt to extract stack trace // New chrome and blink send along a real error object // Let's just report that like a normal error. From 7619d2dcfe2fc29b13fccbf1b1b47b82c45dcf63 Mon Sep 17 00:00:00 2001 From: Ben Vinegar Date: Wed, 1 Mar 2017 14:52:43 -0800 Subject: [PATCH 3/3] Strict error check using isError, fix regexes --- src/raven.js | 18 +++++------------ src/utils.js | 20 +++++++++++++++++++ test/integration/test.js | 34 +++++++++++++++++++++++++++++++- test/integration/throw-object.js | 7 +++++++ vendor/TraceKit/tracekit.js | 6 ++++-- 5 files changed, 69 insertions(+), 16 deletions(-) create mode 100644 src/utils.js create mode 100644 test/integration/throw-object.js diff --git a/src/raven.js b/src/raven.js index 2f54ac747bbe..4fb7be8ec1b7 100644 --- a/src/raven.js +++ b/src/raven.js @@ -3,6 +3,11 @@ var TraceKit = require('../vendor/TraceKit/tracekit'); var RavenConfigError = require('./configError'); +var utils = require('./utils'); + +var isError = utils.isError, + isObject = utils.isObject; + var stringify = require('json-stringify-safe'); var wrapConsoleMethod = require('./console').wrapMethod; @@ -1655,25 +1660,12 @@ function isString(what) { return objectPrototype.toString.call(what) === '[object String]'; } -function isObject(what) { - return typeof what === 'object' && what !== null; -} function isEmptyObject(what) { for (var _ in what) return false; // eslint-disable-line guard-for-in, no-unused-vars return true; } -// Sorta yanked from https://github.com/joyent/node/blob/aa3b4b4/lib/util.js#L560 -// with some tiny modifications -function isError(what) { - var toString = objectPrototype.toString.call(what); - return isObject(what) && - toString === '[object Error]' || - toString === '[object Exception]' || // Firefox NS_ERROR_FAILURE Exceptions - what instanceof Error; -} - function each(obj, callback) { var i, j; diff --git a/src/utils.js b/src/utils.js new file mode 100644 index 000000000000..431761c97211 --- /dev/null +++ b/src/utils.js @@ -0,0 +1,20 @@ +'use strict'; + +function isObject(what) { + return typeof what === 'object' && what !== null; +} + +// Sorta yanked from https://github.com/joyent/node/blob/aa3b4b4/lib/util.js#L560 +// with some tiny modifications +function isError(what) { + var toString = {}.toString.call(what); + return isObject(what) && + toString === '[object Error]' || + toString === '[object Exception]' || // Firefox NS_ERROR_FAILURE Exceptions + what instanceof Error; +} + +module.exports = { + isObject: isObject, + isError: isError +}; \ No newline at end of file diff --git a/test/integration/test.js b/test/integration/test.js index d56e481df229..93b252513820 100644 --- a/test/integration/test.js +++ b/test/integration/test.js @@ -275,7 +275,39 @@ describe('integration', function () { var ravenData = iframe.contentWindow.ravenData[0]; assert.match(ravenData.exception.values[0].value, /stringError$/); assert.equal(ravenData.exception.values[0].stacktrace.frames.length, 1); // always 1 because thrown strings can't provide > 1 frame - assert.match(ravenData.exception.values[0].stacktrace.frames[0].filename, /\/test\/integration\/throw-string\.js/) + + // some browsers extract proper url, line, and column for thrown strings + // but not all - falls back to frame url + assert.match(ravenData.exception.values[0].stacktrace.frames[0].filename, /\/test\/integration\//); + assert.match(ravenData.exception.values[0].stacktrace.frames[0]['function'], /\?|global code/); + } + ); + }); + + it('should catch thrown objects', function (done) { + var iframe = this.iframe; + + iframeExecute(iframe, done, + function () { + // intentionally loading this error via a script file to make + // sure it is 1) not caught by instrumentation 2) doesn't trigger + // "Script error" + var script = document.createElement('script'); + script.src = 'throw-object.js'; + script.onload = function () { + done(); + }; + document.head.appendChild(script); + }, + function () { + var ravenData = iframe.contentWindow.ravenData[0]; + assert.equal(ravenData.exception.values[0].type, undefined); + assert.equal(ravenData.exception.values[0].value, '[object Object]'); + assert.equal(ravenData.exception.values[0].stacktrace.frames.length, 1); // always 1 because thrown objects can't provide > 1 frame + + // some browsers extract proper url, line, and column for thrown objects + // but not all - falls back to frame url + assert.match(ravenData.exception.values[0].stacktrace.frames[0].filename, /\/test\/integration\//); assert.match(ravenData.exception.values[0].stacktrace.frames[0]['function'], /\?|global code/); } ); diff --git a/test/integration/throw-object.js b/test/integration/throw-object.js new file mode 100644 index 000000000000..1e558bde3c43 --- /dev/null +++ b/test/integration/throw-object.js @@ -0,0 +1,7 @@ +function throwStringError() { + // never do this; just making sure Raven.js handles this case + // gracefully + throw {error: 'stuff is broken'}; +} + +throwStringError(); diff --git a/vendor/TraceKit/tracekit.js b/vendor/TraceKit/tracekit.js index d642d70f0f4d..ef63411b4b4f 100644 --- a/vendor/TraceKit/tracekit.js +++ b/vendor/TraceKit/tracekit.js @@ -1,5 +1,7 @@ 'use strict'; +var utils = require('../../src/utils'); + /* TraceKit - Cross brower stack traces @@ -26,7 +28,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 = /^(?:[Uu]ncaught (?:exception: )?)?(?:((?:Eval|Internal|Range|Reference|Syntax|Type|URI|)Error): )?(.*)$/; function getLocationHref() { if (typeof document === 'undefined' || typeof document.location === 'undefined') @@ -153,7 +155,7 @@ TraceKit.report = (function reportModuleWrapper() { if (lastExceptionStack) { TraceKit.computeStackTrace.augmentStackTraceWithInitialElement(lastExceptionStack, url, lineNo, message); processLastException(); - } else if (ex && {}.toString.call(ex) !== '[object String]') { + } else if (ex && utils.isError(ex)) { // non-string `ex` arg; attempt to extract stack trace // New chrome and blink send along a real error object