From 8d82a39a601fbedee58f140a7432c4adf0b054ff Mon Sep 17 00:00:00 2001 From: arturovt Date: Mon, 28 Apr 2025 20:54:05 +0300 Subject: [PATCH] fix(core): async EventEmitter error should not prevent stability (#61028) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit wraps the `fn` invocation with `try-finally`, ensuring that the pending task (added in [this commit](https://github.com/angular/angular/commit/d5c6ee432fcd467c09b4d5d5366e731f5c91e8d4)) is always removed. Prior to this commit, if a subscriber threw an error, it would prevent the application from becoming stable — though this shouldn't happen under normal scenarios because the error should be handled by the RxJS error handler or Angular's error handler. Errors should not silently prevent the application from being rendered on the server. PR Close #61028 --- packages/core/src/event_emitter.ts | 9 ++++++--- packages/core/test/event_emitter_spec.ts | 19 +++++++++++++++++-- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/packages/core/src/event_emitter.ts b/packages/core/src/event_emitter.ts index be95088671d..7ac64d981de 100644 --- a/packages/core/src/event_emitter.ts +++ b/packages/core/src/event_emitter.ts @@ -176,9 +176,12 @@ class EventEmitter_ extends Subject implements OutputRef { return (value: unknown) => { const taskId = this.pendingTasks?.add(); setTimeout(() => { - fn(value); - if (taskId !== undefined) { - this.pendingTasks?.remove(taskId); + try { + fn(value); + } finally { + if (taskId !== undefined) { + this.pendingTasks?.remove(taskId); + } } }); }; diff --git a/packages/core/test/event_emitter_spec.ts b/packages/core/test/event_emitter_spec.ts index f01743f6d92..7d295f4748b 100644 --- a/packages/core/test/event_emitter_spec.ts +++ b/packages/core/test/event_emitter_spec.ts @@ -7,10 +7,10 @@ */ import {TestBed} from '../testing'; -import {filter, tap} from 'rxjs/operators'; +import {filter} from 'rxjs/operators'; import {EventEmitter} from '../src/event_emitter'; -import {ApplicationRef} from '../public_api'; +import {ApplicationRef, NgZone} from '../public_api'; describe('EventEmitter', () => { let emitter: EventEmitter; @@ -206,6 +206,21 @@ describe('EventEmitter', () => { expect(emitValue!).toEqual(1); }); + it('should not prevent app from becoming stable if subscriber throws an error', async () => { + const logs: string[] = []; + const ngZone = TestBed.inject(NgZone); + const appRef = TestBed.inject(ApplicationRef); + appRef.isStable.subscribe((isStable) => logs.push(`isStable=${isStable}`)); + const emitter = TestBed.runInInjectionContext(() => new EventEmitter(true)); + emitter.subscribe(() => { + throw new Error('Given this is some TypeError...'); + }); + // Emit inside the Angular zone so that the error is not captured by Jasmine in `afterAll`. + ngZone.run(() => emitter.emit(1)); + await appRef.whenStable(); + expect(logs).toEqual(['isStable=true', 'isStable=false', 'isStable=true']); + }); + // TODO: vsavkin: add tests cases // should call dispose on the subscription if generator returns {done:true} // should call dispose on the subscription on throw