From 7e916455b36dc9ca4d4afc1e44cade90006d00e3 Mon Sep 17 00:00:00 2001 From: "Andrew C. Greenberg" Date: Tue, 19 Nov 2013 22:53:33 -0800 Subject: [PATCH] fix(ngMock window.inject): Remove Error 'stack' property changes Recent browsers, particularly PhantomJS 1.9.2 and Safari 7.0 treat the stack property as non-configurable and unwritable. Because window.inject captures the stack at the time of the inject, and attempts to insert it into a captured throw from the injected function by modifying e.stack, a meaningless error message and stack is thrown instead. This commit inserts two tests exposing the problem, and implements a proposed solution that builds a new error-like object that mimicks the old Error object, but with the additional stack information, and captures the toString function from the Error object prototype. This appears to work for the browsers suppoerted here. --- src/ngMock/angular-mocks.js | 18 +++++++++++++++++- test/ngMock/angular-mocksSpec.js | 17 +++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/ngMock/angular-mocks.js b/src/ngMock/angular-mocks.js index fb7ef0573c20..b53b4e0264d8 100644 --- a/src/ngMock/angular-mocks.js +++ b/src/ngMock/angular-mocks.js @@ -2079,6 +2079,20 @@ if(window.jasmine || window.mocha) { * * @param {...Function} fns any number of functions which will be injected using the injector. */ + + + + var ErrorAddingDeclarationLocationStack = function(e, errorForStack) { + this.message = e.message; + this.name = e.name; + if (e.line) this.line = e.line; + if (e.sourceId) this.sourceId = e.sourceId; + if (e.stack && errorForStack) + this.stack = e.stack + '\n' + errorForStack.stack; + if (e.stackArray) this.stackArray = e.stackArray; + }; + ErrorAddingDeclarationLocationStack.prototype.toString = Error.prototype.toString; + window.inject = angular.mock.inject = function() { var blockFns = Array.prototype.slice.call(arguments, 0); var errorForStack = new Error('Declaration Location'); @@ -2099,7 +2113,9 @@ if(window.jasmine || window.mocha) { injector.invoke(blockFns[i] || angular.noop, this); /* jshint +W040 */ } catch (e) { - if(e.stack && errorForStack) e.stack += '\n' + errorForStack.stack; + if (e.stack && errorForStack) { + throw new ErrorAddingDeclarationLocationStack(e, errorForStack); + } throw e; } finally { errorForStack = null; diff --git a/test/ngMock/angular-mocksSpec.js b/test/ngMock/angular-mocksSpec.js index ef7e88fae6f8..6c096471d2be 100644 --- a/test/ngMock/angular-mocksSpec.js +++ b/test/ngMock/angular-mocksSpec.js @@ -862,6 +862,23 @@ describe('ngMock', function() { expect(log).toEqual('module;inject;') }); }); + + // We don't run the following tests on IE8. + // IE8 throws "Object does not support this property or method." error, + // when thrown from a function defined on window (which `inject` is). + if (msie <= 8) return; + + it('should not change thrown Errors', function() { + expect(function(){ + throw new Error('test message'); + }).toThrow('test message'); + }); + + it('should not change thrown strings', function(){ + expect(function(){ + throw 'test message'; + }).toThrow('test message'); + }); }); });