Tracking: Better Error Messages

[mhevery]

Issues

Other Links

• https://daveceddia.com/angular-2-errors/
• http://www.bennadel.com/blog/3040-i-have-a-fundamental-misunderstanding-of-change-detection-in-angular-2-beta-8.htm

[btford]

Some general questions:

• How do we want to address type errors at runtime for non-TS users? For instance, several of the issues raised in these blog posts are caused by passing a string when an array was expected, or vice-versa. Should we throw an exception? should we be forgiving and coerce to an array? Is there a way to mark these extra checks as not needed in production mode and strip them out?

[mhevery]

Good question. @IgorMinar may have an opinion.

In AngularJS we had a utility function which could verify a type of an argument and then provide a useful error message. I think something like that may be needed here as well.

function someAPI(name, age) {
  assertString('name', name');
  assertNumber('age', age);
  // do the actual work. 
}

In prod build we could then just make these noops and inline them.

What do you think?

[btford]

Seems reasonable

[dceddia]

I like the propTypes warnings that React issues:

React.PropTypes exports a range of validators that can be used to make sure the data you receive is valid. When an invalid value is provided for a prop, a warning will be shown in the JavaScript console. Note that for performance reasons propTypes is only checked in development mode.

I don't know which of "print a warning" or "throw an error" makes the most sense.

Some example errors:

Warning: Failed propType: Required prop message was not specified in CommentBox.

Warning: Failed propType: Invalid prop message of type number supplied to CommentBox, expected string.

[btford]

WRT: http://www.bennadel.com/blog/3040-i-have-a-fundamental-misunderstanding-of-change-detection-in-angular-2-beta-8.htm

I think the best course of action would be to add some dev mode checks in the template compiler.

Using component metadata and the template AST, we could check for cases where a query is used in a template:

@Component({
  selector: "my-app",
  directives: [ Item ],
  queries: {
    itemList: new ViewChildren( Item )
  },
  template: `
    <h2>Item Count: {{ itemList?.length }}</h2>
    <item></item>
    <item></item>`
})
class WhateverApp {}

Here we can give a slightly better error: Using query 'itemList' in component 'WhateverApp' is likely to cause cycles in change detection. Consider doing Foo instead.

I need some time with @tbosch to be sure that we can do this with his latest compiler changes.

I suspect we can expand these sort of errors to other common cases to provide better feedback than the generic change detection error.

[mhevery]

So it sounds like this one will have to wait until @tbosch lands his change? Sounds reasonable.

[IgorMinar]

wrt assertString it sounds like we are reinventing AtScript all over again
:-)

if only Typescript had runtime (type) assertions...

On Thu, Mar 10, 2016 at 8:14 PM Miško Hevery notifications@github.com
wrote:

So it sounds like this one will have to wait until @tbosch
https://github.com/tbosch lands his change? Sounds reasonable.

—
Reply to this email directly or view it on GitHub
#7481 (comment).

[btford]

I'd like that too. But for now I think we get good bang for our buck adding the assertions ourselves for high traffic areas where we currently throw errors like someVar.map is not a function on a bad invocation.

[IgorMinar]

I agree.

On Fri, Mar 11, 2016 at 12:33 PM Brian Ford notifications@github.com
wrote:

I'd like that too. But for now I think we get good bang for our buck
adding the assertions ourselves for high traffic areas where we currently
throw errors like someVar.map is not a function for bad invocations.

—
Reply to this email directly or view it on GitHub
#7481 (comment).

[robwormald]

Error Message: VM409 angular2.dev.js:23501EXCEPTION: TypeError: Cannot set property 'lastInBinding' of undefined

Cause: I forgot to add a method to be called from a click event: (click)=""

[ericmartinezr]

May I add to make this error better EXCEPTION: Token must be defined! ? The reason : no idea...

See this SO question.

[zoechi]

Invalid argument(s) when a test component doesn't have a selector (Dart)

[zoechi]

@robwormald there is an issue for that error #3754 mentioned in #7481 (comment)

[zoechi]

TypeError: Cannot set property 'endSourceSpan' of null

for missing closing tag </div>

http://stackoverflow.com/questions/36492169/i-have-an-error-in-angular-2-when-im-trying-to-use-nested-components/36492238#36492238

[zoechi]

EXCEPTION: Error during instantiation of Router! (RouterLink -> Router).

when 2 routes have the same path

[zoechi]

multiple structural directives on the same element should produce an error message #8438

[mhevery]

While there are more work to be done on better error messages, we will be closing this issue in favor of other issues.

[chrisnicola]

@mhevery I noticed this wasn't closed but it seems the interest in improving A2's error messages has waned. I feel like that would be a mistake. Right now error messages are pretty much my biggest complaint about this framework.

First, even something as simple as an error thrown from the constructor of a Component logs 8 different messages to the console. Most of them redundant or unimportant information. If this is supposed to be used for debugging the framework, then they should at a minimum be disabled by default.

Second, there is no way to see original source locations with Angular 2 error messages at the moment. See #11722 for more details and my attempts to debug the issue. While this is possibly an issue for the Chrome devtools team, since this primarily affects the Angular 2 development experience I feel it is relevant here.

Finally, error message handling is inconsistent depending on circumstances and where errors happen. For example, you will get different results handled by different mechanisms if the error is thrown during the construction of the module during bootstrapping vs construction of a component. Another result if an error is thrown by a downgraded angular 2 component. Different handler levels all fire and in some cases rethrow to other handler levels which appears to be the cause of all the redundant logging as well.

I'd like to know what, if anything, the Angular team has planned around improving errors and the debugging experience overall, because at the moment it seems to be very hard to work with. Perhaps I'm the only one experiencing these problems, or perhaps I'm just doing it wrong, in which case I'd like to hear what is the right way forward.

[pkozlowski-opensource]

I'd like to know what, if anything, the Angular team has planned around improving errors and the debugging experience overall, because at the moment it seems to be very hard to work with. Perhaps I'm the only one experiencing these problems, or perhaps I'm just doing it wrong, in which case I'd like to hear what is the right way forward.

@chrisnicola error messages are still very important. The right / best way forward is to open small / focused issues with a minimal reproduce scenario each time you encounter a confusing error message. Unfortunately there are way to many issues that are either lacking reproduce scenario or are taken from large apps where many different factors might be at play.

In short: we want and would fix cases we can see. Once again, let's have a small, separate issue for each case.

[chrisnicola]

@pkozlowski-opensource of course, completely makes sense. So should this issue be closed then?

I've opened #11722 to address the specific issue of original source locations not working with stack traces, which seems to have been prematurely closed.

I'm also happy to open another to address the issue of multiple redundant error messages and/or more flexibility for developers to customize/control error reporting behaviour.

[pkozlowski-opensource]

I've opened #11722 to address the specific issue of original source locations not working with stack traces, which seems to have been prematurely closed.

Reopened.

I'm also happy to open another to address the issue of multiple redundant error messages and/or more flexibility for developers to customize/control error reporting behaviour.

Sure, but please, search for duplicates first.

[b264]

Opened #15402 for Error trying to diff message

[jelbourn]

Closing as obsolete. Recent versions of Angular has more descriptive error messages and codes

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}