fix(ngMock window.inject): Remove Error 'stack' property changes

[wizardwerdna]

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 solution passing those tests. The solution provides an object that
mimics the old Error object, with the additional stack information, where
possible (Error is not standardized on stack, line and file information), and
then captures the toString function from the Error object prototype.

An exception is IE8 or less, which fails to yield meaningful messages
under both the status quo and this instant commit. Accordingly, the test
memorializes the current IE8 behavior as a passing example.

modified: src/ngMock/angular-mocks.js
modified: test/ngMock/angular-mocksSpec.js

[petebacondarwin]

Hi @wizardwerdna - have you signed the CLA recently or in the past?

[wizardwerdna]

I went ahead and re-signed the CLA. You should be good to go.

[alekstorm]

+1, this is hurting us quite badly. Please merge ASAP.

[mattdanskey]

+1. Ran into this issue with PhantomJS and got
TypeError: Attempted to assign to readonly property.
After commenting out line 2107 in v1.2.1, the message became a much more helpful,
Error: [$injector:unpr] Unknown provider: base64Provider <- base64 <- authenticator

[wizardwerdna]

Indeed, to those of us who do test-first development, the quality of error
messages in jasmine are essential to efficient development. Remember, we
can't write a line of production code until we get a failing test. When
many of our non-matcher messaes are all "Attempted to assign to readonly
property," it puts a huge crimp in our work, particularly at the outset
when most messages are non-matcher errors.

[trashhalo]

This bug was a pretty big headache for my team. If anyone is interested it cleanly cherry picks onto the the 1.2.0 tag.

Build with the change from this pull request: https://gist.github.com/trashhalo/7660093
Diff between my gist and cdn code: http://www.diffnow.com/?report=xw0vo

[Kazark]

+1 this bit me too

[fidian]

I was about to submit a pull request for this exact same issue. +1 for me and my team. As it is, I will be manually applying the patch as the official packages don't have this extremely useful fix. I look forward to seeing this in 1.2.4.

[booleanbetrayal]

+1 ... very annoying problem to have.

[sulmanen]

didn't find in 1.2.4, patched manually. works. thnx!

[laurelnaiad]

+1 for merging pre-1.2.5

[pmccaughtry]

I am seeing this in 1.2.5. Is anyone else and if so, is there a solution?

Thank you

[laurelnaiad]

Look at the pull request and patch manually. Just a few lines of code.

[pmccaughtry]

Thank you stu-salsbury. Worked perfectly.

[laurelnaiad]

Np @pmccaughtry. You reminded me I forgot to re-patch after updating to 1.2.5. Core team is there anything preventing this from being merged? If so can you let us know. Maybe someone wil beable to pitch in!

[laurelnaiad]

Just thought of something.. I'm on the road so can't test this.. I don't think I saw the problem after updating to 1.2.5. I wonder if its because I added es5shim on my unit tests in order to get chai expect to work?

[munsellj]

+1. Encountered same issue writing unit tests (Grunt, Karma, Jasmine) and running them in PhantomJS using Angular v1.2.5. Running in Chrome produces the correct error stack trace for a failed test.

[jimmykane]

Still here on safari latest and angular 1.2.5

[wizardwerdna]

@jimmykane This solution is targeted for a merge, but apparently isn't a strong priority at the moment. In the meanwhile, you can readily use patched versions of angular-mocks from a number of reputable sources, such as new relic and others, or manually implement it yourself.

[vojtajina]

Guys sorry for delays... It's merged as 7e91645.
I changed the tests a bit - we don't need to test the stupid IE8 behavior.

[markscholtz]

This seems to break Jasmine 2.x's toThrowError() function. The original exception is no longer thrown and the new error object does not inherit from Error.

Given a Jasmine expectation that looks like:

expect(myObject.throwMyExpectedError()).toThrowError(MyExpectedError);

Where the implementation of throwMyExpectedError throws an exception inheriting from Error, I see something along these lines from my Jasmine test output:

...
Expected function to throw an Error, but it threw ({ message: 'my error message', name: 'MyExpectedError', stack: 'the augmented call stack ...'}).

That's a bit of a hand-wavy example. I can try and provide more information and perhaps write a test to prove this, but I wanted to raise it first to see if anyone had any thoughts.

[petebacondarwin]

@markscholtz - if you feel we have an outstanding issue can you please create a new issue for it and reference this one?