From a0eebcd6d14f87fd3f85fddc490ea837d0254632 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Mon, 15 Apr 2024 15:17:53 -0700 Subject: [PATCH] refactor(core): Add error tracking for infinite notifications (#55231) This commit adds helpful stack information for the case when change detection continues to be triggered in the event loop and would cause the browser to freeze. PR Close #55231 --- .../scheduling/zoneless_scheduling_impl.ts | 43 ++++++++++++++++--- .../test/change_detection_scheduler_spec.ts | 32 ++++++++++++++ 2 files changed, 70 insertions(+), 5 deletions(-) diff --git a/packages/core/src/change_detection/scheduling/zoneless_scheduling_impl.ts b/packages/core/src/change_detection/scheduling/zoneless_scheduling_impl.ts index 68bc3319227..58fab7a7913 100644 --- a/packages/core/src/change_detection/scheduling/zoneless_scheduling_impl.ts +++ b/packages/core/src/change_detection/scheduling/zoneless_scheduling_impl.ts @@ -11,6 +11,7 @@ import {Injectable} from '../../di/injectable'; import {inject} from '../../di/injector_compatibility'; import {EnvironmentProviders} from '../../di/interface/provider'; import {makeEnvironmentProviders} from '../../di/provider_collection'; +import {RuntimeError, RuntimeErrorCode} from '../../errors'; import {PendingTasks} from '../../pending_tasks'; import {scheduleCallbackWithMicrotask, scheduleCallbackWithRafRace} from '../../util/callback_scheduler'; import {performanceMarkFeature} from '../../util/performance'; @@ -18,6 +19,27 @@ import {NgZone, NoopNgZone} from '../../zone/ng_zone'; import {ChangeDetectionScheduler, NotificationType, ZONELESS_ENABLED, ZONELESS_SCHEDULER_DISABLED} from './zoneless_scheduling'; +const CONSECUTIVE_MICROTASK_NOTIFICATION_LIMIT = 100; +let consecutiveMicrotaskNotifications = 0; +let stackFromLastFewNotifications: string[] = []; + +function trackMicrotaskNotificationForDebugging() { + consecutiveMicrotaskNotifications++; + if (CONSECUTIVE_MICROTASK_NOTIFICATION_LIMIT - consecutiveMicrotaskNotifications < 5) { + const stack = new Error().stack; + if (stack) { + stackFromLastFewNotifications.push(stack); + } + } + + if (consecutiveMicrotaskNotifications === CONSECUTIVE_MICROTASK_NOTIFICATION_LIMIT) { + throw new RuntimeError( + RuntimeErrorCode.INFINITE_CHANGE_DETECTION, + 'Angular could not stabilize because there were endless change notifications within the browser event loop. ' + + 'The stack from the last several notifications: \n' + + stackFromLastFewNotifications.join('\n')); + } +} @Injectable({providedIn: 'root'}) export class ChangeDetectionSchedulerImpl implements ChangeDetectionScheduler { @@ -41,7 +63,7 @@ export class ChangeDetectionSchedulerImpl implements ChangeDetectionScheduler { this.cleanup(); } }); - private scheduleCallback = scheduleCallbackWithRafRace; + private useMicrotaskScheduler = false; constructor() { // TODO(atscott): These conditions will need to change when zoneless is the default @@ -62,15 +84,26 @@ export class ChangeDetectionSchedulerImpl implements ChangeDetectionScheduler { return; } + if ((typeof ngDevMode === 'undefined' || ngDevMode)) { + if (this.useMicrotaskScheduler) { + trackMicrotaskNotificationForDebugging(); + } else { + consecutiveMicrotaskNotifications = 0; + stackFromLastFewNotifications.length = 0; + } + } + + const scheduleCallback = + this.useMicrotaskScheduler ? scheduleCallbackWithMicrotask : scheduleCallbackWithRafRace; this.pendingRenderTaskId = this.taskService.add(); if (this.zoneIsDefined) { Zone.root.run(() => { - this.cancelScheduledCallback = this.scheduleCallback(() => { + this.cancelScheduledCallback = scheduleCallback(() => { this.tick(this.shouldRefreshViews); }, false /** useNativeTimers */); }); } else { - this.cancelScheduledCallback = this.scheduleCallback(() => { + this.cancelScheduledCallback = scheduleCallback(() => { this.tick(this.shouldRefreshViews); }, false /** useNativeTimers */); } @@ -127,9 +160,9 @@ export class ChangeDetectionSchedulerImpl implements ChangeDetectionScheduler { // which uses Promise.resolve (see NgModel) to avoid // ExpressionChanged...Error to still be reflected in a single browser // paint, even if that spans multiple rounds of change detection. - this.scheduleCallback = scheduleCallbackWithMicrotask; + this.useMicrotaskScheduler = true; scheduleCallbackWithMicrotask(() => { - this.scheduleCallback = scheduleCallbackWithRafRace; + this.useMicrotaskScheduler = false; this.taskService.remove(task); }); } diff --git a/packages/core/test/change_detection_scheduler_spec.ts b/packages/core/test/change_detection_scheduler_spec.ts index 1055d1a7ecd..bf9a8617798 100644 --- a/packages/core/test/change_detection_scheduler_spec.ts +++ b/packages/core/test/change_detection_scheduler_spec.ts @@ -16,6 +16,8 @@ import {withBody} from '@angular/private/testing'; import {BehaviorSubject, firstValueFrom} from 'rxjs'; import {filter, take, tap} from 'rxjs/operators'; +import {RuntimeError, RuntimeErrorCode} from '../src/errors'; +import {handleError} from '../src/render3/instructions/shared'; import {scheduleCallbackWithRafRace} from '../src/util/callback_scheduler'; import {global} from '../src/util/global'; @@ -590,6 +592,36 @@ describe('Angular with zoneless enabled', () => { await new Promise(resolve => scheduleCallbackWithRafRace(resolve)); expect(fixture.nativeElement.innerText).toContain('new'); }); + + it('throws a nice error when notifications prevent exiting the event loop (infinite CD)', + async () => { + let caughtError: unknown; + let previousHandle = (Zone.root as any)._zoneDelegate.handleError; + (Zone.root as any)._zoneDelegate.handleError = (zone: ZoneSpec, e: unknown) => { + caughtError = e; + }; + @Component({ + template: '', + standalone: true, + }) + class App { + cdr = inject(ChangeDetectorRef); + ngDoCheck() { + queueMicrotask(() => { + this.cdr.markForCheck(); + }); + } + } + const fixture = TestBed.createComponent(App); + await fixture.whenStable(); + expect(caughtError).toBeInstanceOf(RuntimeError); + const runtimeError = caughtError as RuntimeError; + expect(runtimeError.code).toEqual(RuntimeErrorCode.INFINITE_CHANGE_DETECTION); + expect(runtimeError.message).toContain('markForCheck'); + expect(runtimeError.message).toContain('notify'); + + (Zone.root as any)._zoneDelegate.handleError = previousHandle; + }); }); describe('Angular with scheduler and ZoneJS', () => {