Merge pull request #872 from getsentry/fix-undefined

Fix bad message pulled from thrown strings and objects

Author: benvinegar

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;
 

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
+};

test/integration/test.js

@@ -256,6 +256,89 @@ 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
+
+                    // 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/);
+                }
+            );
+        });
+
+        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;
 

test/integration/throw-error.js

@@ -0,0 +1,5 @@
+function throwRealError() {
+    throw new Error('realError');
+}
+
+throwRealError();

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();

test/integration/throw-string.js

@@ -0,0 +1,5 @@
+function throwStringError() {
+    throw 'stringError';
+}
+
+throwStringError();

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')
@@ -35,6 +37,7 @@ function getLocationHref() {
     return document.location.href;
 }
 
+
 /**
  * TraceKit.report: cross-browser processing of unhandled exceptions
  *
@@ -152,7 +155,9 @@ TraceKit.report = (function reportModuleWrapper() {
         if (lastExceptionStack) {
             TraceKit.computeStackTrace.augmentStackTraceWithInitialElement(lastExceptionStack, url, lineNo, message);
             processLastException();
-        } else if (ex) {
+        } 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
             // Let's just report that like a normal error.
             // See: https://mikewest.org/2013/08/debugging-runtime-errors-with-window-onerror
@@ -595,7 +600,6 @@ TraceKit.computeStackTrace = (function computeStackTraceWrapper() {
                 throw e;
             }
         }
-
         return {
             'name': ex.name,
             'message': ex.message,