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

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

PR Close #60056
This commit is contained in:
Andrew Scott
2025-02-21 11:35:31 -08:00
committed by kirjs
parent af2e79eb41
commit 491b0a4ead
16 changed files with 11 additions and 51 deletions
@@ -669,13 +669,8 @@ export class ApplicationRef {
this.dirtyFlags |= ApplicationRefDirtyFlags.AfterRender;
// Check all potentially dirty views.
for (let {_lView, notifyErrorHandler} of this.allViews) {
detectChangesInViewIfRequired(
_lView,
notifyErrorHandler,
useGlobalCheck,
this.zonelessEnabled,
);
for (let {_lView} of this.allViews) {
detectChangesInViewIfRequired(_lView, useGlobalCheck, this.zonelessEnabled);
}
// If `markForCheck()` was called during view checking, it will have set the `ViewTreeCheck`
@@ -893,7 +888,6 @@ export const enum ApplicationRefDirtyFlags {
export function detectChangesInViewIfRequired(
lView: LView,
notifyErrorHandler: boolean,
isFirstPass: boolean,
zonelessEnabled: boolean,
) {
@@ -910,5 +904,5 @@ export function detectChangesInViewIfRequired(
ChangeDetectionMode.Global
: // Only refresh views with the `RefreshView` flag or views is a changed signal
ChangeDetectionMode.Targeted;
detectChangesInternal(lView, notifyErrorHandler, mode);
detectChangesInternal(lView, mode);
}
@@ -117,7 +117,7 @@ export class DebugNgZoneForCheckNoChanges extends NgZone {
this.applicationRef ||= this.injector.get(ApplicationRef);
for (const view of this.applicationRef.allViews) {
try {
checkNoChangesInternal(view._lView, this.checkNoChangesMode, view.notifyErrorHandler);
checkNoChangesInternal(view._lView, this.checkNoChangesMode);
} catch (e) {
this.errorHandler ||= this.injector.get(ErrorHandler);
this.errorHandler.handleError(e);
@@ -153,7 +153,7 @@ function exhaustiveCheckNoChangesInterval(
for (const view of applicationRef.allViews) {
try {
checkNoChangesInternal(view._lView, checkNoChangesMode, view.notifyErrorHandler);
checkNoChangesInternal(view._lView, checkNoChangesMode);
} catch (e) {
errorHandler.handleError(e);
}
@@ -374,7 +374,6 @@ export class ComponentRef<T> extends AbstractComponentRef<T> {
this.hostView = this.changeDetectorRef = new ViewRef<T>(
_rootLView,
undefined /* _cdRefInjectingView */,
false /* notifyErrorHandler */,
);
this.componentType = componentType;
}
@@ -69,18 +69,14 @@ import {profiler} from '../profiler';
import {ProfilerEvent} from '../profiler_types';
import {executeViewQueryFn, refreshContentQueries} from '../queries/query_execution';
import {runEffectsInView} from '../reactivity/view_effect_runner';
import {executeTemplate, handleError} from './shared';
import {executeTemplate} from './shared';
/**
* The maximum number of times the change detection traversal will rerun before throwing an error.
*/
export const MAXIMUM_REFRESH_RERUNS = 100;
export function detectChangesInternal(
lView: LView,
notifyErrorHandler = true,
mode = ChangeDetectionMode.Global,
) {
export function detectChangesInternal(lView: LView, mode = ChangeDetectionMode.Global) {
const environment = lView[ENVIRONMENT];
const rendererFactory = environment.rendererFactory;
@@ -95,11 +91,6 @@ export function detectChangesInternal(
try {
detectChangesInViewWhileDirty(lView, mode);
} catch (error) {
if (notifyErrorHandler) {
handleError(lView, error);
}
throw error;
} finally {
if (!checkNoChangesMode) {
rendererFactory.end?.();
@@ -146,14 +137,10 @@ function detectChangesInViewWhileDirty(lView: LView, mode: ChangeDetectionMode)
}
}
export function checkNoChangesInternal(
lView: LView,
mode: CheckNoChangesMode,
notifyErrorHandler = true,
) {
export function checkNoChangesInternal(lView: LView, mode: CheckNoChangesMode) {
setIsInCheckNoChangesMode(mode);
try {
detectChangesInternal(lView, notifyErrorHandler);
detectChangesInternal(lView);
} finally {
setIsInCheckNoChangesMode(CheckNoChangesMode.Off);
}
+2 -7
View File
@@ -74,7 +74,6 @@ export class ViewRef<T> implements EmbeddedViewRef<T>, ChangeDetectorRefInterfac
* This may be different from `_lView` if the `_cdRefInjectingView` is an embedded view.
*/
private _cdRefInjectingView?: LView,
readonly notifyErrorHandler = true,
) {}
get context(): T {
@@ -327,7 +326,7 @@ export class ViewRef<T> implements EmbeddedViewRef<T>, ChangeDetectorRefInterfac
// until the end of the refresh. Using `RefreshView` prevents creating a potential difference
// in the state of the LViewFlags during template execution.
this._lView[FLAGS] |= LViewFlags.RefreshView;
detectChangesInternal(this._lView, this.notifyErrorHandler);
detectChangesInternal(this._lView);
}
/**
@@ -338,11 +337,7 @@ export class ViewRef<T> implements EmbeddedViewRef<T>, ChangeDetectorRefInterfac
*/
checkNoChanges(): void {
if (ngDevMode) {
checkNoChangesInternal(
this._lView,
CheckNoChangesMode.OnlyDirtyViews,
this.notifyErrorHandler,
);
checkNoChangesInternal(this._lView, CheckNoChangesMode.OnlyDirtyViews);
}
}
@@ -270,7 +270,6 @@
"detectChangesInEmbeddedViews",
"detectChangesInView",
"detectChangesInViewIfAttached",
"detectChangesInViewIfRequired",
"detectChangesInternal",
"diPublicInInjector",
"documentElement",
@@ -291,7 +291,6 @@
"detectChangesInEmbeddedViews",
"detectChangesInView",
"detectChangesInViewIfAttached",
"detectChangesInViewIfRequired",
"detectChangesInternal",
"diPublicInInjector",
"documentElement",
@@ -223,7 +223,6 @@
"detectChangesInEmbeddedViews",
"detectChangesInView",
"detectChangesInViewIfAttached",
"detectChangesInViewIfRequired",
"detectChangesInternal",
"diPublicInInjector",
"elementEndFirstCreatePass",
@@ -271,7 +271,6 @@
"detectChangesInEmbeddedViews",
"detectChangesInView",
"detectChangesInViewIfAttached",
"detectChangesInViewIfRequired",
"detectChangesInternal",
"diPublicInInjector",
"elementEndFirstCreatePass",
@@ -324,7 +324,6 @@
"detectChangesInEmbeddedViews",
"detectChangesInView",
"detectChangesInViewIfAttached",
"detectChangesInViewIfRequired",
"detectChangesInternal",
"diPublicInInjector",
"elementEndFirstCreatePass",
@@ -408,7 +407,6 @@
"getTStylingRangePrev",
"getTView",
"getViewRefs",
"handleError",
"handleStoppedNotification",
"handleUnhandledError",
"hasApplyArgsData",
@@ -312,7 +312,6 @@
"detectChangesInEmbeddedViews",
"detectChangesInView",
"detectChangesInViewIfAttached",
"detectChangesInViewIfRequired",
"detectChangesInternal",
"diPublicInInjector",
"elementEndFirstCreatePass",
@@ -395,7 +394,6 @@
"getTStylingRangePrev",
"getTView",
"getViewRefs",
"handleError",
"handleStoppedNotification",
"handleUnhandledError",
"hasApplyArgsData",
@@ -176,7 +176,6 @@
"detectChangesInEmbeddedViews",
"detectChangesInView",
"detectChangesInViewIfAttached",
"detectChangesInViewIfRequired",
"detectChangesInternal",
"diPublicInInjector",
"enterDI",
@@ -233,7 +233,6 @@
"detectChangesInEmbeddedViews",
"detectChangesInView",
"detectChangesInViewIfAttached",
"detectChangesInViewIfRequired",
"detectChangesInternal",
"diPublicInInjector",
"enterDI",
@@ -380,7 +380,6 @@
"detectChangesInEmbeddedViews",
"detectChangesInView",
"detectChangesInViewIfAttached",
"detectChangesInViewIfRequired",
"detectChangesInternal",
"diPublicInInjector",
"elementEndFirstCreatePass",
@@ -483,7 +482,6 @@
"getTView",
"getTokenOrFunctionIdentity",
"getViewRefs",
"handleError",
"handleStoppedNotification",
"handleUnhandledError",
"hasApplyArgsData",
@@ -202,7 +202,6 @@
"detectChangesInEmbeddedViews",
"detectChangesInView",
"detectChangesInViewIfAttached",
"detectChangesInViewIfRequired",
"detectChangesInternal",
"diPublicInInjector",
"enterDI",
@@ -264,7 +264,6 @@
"detectChangesInEmbeddedViews",
"detectChangesInView",
"detectChangesInViewIfAttached",
"detectChangesInViewIfRequired",
"detectChangesInternal",
"diPublicInInjector",
"elementEndFirstCreatePass",
@@ -337,7 +336,6 @@
"getTStylingRangePrev",
"getTView",
"getViewRefs",
"handleError",
"handleStoppedNotification",
"handleUnhandledError",
"hasApplyArgsData",