Repository navigation
fix(@angular-devkit/schematics): include the template path in errors thrown by content templates - #34621
Open
Thanatos9404 wants to merge 2 commits into
Open
fix(@angular-devkit/schematics): include the template path in errors thrown by content templates#34621Thanatos9404 wants to merge 2 commits into
Thanatos9404 wants to merge 2 commits into
Conversation
…ontent templates Since 18.1.0 (41ee8ea), `applyContentTemplate` returns the entry unchanged for any `TypeError`, so that content which is not valid UTF-8 is left alone. Decoding and applying the template shared one `try`, so a `TypeError` thrown by the template itself, such as `<%= name.length %>` with `name` undefined, also left the file unrendered without an error. Decode in a separate `try` so that only decode failures are ignored.
…thrown by content templates Rethrow errors thrown while applying a content template, such as a `ReferenceError` from a misspelled variable or a `SyntaxError` inside `<% %>`, as `Failed to apply template "<path>": <error>` and keep the original error as `cause`. The message did not say which template failed, and the ng CLI printed only `foobat is not defined`. Closes angular#13192
There was a problem hiding this comment.
Code Review
This pull request refactors the applyContentTemplate function in Angular DevKit Schematics to separate content decoding from template execution. Template execution is now wrapped in its own try-catch block, which throws a more descriptive error containing the template path and preserves the original error as the cause. Additionally, comprehensive unit tests have been added to verify this error handling behavior, including checking that template names are included in errors and that the original error is preserved. I have no feedback to provide as the changes are well-implemented and covered by tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Checklist
Please check to confirm your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Errors thrown while
applyContentTemplaterenders a file don't say which file. With a misspelled variable,ng generateprints onlyfoobat is not defined. A syntax error inside<% %>printsUnexpected identifier 'unintended'. In both cases the stack points only into@angular-devkit/core.There is a related bug in the same
tryblock. Since 18.1.0 (41ee8ea), thecatchreturns the entry unchanged for anyTypeError, so that content that isn't valid UTF-8 is left alone. Because decoding and rendering share thattry, aTypeErrorthrown by the template itself is also ignored, for example<%= name.length %>withnameundefined. The schematic then succeeds and writes the unrendered template.Issue Number: #13192
What is the new behavior?
The PR has two commits.
TypeErrorfrom the template is now reported, as it was before 18.1.0.cause, likeHostTree'sFailed to decode "<path>"error. With a template atfiles/sub/__name__.ts.templateapplied throughurl('./files'), the message is:Does this PR introduce a breaking change?
Other information
Tests. The specs in
template_spec.tscover:TypeErrorthrown by a template;ReferenceErrorand aSyntaxError, checking the message and thecause;applyTemplatescase that checks the path in the message.I ran them with jasmine and tsx, outside Bazel. Each commit's new specs fail without its change and pass with it.
Other suites. All 141 specs under
packages/angular_devkit/schematicspass, except twoFileSystemEngineHostspecs that need a Bazel-built fixture; they fail the same way onmain.Built-in templates. Because template TypeErrors used to be ignored, I also ran the 930
@schematics/angularspecs onmainand on this branch. I generated the schema.tsfiles withtools/quicktype_runner.jsfirst. The same 18 specs fail on both in this Windows setup, so none of the built-in templates relied on the old behaviour.Checks.
tsc6.0.3 and prettier are clean for the changed files.Side effects. A
SchematicsExceptionthrown on purpose by a template helper is now wrapped the same way. Its message still appears after the path, and the exception itself is kept ascause.Not covered. The message doesn't include the line inside the template, because the template is compiled with
Function()and its line numbers don't match the template's.