Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Commit

Permalink
fix(*): correctly detect Error instances from different contexts
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Thomas Grainger committed Apr 27, 2017
1 parent dcc57f1 commit a435740
Show file tree
Hide file tree
Showing 8 changed files with 62 additions and 4 deletions.
1 change: 1 addition & 0 deletions src/.eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
"isNumber": false,
"isNumberNaN": false,
"isDate": false,
"isError": false,
"isArray": false,
"isFunction": false,
"isRegExp": false,
Expand Down
19 changes: 19 additions & 0 deletions src/Angular.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
isNumber,
isNumberNaN,
isDate,
isError,
isArray,
isFunction,
isRegExp,
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -3098,7 +3098,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
}
linkQueue = null;
}).catch(function(error) {
if (error instanceof Error) {
if (isError(error)) {
$exceptionHandler(error);
}
});
Expand Down
2 changes: 1 addition & 1 deletion src/ng/log.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/ng/q.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions test/.eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
"isNumber": false,
"isNumberNaN": false,
"isDate": false,
"isError": false,
"isArray": false,
"isFunction": false,
"isRegExp": false,
Expand Down
37 changes: 37 additions & 0 deletions test/AngularSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion test/ng/qSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit a435740

Please sign in to comment.