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

ngMock window.inject() does not get current stack trace correctly on PhantomJS and IE10+ #13591

Closed
jurko-gospodnetic opened this issue Dec 19, 2015 · 2 comments

Comments

@jurko-gospodnetic
Copy link
Contributor

Overview of the Issue
ngMock window.inject() function collects the current stack trace by expecting to find it in a locally created Error object's stack property. This however does not hold true in case when using a browser that fills an Error object's stack information only when that object gets thrown.

Motivation for or Use Case
The bug was found while researching why karma reports invalid error information when using PhantomJS in cases like this:

it('the holy crusade', inject(function() {
  var x = {};
  x.holyGrail();
}));

where the ngMock inject() implementation would incorrectly add the word undefined (read too early from the Error object's stack property) to the end of the collected error stack trace information, thus causing the main error description (gotten by parsing the collected error stack trace information, assuming it has specific structure) to be reported back to karma as undefined.

Angular Version(s)

  • reproduced using angular version 1.4.8
  • current master version still has the exact same code so should be affected by the same bug

Browsers and Operating System

  • reproduced using PhantomJS browser
  • if MSDN docs are to be believed, should not work when used in IE browsers as well
  • reproduced using Ubuntu 15.10, but should not be OS specific

Reproduce the Error
Here's a test for the angular ngMock test suite that can be used to reproduce the error:

      describe('when called outside of test spec context and inject callback throws an Error', function () {
        function testCaller() {
          return inject(function () {
            throw new Error();
          });
        }
        var throwErrorFromInjectCallback = testCaller();

        it('should update thrown Error stack with inject call location', function () {
          try {
            throwErrorFromInjectCallback();
          } catch (e) {
            expect(e.stack).toMatch('testCaller');
          }
        });
      });

Note that this whole functionality is completely untested and, at least to me, seems buggy or unfinished, but I'll open separate issues for that, while this one is targeted specifically to the error stack information constructed in ngMock's window.inject().

Suggest a Fix

  • after creating the Error object in question, throw it and catch it in case it does not already have stack information stored on it
@jurko-gospodnetic jurko-gospodnetic changed the title nngMock window.inject() does not get current stack trace correctly on PhantomJS and possibly IE ngMock window.inject() does not get current stack trace correctly on PhantomJS and possibly IE Dec 19, 2015
jurko-gospodnetic added a commit to jurko-gospodnetic/angular.js that referenced this issue Dec 19, 2015
Angular's ngMock inject() function, when called outside of a test spec
context will not directly call the provided callback but will instead
return a wrapper function to call the provided function at a later time,
presumably while in some test spec context. And if that is the case,
Angular would like to include the information on the inject() calling
location to be included in the thrown error's stack trace information,
so it manually appends it to the ones included in the actual error's
stack trace.

The added test makes sure this functionality works as expected.

Closes angular#13591
jurko-gospodnetic added a commit to jurko-gospodnetic/angular.js that referenced this issue Dec 20, 2015
Angular's ngMock inject() function, when called outside of a test spec
context will not directly call the provided callback but will instead
return a wrapper function to call the provided function at a later time,
presumably while in some test spec context. And if that is the case,
Angular would like to include the information on the inject() calling
location to be included in the thrown error's stack trace information,
so it manually appends it to the ones included in the actual error's
stack trace.

The added test makes sure this functionality works as expected.

Closes angular#13591
jurko-gospodnetic added a commit to jurko-gospodnetic/angular.js that referenced this issue Dec 20, 2015
Angular's ngMock inject() function, when called outside of a test spec
context will not directly call the provided callback but will instead
return a wrapper function to call the provided function at a later time,
presumably while in some test spec context. And if that is the case,
Angular would like to include the information on the inject() calling
location to be included in the thrown error's stack trace information,
so it manually appends it to the ones included in the actual error's
stack trace.

The added test makes sure this functionality:
- works as expected in browsers supporting JavaScript stack trace
  collection, e.g. Chrome, Firefox, IE10+, Opera & PhantomJS
- does not add any bogus stack track information in browsers that do
  not support JavaScript stack trace collection, e.g. IE8 or IE9

Closes angular#13591
jurko-gospodnetic added a commit to jurko-gospodnetic/angular.js that referenced this issue Dec 20, 2015
Angular's ngMock inject() function, when called outside of a test spec
context will not directly call the provided callback but will instead
return a wrapper function to call the provided function at a later time,
presumably while in some test spec context. And if that is the case,
Angular would like to include the information on the inject() calling
location to be included in the thrown error's stack trace information,
so it manually appends it to the ones included in the actual error's
stack trace.

The added test makes sure this functionality:
- works as expected in browsers supporting JavaScript stack trace
  collection, e.g. Chrome, Firefox, IE10+, Opera & PhantomJS
- does not add any bogus stack track information in browsers that do
  not support JavaScript stack trace collection, e.g. IE8 or IE9

Closes angular#13591
jurko-gospodnetic added a commit to jurko-gospodnetic/angular.js that referenced this issue Dec 20, 2015
Angular's ngMock inject() function, when called outside of a test spec
context will not directly call the provided callback but will instead
return a wrapper function to call the provided function at a later time,
presumably while in some test spec context. And if that is the case,
Angular would like to include the information on the inject() calling
location to be included in the thrown error's stack trace information,
so it manually appends it to the ones included in the actual error's
stack trace.

The added test makes sure this functionality:
- works as expected in browsers supporting JavaScript stack trace
  collection, e.g. Chrome, Firefox, IE10+, Opera & PhantomJS
- does not add any bogus stack track information in browsers that do
  not support JavaScript stack trace collection, e.g. IE8 or IE9

Closes angular#13591
jurko-gospodnetic added a commit to jurko-gospodnetic/angular.js that referenced this issue Dec 20, 2015
Angular's ngMock inject() function, when called outside of a test spec
context will not directly call the provided callback but will instead
return a wrapper function to call the provided function at a later time,
presumably while in some test spec context. And if that is the case,
Angular would like to include the information on the inject() calling
location to be included in the thrown error's stack trace information,
so it manually appends it to the ones included in the actual error's
stack trace.

The added test makes sure this functionality:
- works as expected in browsers supporting JavaScript stack trace
  collection, e.g. Chrome, Firefox, IE10+, Opera & PhantomJS
- does not add any bogus stack track information in browsers that do
  not support JavaScript stack trace collection, e.g. IE8 or IE9

Closes angular#13591
@jurko-gospodnetic jurko-gospodnetic changed the title ngMock window.inject() does not get current stack trace correctly on PhantomJS and possibly IE ngMock window.inject() does not get current stack trace correctly on PhantomJS and IE10+ Dec 20, 2015
@jurko-gospodnetic
Copy link
Contributor Author

Pull request #13592 is a fix for this issue for the 1.4.x branch.
Pull request #13593 is a fix for this issue for the master branch.

@jurko-gospodnetic
Copy link
Contributor Author

ping?

@Narretz Narretz added this to the Backlog milestone Jan 5, 2016
jurko-gospodnetic added a commit to jurko-gospodnetic/angular.js that referenced this issue Apr 8, 2016
Angular's ngMock inject() function, when called outside of a test spec
context will not directly call the provided callback but will instead
return a wrapper function to call the provided function at a later time,
presumably while in some test spec context. And if that is the case,
Angular would like to include the information on the inject() calling
location to be included in the thrown error's stack trace information,
so it manually appends it to the ones included in the actual error's
stack trace.

The added test makes sure this functionality:
- works as expected in browsers supporting JavaScript stack trace
  collection, e.g. Chrome, Firefox, IE10+, Opera & PhantomJS
- does not add any bogus stack track information in browsers that do
  not support JavaScript stack trace collection, e.g. IE8 or IE9

Closes angular#13591
jurko-gospodnetic added a commit to jurko-gospodnetic/angular.js that referenced this issue Apr 8, 2016
Angular's ngMock inject() function, when called outside of a test spec
context will not directly call the provided callback but will instead
return a wrapper function to call the provided function at a later time,
presumably while in some test spec context. And if that is the case,
Angular would like to include the information on the inject() calling
location to be included in the thrown error's stack trace information,
so it manually appends it to the ones included in the actual error's
stack trace.

The added test makes sure this functionality:
- works as expected in browsers supporting JavaScript stack trace
  collection, e.g. Chrome, Firefox, IE10+, Opera & PhantomJS
- does not add any bogus stack track information in browsers that do
  not support JavaScript stack trace collection, e.g. IE8 or IE9

Closes angular#13591
@gkalpak gkalpak closed this as completed in 92c3b75 Apr 9, 2016
gkalpak pushed a commit that referenced this issue Apr 9, 2016
…omJS

Add support for collecting current stack trace information in browsers
(e.g. IE10+, PhantomJS) that do not automatically store the current stack trace
information in a newly created `Error` object's `stack` property, but
only add it there once the `Error` gets thrown.

The original implementation works fine in Firefox & Chrome, but fails on IE10+
and PhantomJS where it, for example, breaks Karma's error reporting in cases
when an exception is thrown in a test like the following:

```
it('the holy crusade', inject(function() {
  var x = {};
  x.holyGrail();
}));
```

In this case, the ngMock `inject()` implementation would incorrectly add the
word `undefined` at the end of the collected error stack trace information,
thus causing the main error description to be reported back to Karma as
`undefined`.

The added test makes sure this functionality:

- works as expected in browsers supporting JavaScript stack trace
  collection, e.g. Chrome, Firefox, IE10+, Opera & PhantomJS
- does not add any bogus stack track information in browsers that do
  not support JavaScript stack trace collection, e.g. IE9

Fixes #13591
Closes #13592

Closes #13593
gkalpak pushed a commit that referenced this issue Apr 9, 2016
…omJS

Add support for collecting current stack trace information in browsers
(e.g. IE10+, PhantomJS) that do not automatically store the current stack trace
information in a newly created `Error` object's `stack` property, but
only add it there once the `Error` gets thrown.

The original implementation works fine in Firefox & Chrome, but fails on IE10+
and PhantomJS where it, for example, breaks Karma's error reporting in cases
when an exception is thrown in a test like the following:

```
it('the holy crusade', inject(function() {
  var x = {};
  x.holyGrail();
}));
```

In this case, the ngMock `inject()` implementation would incorrectly add the
word `undefined` at the end of the collected error stack trace information,
thus causing the main error description to be reported back to Karma as
`undefined`.

The added test makes sure this functionality:

- works as expected in browsers supporting JavaScript stack trace
  collection, e.g. Chrome, Firefox, IE10+, Opera & PhantomJS
- does not add any bogus stack track information in browsers that do
  not support JavaScript stack trace collection, e.g. IE9

Fixes #13591
Closes #13592

Closes #13593
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants