Skip to content

fix(core): Remove duplicate reporting of errors in CDR.detectChanges#60056

Closed
atscott wants to merge 1 commit intoangular:mainfrom
atscott:detectChangesNoReport
Closed

fix(core): Remove duplicate reporting of errors in CDR.detectChanges#60056
atscott wants to merge 1 commit intoangular:mainfrom
atscott:detectChangesNoReport

Conversation

@atscott
Copy link
Copy Markdown
Contributor

@atscott atscott commented Feb 21, 2025

This change removes the reporting of errors from the ChangeDetectorRef.detectChanges API. The reporting results in the error being "handled" in two ways, both by reporting to error handler and rethrowing the error. This rethrown error generally ends up being caught further up and again reported to the error handler. The error handler is meant to be for uncaught errors, and since Angular is not at the top of the stack of the call of CDR.detectChanges, it does not know what is being done with the rethrown error.

Note that for zone-based applications, this will likely have no effect other than removing duplicate reporting of the error. If the rethrown error is not already being caught, it will reach the NgZone's error trap and still be reported to the application ErrorHandler.

@atscott atscott added breaking changes target: major This PR is targeted for the next major release requires: TGP This PR requires a passing TGP before merging is allowed labels Feb 21, 2025
@pullapprove pullapprove bot removed the requires: TGP This PR requires a passing TGP before merging is allowed label Feb 21, 2025
@angular-robot angular-robot bot added detected: breaking change PR contains a commit with a breaking change area: core Issues related to the framework runtime labels Feb 21, 2025
@ngbot ngbot bot added this to the Backlog milestone Feb 21, 2025
@atscott
Copy link
Copy Markdown
Contributor Author

atscott commented Feb 24, 2025

Green TGP

@atscott atscott requested a review from alxhub February 24, 2025 19:04
@atscott atscott force-pushed the detectChangesNoReport branch from ce1288d to d018509 Compare February 24, 2025 19:48
@angular-robot angular-robot bot added the area: language-service Issues related to Angular's VS Code language service label Feb 24, 2025
@atscott atscott force-pushed the detectChangesNoReport branch from 64fc716 to d018509 Compare February 24, 2025 22:01
This change removes the reporting of errors from the
`ChangeDetectorRef.detectChanges` API. The reporting results in the
error being "handled" in two ways, both by reporting to error handler
and rethrowing the error. This rethrown error generally ends up being
caught further up and again reported to the error handler. The error
handler is meant to be for uncaught errors, and since Angular is not at
the top of the stack of the call of `CDR.detectChanges`, it does not
know what is being done with the rethrown error.

Note that for zone-based applications, this will likely have no effect
other than removing duplicate reporting of the error. If the rethrown
error is not already being caught, it will reach the NgZone's error
trap and still be reported to the application `ErrorHandler`.
@atscott atscott force-pushed the detectChangesNoReport branch from d018509 to 6036251 Compare February 24, 2025 22:08
@angular-robot angular-robot bot removed the detected: breaking change PR contains a commit with a breaking change label Feb 24, 2025
@atscott atscott added the action: merge The PR is ready for merge by the caretaker label Feb 24, 2025
@kirjs
Copy link
Copy Markdown
Contributor

kirjs commented Feb 25, 2025

This PR was merged into the repository by commit 491b0a4.

The changes were merged into the following branches: main

@angular-automatic-lock-bot
Copy link
Copy Markdown

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Mar 28, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime area: language-service Issues related to Angular's VS Code language service target: major This PR is targeted for the next major release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants