fix(core): async EventEmitter error should not prevent stability (#61028)

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
This commit is contained in:
arturovt
2025-04-28 20:54:05 +03:00
committed by Miles Malerba
parent e6a34277fc
commit 8d82a39a60
2 changed files with 23 additions and 5 deletions
+6 -3
View File
@@ -176,9 +176,12 @@ class EventEmitter_ extends Subject<any> implements OutputRef<any> {
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);
}
}
});
};
+17 -2
View File
@@ -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<number>;
@@ -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<number>(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