From a43574052e9775cbc1d7dd8a086752c979b0f020 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Thu, 30 Mar 2017 09:42:29 +0100 Subject: [PATCH] fix(*): correctly detect Error instances from different contexts 12345678901234567890123456789012345678901234567890123456789012345678901234567890 Errors thrown from different contexts (such as an iframe or webworker) are not detected as Error instances and handled accordingly. This commit fixes it by introducing an isError() helper, that is able to correctly detect such instances. Fixes #15868 --- src/.eslintrc.json | 1 + src/Angular.js | 19 +++++++++++++++++++ src/ng/compile.js | 2 +- src/ng/log.js | 2 +- src/ng/q.js | 2 +- test/.eslintrc.json | 1 + test/AngularSpec.js | 37 +++++++++++++++++++++++++++++++++++++ test/ng/qSpec.js | 2 +- 8 files changed, 62 insertions(+), 4 deletions(-) diff --git a/src/.eslintrc.json b/src/.eslintrc.json index 4c240b733bf9..3f8163dcc9af 100644 --- a/src/.eslintrc.json +++ b/src/.eslintrc.json @@ -50,6 +50,7 @@ "isNumber": false, "isNumberNaN": false, "isDate": false, + "isError": false, "isArray": false, "isFunction": false, "isRegExp": false, diff --git a/src/Angular.js b/src/Angular.js index 42942d432c01..2c5ef52116c2 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -45,6 +45,7 @@ isNumber, isNumberNaN, isDate, + isError, isArray, isFunction, isRegExp, @@ -673,6 +674,24 @@ function isDate(value) { */ var isArray = Array.isArray; +/** + * @description + * Determines if a reference is an `Error`. + * Loosely based on https://www.npmjs.com/package/iserror + * + * @param {*} value Reference to check. + * @returns {boolean} True if `value` is an `Error`. + */ +function isError(value) { + var tag = toString.call(value); + switch (tag) { + case '[object Error]': return true; + case '[object Exception]': return true; + case '[object DOMException]': return true; + default: return value instanceof Error; + } +} + /** * @ngdoc function * @name angular.isFunction diff --git a/src/ng/compile.js b/src/ng/compile.js index 0405d3be76d3..a2d9730c3920 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -3098,7 +3098,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } linkQueue = null; }).catch(function(error) { - if (error instanceof Error) { + if (isError(error)) { $exceptionHandler(error); } }); diff --git a/src/ng/log.js b/src/ng/log.js index 61aef0887ecc..51f6f6ec6c29 100644 --- a/src/ng/log.js +++ b/src/ng/log.js @@ -132,7 +132,7 @@ function $LogProvider() { }; function formatError(arg) { - if (arg instanceof Error) { + if (isError(arg)) { if (arg.stack && formatStackTrace) { arg = (arg.message && arg.stack.indexOf(arg.message) === -1) ? 'Error: ' + arg.message + '\n' + arg.stack diff --git a/src/ng/q.js b/src/ng/q.js index 8aff2e0cfe21..03cb238cd7dd 100644 --- a/src/ng/q.js +++ b/src/ng/q.js @@ -381,7 +381,7 @@ function qFactory(nextTick, exceptionHandler, errorOnUnhandledRejections) { if (!toCheck.pur) { toCheck.pur = true; var errorMessage = 'Possibly unhandled rejection: ' + toDebugString(toCheck.value); - if (toCheck.value instanceof Error) { + if (isError(toCheck.value)) { exceptionHandler(toCheck.value, errorMessage); } else { exceptionHandler(errorMessage); diff --git a/test/.eslintrc.json b/test/.eslintrc.json index 007b458949ad..37a263e305f2 100644 --- a/test/.eslintrc.json +++ b/test/.eslintrc.json @@ -64,6 +64,7 @@ "isNumber": false, "isNumberNaN": false, "isDate": false, + "isError": false, "isArray": false, "isFunction": false, "isRegExp": false, diff --git a/test/AngularSpec.js b/test/AngularSpec.js index a44660ed51b1..a5589c1fe00f 100644 --- a/test/AngularSpec.js +++ b/test/AngularSpec.js @@ -1880,6 +1880,43 @@ describe('angular', function() { }); }); + describe('isError', function() { + function testErrorFromDifferentContext(createError) { + var iframe = document.createElement('iframe'); + document.body.appendChild(iframe); + try { + var error = createError(iframe.contentWindow); + expect(isError(error)).toBe(true); + } finally { + iframe.parentElement.removeChild(iframe); + } + } + + it('should not assume objects are errors', function() { + var fakeError = { message: 'A fake error', stack: 'no stack here'}; + expect(isError(fakeError)).toBe(false); + }); + + it('should detect simple error instances', function() { + expect(isError(new Error())).toBe(true); + }); + + it('should detect errors from another context', function() { + testErrorFromDifferentContext(function(win) { + return new win.Error(); + }); + }); + + it('should detect DOMException errors from another context', function() { + testErrorFromDifferentContext(function(win) { + try { + win.document.querySelectorAll(''); + } catch (e) { + return e; + } + }); + }); + }); describe('isRegExp', function() { it('should return true for RegExp object', function() { diff --git a/test/ng/qSpec.js b/test/ng/qSpec.js index f3a983100e41..23c939e689f4 100644 --- a/test/ng/qSpec.js +++ b/test/ng/qSpec.js @@ -37,7 +37,7 @@ describe('q', function() { // The following private functions are used to help with logging for testing invocation of the // promise callbacks. function _argToString(arg) { - return (typeof arg === 'object' && !(arg instanceof Error)) ? toJson(arg) : '' + arg; + return (typeof arg === 'object' && !(isError(arg))) ? toJson(arg) : '' + arg; } function _argumentsToString(args) {