fix(core): Catch and report rejections in async function of PendingTasks.run (#60044)

atscott · mmalerba · commit 7232ce5b17c9 · 2025-03-03T17:13:33.000Z
This commit ensures that rejections of the promise of the async function passed to `PendingTasks.run` are not dangling and get reported to the application error handler. This prevents what would likely be a common dangling promise that could end up crashing the node process. BREAKING CHANGE: `PendingTasks.run` no longer returns the result of the async function. If this behavior is desired, it can be re-implemented manually with the `PendingTasks.add`. Be aware, however, that promise rejections will need to be handled or they can cause the node process to shut down when using SSR. PR Close #60044
diff --git a/goldens/public-api/core/index.api.md b/goldens/public-api/core/index.api.md
@@ -1396,7 +1396,7 @@ export const PACKAGE_ROOT_URL: InjectionToken<string>;
 // @public
 export class PendingTasks {
     add(): () => void;
-    run<T>(fn: () => Promise<T>): Promise<T>;
+    run<T>(fn: () => Promise<T>): void;
     // (undocumented)
     static ɵprov: unknown;
 }
diff --git a/packages/core/src/change_detection/scheduling/ng_zone_scheduling.ts b/packages/core/src/change_detection/scheduling/ng_zone_scheduling.ts
@@ -31,6 +31,7 @@ import {
   SCHEDULE_IN_ROOT_ZONE,
 } from './zoneless_scheduling';
 import {SCHEDULE_IN_ROOT_ZONE_DEFAULT} from './flags';
+import {ErrorHandler, INTERNAL_APPLICATION_ERROR_HANDLER} from '../../error_handler';
 
 @Injectable({providedIn: 'root'})
 export class NgZoneChangeDetectionScheduler {
@@ -124,6 +125,14 @@ export function internalProvideZoneChangeDetection({
       provide: SCHEDULE_IN_ROOT_ZONE,
       useValue: scheduleInRootZone ?? SCHEDULE_IN_ROOT_ZONE_DEFAULT,
     },
+    {
+      provide: INTERNAL_APPLICATION_ERROR_HANDLER,
+      useFactory: () => {
+        const zone = inject(NgZone);
+        const userErrorHandler = inject(ErrorHandler);
+        return (e: unknown) => zone.runOutsideAngular(() => userErrorHandler.handleError(e));
+      },
+    },
   ];
 }
 
diff --git a/packages/core/src/error_handler.ts b/packages/core/src/error_handler.ts
@@ -7,7 +7,6 @@
  */
 
 import {inject, InjectionToken} from './di';
-import {NgZone} from './zone';
 
 /**
  * Provides a hook for centralized exception handling.
@@ -53,18 +52,14 @@ export class ErrorHandler {
 
 /**
  * `InjectionToken` used to configure how to call the `ErrorHandler`.
- *
- * `NgZone` is provided by default today so the default (and only) implementation for this
- * is calling `ErrorHandler.handleError` outside of the Angular zone.
  */
 export const INTERNAL_APPLICATION_ERROR_HANDLER = new InjectionToken<(e: any) => void>(
   typeof ngDevMode === 'undefined' || ngDevMode ? 'internal error handler' : '',
   {
     providedIn: 'root',
     factory: () => {
-      const zone = inject(NgZone);
       const userErrorHandler = inject(ErrorHandler);
-      return (e: unknown) => zone.runOutsideAngular(() => userErrorHandler.handleError(e));
+      return (e: unknown) => userErrorHandler.handleError(e);
     },
   },
 );
diff --git a/packages/core/src/pending_tasks.ts b/packages/core/src/pending_tasks.ts
@@ -15,6 +15,7 @@ import {
   ChangeDetectionScheduler,
   NotificationSource,
 } from './change_detection/scheduling/zoneless_scheduling';
+import {INTERNAL_APPLICATION_ERROR_HANDLER} from './error_handler';
 
 /**
  * Internal implementation of the pending tasks service.
@@ -85,8 +86,9 @@ export class PendingTasksInternal implements OnDestroy {
  * @developerPreview
  */
 export class PendingTasks {
-  private internalPendingTasks = inject(PendingTasksInternal);
-  private scheduler = inject(ChangeDetectionScheduler);
+  private readonly internalPendingTasks = inject(PendingTasksInternal);
+  private readonly scheduler = inject(ChangeDetectionScheduler);
+  private readonly errorHandler = inject(INTERNAL_APPLICATION_ERROR_HANDLER);
   /**
    * Adds a new task that should block application's stability.
    * @returns A cleanup function that removes a task when called.
@@ -114,23 +116,11 @@ export class PendingTasks {
    * });
    * ```
    *
-   * Application stability is at least delayed until the next tick after the `run` method resolves
-   * so it is safe to make additional updates to application state that would require UI synchronization:
-   *
-   * ```ts
-   * const userData = await pendingTasks.run(() => fetch('/api/user'));
-   * this.userData.set(userData);
-   * ```
-   *
    * @param fn The asynchronous function to execute
    */
-  async run<T>(fn: () => Promise<T>): Promise<T> {
+  run<T>(fn: () => Promise<T>): void {
     const removeTask = this.add();
-    try {
-      return await fn();
-    } finally {
-      removeTask();
-    }
+    fn().catch(this.errorHandler).finally(removeTask);
   }
 
   /** @nocollapse */
diff --git a/packages/core/test/acceptance/pending_tasks_spec.ts b/packages/core/test/acceptance/pending_tasks_spec.ts
@@ -6,10 +6,10 @@
  * found in the LICENSE file at https://angular.dev/license
  */
 
-import {ApplicationRef, PendingTasks} from '@angular/core';
+import {ApplicationRef, PendingTasks, ErrorHandler} from '@angular/core';
 import {TestBed} from '@angular/core/testing';
 import {EMPTY, firstValueFrom, of} from 'rxjs';
-import {filter, map, take, withLatestFrom} from 'rxjs/operators';
+import {map, withLatestFrom} from 'rxjs/operators';
 
 import {PendingTasksInternal} from '../../src/pending_tasks';
 
@@ -96,36 +96,22 @@ describe('public PendingTasks', () => {
     await expectAsync(TestBed.inject(ApplicationRef).whenStable()).toBeResolved();
   });
 
-  it('should return the result of the run function', async () => {
-    const appRef = TestBed.inject(ApplicationRef);
-    const pendingTasks = TestBed.inject(PendingTasks);
-
-    const result = await pendingTasks.run(async () => {
-      await expectAsync(applicationRefIsStable(appRef)).toBeResolvedTo(false);
-      return 1;
-    });
-
-    expect(result).toBe(1);
-    await expectAsync(applicationRefIsStable(appRef)).toBeResolvedTo(false);
-    await expectAsync(TestBed.inject(ApplicationRef).whenStable()).toBeResolved();
-  });
-
-  xit('should stop blocking stability if run promise rejects', async () => {
+  it('should stop blocking stability if run promise rejects', async () => {
     const appRef = TestBed.inject(ApplicationRef);
     const pendingTasks = TestBed.inject(PendingTasks);
+    const errorHandler = TestBed.inject(ErrorHandler);
+    const spy = spyOn(errorHandler, 'handleError');
 
     let rejectFn: () => void;
-    const task = pendingTasks.run(() => {
+    pendingTasks.run(() => {
       return new Promise<void>((_, reject) => {
         rejectFn = reject;
       });
     });
     await expectAsync(applicationRefIsStable(appRef)).toBeResolvedTo(false);
-    try {
-      rejectFn!();
-      await task;
-    } catch {}
-    await expectAsync(applicationRefIsStable(appRef)).toBeResolvedTo(true);
+    rejectFn!();
+    await expectAsync(appRef.whenStable()).toBeResolved();
+    expect(spy).toHaveBeenCalled();
   });
 });
 

Original file line number	Diff line number	Diff line change
`@@ -1396,7 +1396,7 @@ export const PACKAGE_ROOT_URL: InjectionToken<string>;`
`1396`	`1396`	`// @public`
`1397`	`1397`	`export class PendingTasks {`
`1398`	`1398`	`add(): () => void;`
`1399`		`- run<T>(fn: () => Promise<T>): Promise<T>;`
	`1399`	`+ run<T>(fn: () => Promise<T>): void;`
`1400`	`1400`	`// (undocumented)`
`1401`	`1401`	`static ɵprov: unknown;`
`1402`	`1402`	`}`