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

atscott · kirjs · commit 491b0a4ead98 · 2025-02-25T12:20:22.000-05:00
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
diff --git a/packages/core/src/application/application_ref.ts b/packages/core/src/application/application_ref.ts
@@ -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);
 }
diff --git a/packages/core/src/change_detection/scheduling/exhaustive_check_no_changes.ts b/packages/core/src/change_detection/scheduling/exhaustive_check_no_changes.ts
@@ -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);
                 }
diff --git a/packages/core/src/render3/component_ref.ts b/packages/core/src/render3/component_ref.ts
@@ -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;
   }
diff --git a/packages/core/src/render3/instructions/change_detection.ts b/packages/core/src/render3/instructions/change_detection.ts
@@ -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);
   }
diff --git a/packages/core/src/render3/view_ref.ts b/packages/core/src/render3/view_ref.ts
@@ -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);
     }
   }
 
diff --git a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json
@@ -270,7 +270,6 @@
   "detectChangesInEmbeddedViews",
   "detectChangesInView",
   "detectChangesInViewIfAttached",
-  "detectChangesInViewIfRequired",
   "detectChangesInternal",
   "diPublicInInjector",
   "documentElement",
diff --git a/packages/core/test/bundling/animations/bundle.golden_symbols.json b/packages/core/test/bundling/animations/bundle.golden_symbols.json
@@ -291,7 +291,6 @@
   "detectChangesInEmbeddedViews",
   "detectChangesInView",
   "detectChangesInViewIfAttached",
-  "detectChangesInViewIfRequired",
   "detectChangesInternal",
   "diPublicInInjector",
   "documentElement",
diff --git a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json
@@ -223,7 +223,6 @@
   "detectChangesInEmbeddedViews",
   "detectChangesInView",
   "detectChangesInViewIfAttached",
-  "detectChangesInViewIfRequired",
   "detectChangesInternal",
   "diPublicInInjector",
   "elementEndFirstCreatePass",
diff --git a/packages/core/test/bundling/defer/bundle.golden_symbols.json b/packages/core/test/bundling/defer/bundle.golden_symbols.json
@@ -271,7 +271,6 @@
   "detectChangesInEmbeddedViews",
   "detectChangesInView",
   "detectChangesInViewIfAttached",
-  "detectChangesInViewIfRequired",
   "detectChangesInternal",
   "diPublicInInjector",
   "elementEndFirstCreatePass",
diff --git a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json
@@ -324,7 +324,6 @@
   "detectChangesInEmbeddedViews",
   "detectChangesInView",
   "detectChangesInViewIfAttached",
-  "detectChangesInViewIfRequired",
   "detectChangesInternal",
   "diPublicInInjector",
   "elementEndFirstCreatePass",
@@ -408,7 +407,6 @@
   "getTStylingRangePrev",
   "getTView",
   "getViewRefs",
-  "handleError",
   "handleStoppedNotification",
   "handleUnhandledError",
   "hasApplyArgsData",
diff --git a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json
@@ -312,7 +312,6 @@
   "detectChangesInEmbeddedViews",
   "detectChangesInView",
   "detectChangesInViewIfAttached",
-  "detectChangesInViewIfRequired",
   "detectChangesInternal",
   "diPublicInInjector",
   "elementEndFirstCreatePass",
@@ -395,7 +394,6 @@
   "getTStylingRangePrev",
   "getTView",
   "getViewRefs",
-  "handleError",
   "handleStoppedNotification",
   "handleUnhandledError",
   "hasApplyArgsData",
diff --git a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json
@@ -176,7 +176,6 @@
   "detectChangesInEmbeddedViews",
   "detectChangesInView",
   "detectChangesInViewIfAttached",
-  "detectChangesInViewIfRequired",
   "detectChangesInternal",
   "diPublicInInjector",
   "enterDI",
diff --git a/packages/core/test/bundling/hydration/bundle.golden_symbols.json b/packages/core/test/bundling/hydration/bundle.golden_symbols.json
@@ -233,7 +233,6 @@
   "detectChangesInEmbeddedViews",
   "detectChangesInView",
   "detectChangesInViewIfAttached",
-  "detectChangesInViewIfRequired",
   "detectChangesInternal",
   "diPublicInInjector",
   "enterDI",
diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json
@@ -380,7 +380,6 @@
   "detectChangesInEmbeddedViews",
   "detectChangesInView",
   "detectChangesInViewIfAttached",
-  "detectChangesInViewIfRequired",
   "detectChangesInternal",
   "diPublicInInjector",
   "elementEndFirstCreatePass",
@@ -483,7 +482,6 @@
   "getTView",
   "getTokenOrFunctionIdentity",
   "getViewRefs",
-  "handleError",
   "handleStoppedNotification",
   "handleUnhandledError",
   "hasApplyArgsData",
diff --git a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json
@@ -202,7 +202,6 @@
   "detectChangesInEmbeddedViews",
   "detectChangesInView",
   "detectChangesInViewIfAttached",
-  "detectChangesInViewIfRequired",
   "detectChangesInternal",
   "diPublicInInjector",
   "enterDI",
diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json
@@ -264,7 +264,6 @@
   "detectChangesInEmbeddedViews",
   "detectChangesInView",
   "detectChangesInViewIfAttached",
-  "detectChangesInViewIfRequired",
   "detectChangesInternal",
   "diPublicInInjector",
   "elementEndFirstCreatePass",
@@ -337,7 +336,6 @@
   "getTStylingRangePrev",
   "getTView",
   "getViewRefs",
-  "handleError",
   "handleStoppedNotification",
   "handleUnhandledError",
   "hasApplyArgsData",

Original file line number	Diff line number	Diff line change
`@@ -374,7 +374,6 @@ export class ComponentRef<T> extends AbstractComponentRef<T> {`
`374`	`374`	`this.hostView = this.changeDetectorRef = new ViewRef<T>(`
`375`	`375`	`_rootLView,`
`376`	`376`	`undefined /* _cdRefInjectingView */,`
`377`		`- false /* notifyErrorHandler */,`
`378`	`377`	`);`
`379`	`378`	`this.componentType = componentType;`
`380`	`379`	`}`