Skip to content

Commit a55482f

Browse files
JoostKkirjs
authored andcommitted
fix(core): notify profiler events in case of errors
Profiler events are expected to be symmetric, yet in the case of errors this symmetry may break if events aren't always kept in sync with their corresponding start event. This commit moves various end events to be run from a finally-block, allowing them to notify the profiler even when an error has occurred. Fixes #62947 (cherry picked from commit 3760045)
1 parent cc3b4ae commit a55482f

File tree

4 files changed

+133
-11
lines changed

4 files changed

+133
-11
lines changed

packages/core/src/application/application_ref.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -591,6 +591,7 @@ export class ApplicationRef {
591591
private tickImpl = (): void => {
592592
(typeof ngDevMode === 'undefined' || ngDevMode) && warnIfDestroyed(this._destroyed);
593593
if (this._runningTick) {
594+
profiler(ProfilerEvent.ChangeDetectionEnd);
594595
throw new RuntimeError(
595596
RuntimeErrorCode.RECURSIVE_APPLICATION_REF_TICK,
596597
ngDevMode && 'ApplicationRef.tick is called recursively',
@@ -629,8 +630,11 @@ export class ApplicationRef {
629630
let runs = 0;
630631
while (this.dirtyFlags !== ApplicationRefDirtyFlags.None && runs++ < MAXIMUM_REFRESH_RERUNS) {
631632
profiler(ProfilerEvent.ChangeDetectionSyncStart);
632-
this.synchronizeOnce();
633-
profiler(ProfilerEvent.ChangeDetectionSyncEnd);
633+
try {
634+
this.synchronizeOnce();
635+
} finally {
636+
profiler(ProfilerEvent.ChangeDetectionSyncEnd);
637+
}
634638
}
635639

636640
if ((typeof ngDevMode === 'undefined' || ngDevMode) && runs >= MAXIMUM_REFRESH_RERUNS) {

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

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -426,9 +426,11 @@ function detectChangesInComponent(
426426
profiler(ProfilerEvent.ComponentStart);
427427

428428
const componentView = getComponentLViewByIndex(componentHostIdx, hostLView);
429-
detectChangesInViewIfAttached(componentView, mode);
430-
431-
profiler(ProfilerEvent.ComponentEnd, componentView[CONTEXT] as any as {});
429+
try {
430+
detectChangesInViewIfAttached(componentView, mode);
431+
} finally {
432+
profiler(ProfilerEvent.ComponentEnd, componentView[CONTEXT] as any as {});
433+
}
432434
}
433435

434436
/**
@@ -551,8 +553,11 @@ function processHostBindingOpCodes(tView: TView, lView: LView): void {
551553
setBindingRootForHostBindings(bindingRootIndx, directiveIdx);
552554
const context = lView[directiveIdx];
553555
profiler(ProfilerEvent.HostBindingsUpdateStart, context);
554-
hostBindingFn(RenderFlags.Update, context);
555-
profiler(ProfilerEvent.HostBindingsUpdateEnd, context);
556+
try {
557+
hostBindingFn(RenderFlags.Update, context);
558+
} finally {
559+
profiler(ProfilerEvent.HostBindingsUpdateEnd, context);
560+
}
556561
}
557562
}
558563
} finally {

packages/core/src/render3/instructions/render.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,11 @@ export function renderComponent(hostLView: LView, componentHostIdx: number) {
4343

4444
profiler(ProfilerEvent.ComponentStart);
4545

46-
renderView(componentTView, componentView, componentView[CONTEXT]);
47-
48-
profiler(ProfilerEvent.ComponentEnd, componentView[CONTEXT] as any as {});
46+
try {
47+
renderView(componentTView, componentView, componentView[CONTEXT]);
48+
} finally {
49+
profiler(ProfilerEvent.ComponentEnd, componentView[CONTEXT] as any as {});
50+
}
4951
}
5052

5153
/**

packages/core/test/acceptance/profiler_spec.ts

Lines changed: 112 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,117 @@ describe('profiler', () => {
492492
).toBeTrue();
493493
});
494494

495+
it('should capture child component creation events when a template error occurs', () => {
496+
@Component({selector: 'my-child', template: '{{ error() }}'})
497+
class ChildComponent {
498+
constructor() {
499+
throw new Error('Simulated error');
500+
}
501+
}
502+
@Component({selector: 'my-comp', imports: [ChildComponent], template: '<my-child/>'})
503+
class MyComponent {}
504+
505+
expect(() => TestBed.createComponent(MyComponent)).toThrow();
506+
507+
expect(p.hasEvents(ProfilerEvent.ComponentStart, ProfilerEvent.ComponentEnd)).toBeTrue();
508+
});
509+
510+
it('should capture child component change detection events when a template error occurs', () => {
511+
@Component({selector: 'my-child', template: '{{ error() }}'})
512+
class ChildComponent {
513+
error() {
514+
throw new Error('Simulated error');
515+
}
516+
}
517+
@Component({selector: 'my-comp', imports: [ChildComponent], template: '<my-child/>'})
518+
class MyComponent {}
519+
520+
const fixture = TestBed.createComponent(MyComponent);
521+
522+
p.clearEvents();
523+
524+
expect(() => fixture.detectChanges(false)).toThrow();
525+
526+
expect(p.hasEvents(ProfilerEvent.ComponentStart, ProfilerEvent.ComponentEnd)).toBeTrue();
527+
});
528+
529+
it('should capture child component change detection events when a template error occurs', () => {
530+
@Component({selector: 'my-child', template: '{{ error() }}'})
531+
class ChildComponent {
532+
error() {
533+
throw new Error('Simulated error');
534+
}
535+
}
536+
@Component({selector: 'my-comp', imports: [ChildComponent], template: '<my-child/>'})
537+
class MyComponent {}
538+
539+
TestBed.createComponent(MyComponent);
540+
541+
p.clearEvents();
542+
543+
expect(() => TestBed.tick()).toThrow();
544+
expect(p.events).toEqual([
545+
ProfilerEvent.ChangeDetectionStart,
546+
ProfilerEvent.ChangeDetectionSyncStart,
547+
ProfilerEvent.ComponentStart,
548+
ProfilerEvent.TemplateUpdateStart,
549+
ProfilerEvent.TemplateUpdateEnd,
550+
ProfilerEvent.ComponentStart,
551+
ProfilerEvent.TemplateUpdateStart,
552+
ProfilerEvent.TemplateUpdateEnd,
553+
ProfilerEvent.ComponentEnd,
554+
ProfilerEvent.ComponentEnd,
555+
ProfilerEvent.ChangeDetectionSyncEnd,
556+
ProfilerEvent.ChangeDetectionEnd,
557+
]);
558+
});
559+
560+
it('should capture host binding events when an error occurs', () => {
561+
@Component({selector: 'my-comp', host: {'[a]': 'error()'}, template: ''})
562+
class MyComponent {
563+
error() {
564+
throw new Error('Simulated error');
565+
}
566+
}
567+
568+
const fixture = TestBed.createComponent(MyComponent);
569+
570+
p.clearEvents();
571+
572+
expect(() => fixture.detectChanges(false)).toThrow();
573+
574+
expect(
575+
p.hasEvents(ProfilerEvent.HostBindingsUpdateEnd, ProfilerEvent.HostBindingsUpdateEnd),
576+
).toBeTrue();
577+
});
578+
579+
it('should capture symmetric tick events when incorrectly called recursively', () => {
580+
@Component({selector: 'my-comp', template: '{{ illegalTick() }}'})
581+
class MyComponent {
582+
illegalTick() {
583+
TestBed.tick();
584+
}
585+
}
586+
587+
TestBed.createComponent(MyComponent);
588+
p.clearEvents();
589+
590+
expect(() => TestBed.tick()).toThrow();
591+
592+
expect(p.events).toEqual([
593+
ProfilerEvent.ChangeDetectionStart,
594+
ProfilerEvent.ChangeDetectionSyncStart,
595+
ProfilerEvent.ComponentStart,
596+
ProfilerEvent.TemplateUpdateStart,
597+
ProfilerEvent.ChangeDetectionStart,
598+
ProfilerEvent.ChangeDetectionEnd,
599+
ProfilerEvent.TemplateUpdateEnd,
600+
ProfilerEvent.ComponentEnd,
601+
ProfilerEvent.ChangeDetectionSyncEnd,
602+
ProfilerEvent.ChangeDetectionEnd,
603+
]);
604+
});
605+
495606
it('should invoke a profiler when host bindings are evaluated', () => {
496607
@Component({
497608
selector: 'my-comp',
@@ -533,7 +644,7 @@ describe('profiler', () => {
533644
template: `
534645
@defer (on immediate) {
535646
nothing to see here...
536-
}
647+
}
537648
`,
538649
})
539650
class MyComponent {}

0 commit comments

Comments
 (0)