No sourcemap-enabled stack trace in loadModules $injectorMinErr (at least not in Chrome)

[motin]

Do you want to request a feature or report a bug?

Bug

Current behavior + steps to reproduce

1. Use webpack or some other concatenation/minification of scripts that benefits of sourcemaps during debugging
2. (to be sure that minification errors are thrown) Enable strictDi: true in angular.bootstrap (or use ng-strict-di)
3. Attempt to include an angular module that has a reference or other problem. In this case, there is a problem with an undefined variable in config.js
4. Notice that the stack trace thrown by loadModules() is referencing to your bundled/minified javascript and not using sourcemaps (latest Chrome):

Error: [$injector:modulerr] Failed to instantiate module app due to:
ReferenceError: env is not defined
    at http://localhost:8080/bundle.app.cf5a8012b28c1225c1ed.js:87540:36
    at Object.invoke (http://localhost:8080/bundle.app.cf5a8012b28c1225c1ed.js:13342:20)
    at runInvokeQueue (http://localhost:8080/bundle.app.cf5a8012b28c1225c1ed.js:13225:36)
    at http://localhost:8080/bundle.app.cf5a8012b28c1225c1ed.js:13234:12
    at forEach (http://localhost:8080/bundle.app.cf5a8012b28c1225c1ed.js:8852:21)
    at loadModules (http://localhost:8080/bundle.app.cf5a8012b28c1225c1ed.js:13215:6)
    at createInjector (http://localhost:8080/bundle.app.cf5a8012b28c1225c1ed.js:13137:20)
    at doBootstrap (http://localhost:8080/bundle.app.cf5a8012b28c1225c1ed.js:10333:21)

5. Also notice that it is possible to click the arrow in the Chrome dev console and see a sourcemaps-enabled stack-trace, but without the entry of config.js where the actual ReferenceError is (it starts from the parent entry to the actual error)

What is the expected behavior?

On step 4, the stack trace should reference the actual source files, so that it is easy to understand where the problem lies in the source code, namely config.js:

ReferenceError: env is not defined
    at config.js:1268
    at Object.invoke (angular.js:4847)
    at runInvokeQueue (angular.js:4730)
    at angular.js:4739
    at forEach (angular.js:357)
    at loadModules (angular.js:4720)
    at createInjector (angular.js:4642)
    at doBootstrap (angular.js:1838)
...

What is the motivation / use case for changing the behavior?

Easier debugging of module inclusion, especially important when migrating to webpack or other bundler that requires sourcemaps to even have a change debugging in the development-environment.

Which versions of Angular, and which browser / OS are affected by this issue? Did this work in previous versions of Angular? Please also test with the latest stable and snapshot (https://code.angularjs.org/snapshot/) versions.

Angular 1.6.1 (latest stable)
Chrome 55
Mac OSX El Capitan

Have not tested with latest snapshot.

Other information (e.g. stacktraces, related issues, suggestions how to fix)

A workaround is to replace the following line in angular.js:
throw $injectorMinErr('modulerr', 'Failed to instantiate module {0} due to:\n{1}',
module, e.stack || e.message || e);
with:
throw e;

This yields an understandable stacktrace in the console (shown above).

Note, according to https://bugs.chromium.org/p/chromium/issues/detail?id=376409 "there is no way to fix it for Error.stack property because this property is generated by V8 which know nothing about source map". Despite this, e.stack is preferred as the ErrorConstructor argument over e.

This is evident when running
console.log(e, e.stack);
just before the error is throw. First an error with a sourcemap-enabled stacktrace is displayed in the console, then a stacktrace without sourcemaps.

However, simply replacing "e.stack || e.message || e" with "e" leads to a stack trace with sourcemaps enable but without config.js mentioned:

Uncaught Error: [$injector:modulerr] Failed to instantiate module app due to:
{}
http://errors.angularjs.org/1.6.1/$injector/modulerr?p0=app&p1=%7B%7D
at angular.js:68
at angular.js:4764
at forEach (angular.js:357)
at loadModules (angular.js:4720)
at createInjector (angular.js:4642)
at doBootstrap (angular.js:1838)
at bootstrap (angular.js:1859)

[gkalpak]

It would be nice to have better support for sourcemaps, but I am not sure how we can do this (without dropping minErr).

Maybe we could provide some hooks for people to customize their error reporting (i.e. the behavior of minErr). This could be handled together with #15433. E.g. something like:

angular.errorHandlingConfig({
  objectMaxDepth: 2,
  beforeError: function() {
    var lastArg = arguments[arguments.length - 1];
    if (lastArg instanceof Error) console.error(lastArg);
  }
});

[mgol]

@motin Thanks for the report. I've been thinking about this problem in a while. In my apps I workaround it via replacing the $exceptionHandler:

myModule
    .factory('$exceptionHandler', () => {
        'ngInject';

        return (...params) => {
            console.error(...params);
        };
    });

I've been thinking about removing this whole preprocessing inside of the $log service (this code has more than 5 years!) as the browser can only preprocess the stack trace if it gets an error logged; if we're just logging the stack as a string, the browser can't know we're printing an error stack and doesn't apply any transformations.

I've been looking at how various browsers deal with stack traces referring to files with source maps attached if and here is what I found:

1. Chrome & Safari map the stack traces to the original files (if you log the error).
2. Firefox always logs the original stack trace.
3. IE & Edge log the original stack trace but kind of buried inside of the object printed.

In IE/Edge you have to click once to reveal the object which will show you (among other things) the beginning of the stack. You have to then click on the stack to actually get it. Not the most pleasant experience. What's worse, in the exception handler we're actually often passing an error as the first argument and the cause (a string) as the second one. In such a case under the hood we get something like console.error(formatError(error), cause).

If we changed that to console.error(error, cause), i.e. if we remove the error formatting Edge & IE print error message and clause in the first line and everything else is wrapped in a dropdown so you have to click once more to get to the stack (3 clicks in total!). The screenshots at the bottom of this post show all the states.

Stopping formatting the errors would then decrease the developer experience in IE/Edge. Because of that, we've been thinking about user agent sniffing Edge (IE detection is simple; just check document.documentMode) and formatting the error then; in other browsers we'd disable the formatting.

---

Error logging in Edge (console.error(error, cause))

The first state:

After clicking on the arrow:

After clicking on the second arrow:

After clicking on the stack field:

[mgol]

@csuwildcat A while ago you reached to us to see if there are any issues in Edge that affect AngularJS in particular so that the Edge team can prioritize them. This is one of those issues (see my comment above, with screenshots, to get the problem). Are you still the person-to-go wrt issues affecting major frameworks/libraries? If not, who can we contact?

I opened an issue in the Edge bug tracker about the above problem: https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/10868717/

[motin]

@motin Thanks for the report. I've been thinking about this problem in a while. In my apps I workaround it via replacing the $exceptionHandler

Thank you! That workaround improves usability of stack traces HEAPS, simply by finally showing the actual line of the error.

Without the workaround:

With the workaround:

Night and day! :)

[csuwildcat]

@mgol just seeing this, apologies on the delay. I am no longer working in depth on Edge, but I can try to track down the person who handles this now if that would still help?

[mgol]

@csuwildcat I opened https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/10868717/ to track the issue, it's been closed as it apparently requires bigger architecture changes which have a uservoice entry at https://wpdev.uservoice.com/forums/257854-microsoft-edge-developer/suggestions/6225641-call-stacks-for-messages. We'd appreciate if this change was prioritized in some way but I'm not sure how feasible it is for you in the nearby future.

[mgol]

PR: #15767