Skip to content

Commit 491b0a4

Browse files
atscottkirjs
atscott
authored and
kirjs
committed
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
1 parent af2e79e commit 491b0a4

16 files changed

Lines changed: 11 additions & 51 deletions

File tree

‎packages/core/src/application/application_ref.ts‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -669,13 +669,8 @@ export class ApplicationRef {
669669
this.dirtyFlags |= ApplicationRefDirtyFlags.AfterRender;
670670

671671
// Check all potentially dirty views.
672-
for (let {_lView, notifyErrorHandler} of this.allViews) {
673-
detectChangesInViewIfRequired(
674-
_lView,
675-
notifyErrorHandler,
676-
useGlobalCheck,
677-
this.zonelessEnabled,
678-
);
672+
for (let {_lView} of this.allViews) {
673+
detectChangesInViewIfRequired(_lView, useGlobalCheck, this.zonelessEnabled);
679674
}
680675

681676
// If `markForCheck()` was called during view checking, it will have set the `ViewTreeCheck`
@@ -893,7 +888,6 @@ export const enum ApplicationRefDirtyFlags {
893888

894889
export function detectChangesInViewIfRequired(
895890
lView: LView,
896-
notifyErrorHandler: boolean,
897891
isFirstPass: boolean,
898892
zonelessEnabled: boolean,
899893
) {
@@ -910,5 +904,5 @@ export function detectChangesInViewIfRequired(
910904
ChangeDetectionMode.Global
911905
: // Only refresh views with the `RefreshView` flag or views is a changed signal
912906
ChangeDetectionMode.Targeted;
913-
detectChangesInternal(lView, notifyErrorHandler, mode);
907+
detectChangesInternal(lView, mode);
914908
}

‎packages/core/src/change_detection/scheduling/exhaustive_check_no_changes.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ export class DebugNgZoneForCheckNoChanges extends NgZone {
117117
this.applicationRef ||= this.injector.get(ApplicationRef);
118118
for (const view of this.applicationRef.allViews) {
119119
try {
120-
checkNoChangesInternal(view._lView, this.checkNoChangesMode, view.notifyErrorHandler);
120+
checkNoChangesInternal(view._lView, this.checkNoChangesMode);
121121
} catch (e) {
122122
this.errorHandler ||= this.injector.get(ErrorHandler);
123123
this.errorHandler.handleError(e);
@@ -153,7 +153,7 @@ function exhaustiveCheckNoChangesInterval(
153153

154154
for (const view of applicationRef.allViews) {
155155
try {
156-
checkNoChangesInternal(view._lView, checkNoChangesMode, view.notifyErrorHandler);
156+
checkNoChangesInternal(view._lView, checkNoChangesMode);
157157
} catch (e) {
158158
errorHandler.handleError(e);
159159
}

‎packages/core/src/render3/component_ref.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,6 @@ export class ComponentRef<T> extends AbstractComponentRef<T> {
374374
this.hostView = this.changeDetectorRef = new ViewRef<T>(
375375
_rootLView,
376376
undefined /* _cdRefInjectingView */,
377-
false /* notifyErrorHandler */,
378377
);
379378
this.componentType = componentType;
380379
}

‎packages/core/src/render3/instructions/change_detection.ts‎

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -69,18 +69,14 @@ import {profiler} from '../profiler';
6969
import {ProfilerEvent} from '../profiler_types';
7070
import {executeViewQueryFn, refreshContentQueries} from '../queries/query_execution';
7171
import {runEffectsInView} from '../reactivity/view_effect_runner';
72-
import {executeTemplate, handleError} from './shared';
72+
import {executeTemplate} from './shared';
7373

7474
/**
7575
* The maximum number of times the change detection traversal will rerun before throwing an error.
7676
*/
7777
export const MAXIMUM_REFRESH_RERUNS = 100;
7878

79-
export function detectChangesInternal(
80-
lView: LView,
81-
notifyErrorHandler = true,
82-
mode = ChangeDetectionMode.Global,
83-
) {
79+
export function detectChangesInternal(lView: LView, mode = ChangeDetectionMode.Global) {
8480
const environment = lView[ENVIRONMENT];
8581
const rendererFactory = environment.rendererFactory;
8682

@@ -95,11 +91,6 @@ export function detectChangesInternal(
9591

9692
try {
9793
detectChangesInViewWhileDirty(lView, mode);
98-
} catch (error) {
99-
if (notifyErrorHandler) {
100-
handleError(lView, error);
101-
}
102-
throw error;
10394
} finally {
10495
if (!checkNoChangesMode) {
10596
rendererFactory.end?.();
@@ -146,14 +137,10 @@ function detectChangesInViewWhileDirty(lView: LView, mode: ChangeDetectionMode)
146137
}
147138
}
148139

149-
export function checkNoChangesInternal(
150-
lView: LView,
151-
mode: CheckNoChangesMode,
152-
notifyErrorHandler = true,
153-
) {
140+
export function checkNoChangesInternal(lView: LView, mode: CheckNoChangesMode) {
154141
setIsInCheckNoChangesMode(mode);
155142
try {
156-
detectChangesInternal(lView, notifyErrorHandler);
143+
detectChangesInternal(lView);
157144
} finally {
158145
setIsInCheckNoChangesMode(CheckNoChangesMode.Off);
159146
}

‎packages/core/src/render3/view_ref.ts‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,6 @@ export class ViewRef<T> implements EmbeddedViewRef<T>, ChangeDetectorRefInterfac
7474
* This may be different from `_lView` if the `_cdRefInjectingView` is an embedded view.
7575
*/
7676
private _cdRefInjectingView?: LView,
77-
readonly notifyErrorHandler = true,
7877
) {}
7978

8079
get context(): T {
@@ -327,7 +326,7 @@ export class ViewRef<T> implements EmbeddedViewRef<T>, ChangeDetectorRefInterfac
327326
// until the end of the refresh. Using `RefreshView` prevents creating a potential difference
328327
// in the state of the LViewFlags during template execution.
329328
this._lView[FLAGS] |= LViewFlags.RefreshView;
330-
detectChangesInternal(this._lView, this.notifyErrorHandler);
329+
detectChangesInternal(this._lView);
331330
}
332331

333332
/**
@@ -338,11 +337,7 @@ export class ViewRef<T> implements EmbeddedViewRef<T>, ChangeDetectorRefInterfac
338337
*/
339338
checkNoChanges(): void {
340339
if (ngDevMode) {
341-
checkNoChangesInternal(
342-
this._lView,
343-
CheckNoChangesMode.OnlyDirtyViews,
344-
this.notifyErrorHandler,
345-
);
340+
checkNoChangesInternal(this._lView, CheckNoChangesMode.OnlyDirtyViews);
346341
}
347342
}
348343

‎packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,6 @@
270270
"detectChangesInEmbeddedViews",
271271
"detectChangesInView",
272272
"detectChangesInViewIfAttached",
273-
"detectChangesInViewIfRequired",
274273
"detectChangesInternal",
275274
"diPublicInInjector",
276275
"documentElement",

‎packages/core/test/bundling/animations/bundle.golden_symbols.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,6 @@
291291
"detectChangesInEmbeddedViews",
292292
"detectChangesInView",
293293
"detectChangesInViewIfAttached",
294-
"detectChangesInViewIfRequired",
295294
"detectChangesInternal",
296295
"diPublicInInjector",
297296
"documentElement",

‎packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,6 @@
223223
"detectChangesInEmbeddedViews",
224224
"detectChangesInView",
225225
"detectChangesInViewIfAttached",
226-
"detectChangesInViewIfRequired",
227226
"detectChangesInternal",
228227
"diPublicInInjector",
229228
"elementEndFirstCreatePass",

‎packages/core/test/bundling/defer/bundle.golden_symbols.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,6 @@
271271
"detectChangesInEmbeddedViews",
272272
"detectChangesInView",
273273
"detectChangesInViewIfAttached",
274-
"detectChangesInViewIfRequired",
275274
"detectChangesInternal",
276275
"diPublicInInjector",
277276
"elementEndFirstCreatePass",

‎packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,6 @@
324324
"detectChangesInEmbeddedViews",
325325
"detectChangesInView",
326326
"detectChangesInViewIfAttached",
327-
"detectChangesInViewIfRequired",
328327
"detectChangesInternal",
329328
"diPublicInInjector",
330329
"elementEndFirstCreatePass",
@@ -408,7 +407,6 @@
408407
"getTStylingRangePrev",
409408
"getTView",
410409
"getViewRefs",
411-
"handleError",
412410
"handleStoppedNotification",
413411
"handleUnhandledError",
414412
"hasApplyArgsData",

0 commit comments

Comments
 (0)