Include component code location in runtime error messages #51919

pmvald · 2023-09-26T23:13:07Z

Currently the runtime error messages only include the name of the Type in the error. It is up to the dev to code search and find the class. This change provides infra tools by which we can include code location info (file path and line number) to error messages. This change focuses on components, but the same can be done for directives and pipes as well. This PR uses the new infra tools to revamp the "orphan component" error message (NG01001). Subsequent PRs can migrate more error messages.

Note that this feature is only supported in full and local compilation modes for now. Supporting it in partial mode needs more discussion.

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/main/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

Bugfix
Feature
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Documentation content changes
angular.io application / infrastructure changes
Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Some errors contain file path and line number of the components which are part of the error

Does this PR introduce a breaking change?

Yes
No

Other information

devversion

LGTM overall, but some comments

packages/core/src/render3/util/stringify_utils.ts

packages/core/test/render3/deps_tracker_spec.ts

packages/core/src/render3/debug/set_debug_info.ts

packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts

devversion · 2023-09-27T09:46:02Z

packages/compiler-cli/src/ngtsc/annotations/common/src/debug_info.ts

+  for (const rootDir of rootDirs) {
+    const rel = path.relative(rootDir, filePath);
+    if (!rel.startsWith('..')) {
+      return path.join('$PROJECT_ROOT', rel);


Can you extract this in to a constant?

Also: Do we actually need this? I think we can just always assume paths in DebugInfo#filePath to be relative to the project root.

Yeah removed this. For now we assume all paths are relative, unless we fail to resolve, in which case they will be absolute.

This is where g3 and 3P diverge. In g3 it makes perfect sense to replace $PROJECT_ROOT with either google3 or an addition /, and then the path will look so familiar. In 3P probably a relative path makes the most sense.

Maybe as an improvement in the future, if this debug info happened to be very popular we can add an field to options fir the path prefix, and in g3 we can set it to google3 or something and leave it undefined in 3P to just have relative paths.

packages/compiler/src/render3/r3_class_debug_info_compiler.ts

crisbeto · 2023-09-28T08:12:19Z

...li/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/debug_info.js

+
+…
+
+(() => { (typeof ngDevMode === "undefined" || ngDevMode) && i0.ɵsetClassDebugInfo(MainStandalone, { className: "MainStandalone", filePath: "debug_info.ts", lineNumber: 13 }); })();


I'm wondering if there isn't some security risk here since we're exposing details about the local file system in the JS bundle.

Interesting. good point to talk about. In production people would not see this- especially for Angular CLI where ngDevMode is properly set to false for production builds. I know there is a risk of not setting this up properly/or using the wrong build. Notably though, source maps would impose the same risk for dev-mode (or even prod)

Sourcemaps would indeed provide similar information. However, source maps are separate generated files that have explicit build options.

One area that should probably be tested here is that the file paths are always relative to the project root. Absolute paths should never be present here.

Source maps could also be inline here I'd assume. The generated code would also be stripped by the optimization build option. I agree though that the file paths should never be absolute

Modifed to either show the relative path or null, no absolute path

clydin · 2023-10-03T18:48:17Z

packages/compiler/src/render3/r3_class_debug_info_compiler.ts

+  });
+
+  const fnCall = o.importExpr(R3.setClassDebugInfo).callFn([
+    debugInfo.type,


The value reference here is going to cause code splitting and optimization issues at build time. The ngDevMode global is not effective in this regard since it is replaced after those decisions have been made.

Either an extra transformation will need to be added to the application build pipeline to manually remove each of these lines for each build or an additional compiler-cli option will be needed to control the class debug info emit so they will not be present during bundling.

We can use some field in NgCompilerOptions - I would prefer to use a flag indicating that we are compiling in dev mode. Do we have such a flag currently in NgCompilerOptions ? Or we want to have a flag specifically for debug info?

Also which interface you recommend to place the new flag, InternalOptions ?

@clydin Unless I miss something, I'm seeing ESBuild performing the code splitting & preservation of side-effects with respect to global defines?

https://esbuild.github.io/try/#YgAwLjE5LjMALS1idW5kbGUgLS10cmVlLXNoYWtpbmc9dHJ1ZSAtLW1pbmlmeS1zeW50YXggLS1taW5pZnktaWRlbnRpZmllcnM9ZmFsc2UgLS1kZWZpbmU6bmdEZXZNb2RlPWZhbHNlAGUAZW50cnkuanMAaW1wb3J0IHt9IGZyb20gJy4vdGVzdCc7AABleHBvcnQuanMAAAB0ZXN0LmpzAGV4cG9ydCBjbGFzcyBBIHt9CgppZiAobmdEZXZNb2RlKSBjb25zb2xlLmxvZyhBLCBpbXBvcnQoJy4vb2snKSk7CgoKAABvay5qcwBleHBvcnQgZGVmYXVsdCB3b3JrcyA9IHRydWU7Cgpjb25zb2xlLmxvZygnU2lkZSBlZmZlY3QnKTs

We had some issues with setClassMetadata and the defer block which caused eager imports of code that should be deferred. However, if this new construct is always in the same module, i think it should be alright. We should test internally though to ensure expected behavior with the build pipeline.

However, from a more general perspective, generating code that is then immediately removed, i feel, seems counterproductive.

I agree with all said- for the generating code that is immediately removed. I think we are not doing anything new here. i.e. JIT class metadata TestBed is fully based around a similar principle

So for now lets follow the existing pattern. But I'm open to the idea that we define dev mode (and other modes) in the cli compiler instead of runtime flags such as ngDevMode, and the compiler basically does not generate the unnecessary code in the first place.

One of the reasons we always emit the ngDevMode output is fully compiled code may be used in other places as well. e.g. a dev-mode application might import the full compilation output from a separate target. I think keeping these as runtime checks simplifies this and, ultimately, optimizers/bundlers have the ability to strip off the dev-mode checks globally, or in dev-mode all compilation output artifacts integrate nicely. But yes, we can revisit in the future.

clydin · 2023-10-04T16:26:36Z

packages/compiler/src/render3/r3_class_debug_info_compiler.ts

+  const debugInfoObject = mapLiteral({
+    className: debugInfo.className,
+    filePath: debugInfo.filePath,
+    lineNumber: debugInfo.lineNumber,


A note that the line number may not always be accurate may be useful here.
The file may have been modified within the build process prior to being given to the TS/NG compiler.

devversion

LGTM, capturing some things we talked about:

in the future, we may need/want the same for partial compilation output. i.e. mostly libraries from npm
we are running with the idea that there are no security concerns with capturing the file path, as this is guarded by ngDevMode and paths are also relative. Somewhat related to source-maps which could also expose file paths and even source code.

packages/core/src/render3/interfaces/definition.ts

A new utility function `compileClassDebugInfo` is introduced which creates compile result necessary to generate statement for attaching some useful debug info into angular classes. An example of teh new statement would be: ``` (() => { (typeof ngDevMode === "undefined" || ngDevMode) && i0.ɵsetClassDebugInfo(Main, { className: "Main", filePath: "$PROJECT_ROOT/src/main.ts", lineNumber: 8 }); })(); ``` Currently, the debug info contains: - the class name - the file path in which it is defined - the line number in which it is defined The debug info will be used in runtime to generate more helpful error messages.

A new statement will be generated for components which will attach some useful debug info to them to be used in runtime error handling. Currently this only happens in full and local compilation modes.

…DebugInfo This runtime will set the runtime debug info for the given angular class

A new field `debugInfo` is added to the component definition. Now the runtime ɵsetClassDebugInfo stores the debug info for components in this new field.

…es debug info such as the file path and line number The current error stringifier only includes the class name. In this change a new stringifier is added which returns a more helpful string which includes the file path and line number. Note that this is only the case with components, and for other class types (directive, pipes) it will fallback to the current stringifier. Subsequent changes can cover the case of directive and pipes as well.

… include full component info The error message now contains the code location of the component. It now looks like: "Error: NG01001: Orphan component found! Trying to render the component Main (at $PROJECT_ROOT/src/main.ts:8) without first loading the NgModule ..."

atscott · 2023-10-09T22:57:06Z

This PR was merged into the repository by commit 68ba798.

A new statement will be generated for components which will attach some useful debug info to them to be used in runtime error handling. Currently this only happens in full and local compilation modes. PR Close #51919

…DebugInfo (#51919) This runtime will set the runtime debug info for the given angular class PR Close #51919

…ts (#51919) A new field `debugInfo` is added to the component definition. Now the runtime ɵsetClassDebugInfo stores the debug info for components in this new field. PR Close #51919

…es debug info such as the file path and line number (#51919) The current error stringifier only includes the class name. In this change a new stringifier is added which returns a more helpful string which includes the file path and line number. Note that this is only the case with components, and for other class types (directive, pipes) it will fallback to the current stringifier. Subsequent changes can cover the case of directive and pipes as well. PR Close #51919

… include full component info (#51919) The error message now contains the code location of the component. It now looks like: "Error: NG01001: Orphan component found! Trying to render the component Main (at $PROJECT_ROOT/src/main.ts:8) without first loading the NgModule ..." PR Close #51919

angular-automatic-lock-bot · 2023-11-09T00:09:01Z

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.}

…1919) A new utility function `compileClassDebugInfo` is introduced which creates compile result necessary to generate statement for attaching some useful debug info into angular classes. An example of teh new statement would be: ``` (() => { (typeof ngDevMode === "undefined" || ngDevMode) && i0.ɵsetClassDebugInfo(Main, { className: "Main", filePath: "$PROJECT_ROOT/src/main.ts", lineNumber: 8 }); })(); ``` Currently, the debug info contains: - the class name - the file path in which it is defined - the line number in which it is defined The debug info will be used in runtime to generate more helpful error messages. PR Close angular#51919

A new statement will be generated for components which will attach some useful debug info to them to be used in runtime error handling. Currently this only happens in full and local compilation modes. PR Close angular#51919

…DebugInfo (angular#51919) This runtime will set the runtime debug info for the given angular class PR Close angular#51919

…ts (angular#51919) A new field `debugInfo` is added to the component definition. Now the runtime ɵsetClassDebugInfo stores the debug info for components in this new field. PR Close angular#51919

…es debug info such as the file path and line number (angular#51919) The current error stringifier only includes the class name. In this change a new stringifier is added which returns a more helpful string which includes the file path and line number. Note that this is only the case with components, and for other class types (directive, pipes) it will fallback to the current stringifier. Subsequent changes can cover the case of directive and pipes as well. PR Close angular#51919

… include full component info (angular#51919) The error message now contains the code location of the component. It now looks like: "Error: NG01001: Orphan component found! Trying to render the component Main (at $PROJECT_ROOT/src/main.ts:8) without first loading the NgModule ..." PR Close angular#51919

angular-robot bot added the detected: feature PR contains a feature commit label Sep 26, 2023

pmvald force-pushed the runtime-debug-info branch 2 times, most recently from ed89607 to eb48386 Compare September 27, 2023 03:01

ngbot bot added this to the Backlog milestone Sep 27, 2023

pmvald changed the title ~~Runtime debug info~~ Include component code location in runtime error messages Sep 27, 2023

pmvald requested review from devversion, AndrewKushnir and crisbeto September 27, 2023 04:58

pmvald marked this pull request as ready for review September 27, 2023 04:58

devversion approved these changes Sep 27, 2023

View reviewed changes

pmvald force-pushed the runtime-debug-info branch 3 times, most recently from 125d1ff to ef003eb Compare September 27, 2023 22:56

crisbeto reviewed Sep 28, 2023

View reviewed changes

AndrewKushnir requested review from alxhub and removed request for AndrewKushnir October 2, 2023 21:18

pmvald removed the request for review from alxhub October 3, 2023 02:26

clydin reviewed Oct 3, 2023

View reviewed changes

clydin reviewed Oct 4, 2023

View reviewed changes

pmvald force-pushed the runtime-debug-info branch from ef003eb to ef2bc75 Compare October 5, 2023 02:40

pmvald requested a review from devversion October 5, 2023 02:41

devversion approved these changes Oct 5, 2023

View reviewed changes

packages/core/src/render3/interfaces/definition.ts Show resolved Hide resolved

pmvald added 3 commits October 5, 2023 14:18

refactor(compiler-cli): add debug info to components

8519a64

A new statement will be generated for components which will attach some useful debug info to them to be used in runtime error handling. Currently this only happens in full and local compilation modes.

refactor(core): add an empty implementation for the runtime ɵsetClass…

290ded4

…DebugInfo This runtime will set the runtime debug info for the given angular class

pmvald added 3 commits October 5, 2023 14:18

refactor(core): implement the runtime ɵsetClassDebugInfo for components

aab44aa

A new field `debugInfo` is added to the component definition. Now the runtime ɵsetClassDebugInfo stores the debug info for components in this new field.

pmvald force-pushed the runtime-debug-info branch from ef2bc75 to e4c099a Compare October 5, 2023 18:19

pmvald requested a review from crisbeto October 5, 2023 18:21

pmvald mentioned this pull request Oct 6, 2023

Forbid orphan components #52061

Closed

14 tasks

crisbeto approved these changes Oct 6, 2023

View reviewed changes

pmvald added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Oct 6, 2023

atscott closed this in 421d790 Oct 9, 2023

atscott pushed a commit that referenced this pull request Oct 9, 2023

refactor(core): add an empty implementation for the runtime ɵsetClass…

7f9b32c

…DebugInfo (#51919) This runtime will set the runtime debug info for the given angular class PR Close #51919

angular-automatic-lock-bot bot locked and limited conversation to collaborators Nov 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Include component code location in runtime error messages #51919

Include component code location in runtime error messages #51919

pmvald commented Sep 26, 2023 •

edited

devversion left a comment

devversion Sep 27, 2023

pmvald Sep 27, 2023

crisbeto Sep 28, 2023

devversion Sep 28, 2023

clydin Oct 3, 2023

devversion Oct 4, 2023

pmvald Oct 5, 2023

clydin Oct 3, 2023

pmvald Oct 3, 2023

devversion Oct 4, 2023 •

edited

clydin Oct 4, 2023

devversion Oct 4, 2023

pmvald Oct 5, 2023

devversion Oct 5, 2023

clydin Oct 4, 2023

devversion left a comment

atscott commented Oct 9, 2023

angular-automatic-lock-bot bot commented Nov 9, 2023


		…

		(() => { (typeof ngDevMode === "undefined" \|\| ngDevMode) && i0.ɵsetClassDebugInfo(MainStandalone, { className: "MainStandalone", filePath: "debug_info.ts", lineNumber: 13 }); })();

Include component code location in runtime error messages #51919

Include component code location in runtime error messages #51919

Conversation

pmvald commented Sep 26, 2023 • edited

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

devversion left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devversion Oct 4, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devversion left a comment

Choose a reason for hiding this comment

atscott commented Oct 9, 2023

angular-automatic-lock-bot bot commented Nov 9, 2023

pmvald commented Sep 26, 2023 •

edited

devversion Oct 4, 2023 •

edited