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
This commit is contained in:
Andrew Scott 2024-04-15 15:17:53 -07:00 committed by Alex Rickabaugh
parent b47ac4d7c7
commit a0eebcd6d1
2 changed files with 70 additions and 5 deletions

View file

@ -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);
});
}

View file

@ -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<void>(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', () => {