fix(router): prevent error handling when injector is destroyed (#59457)

arturovt · alxhub · commit e3da35ec7493 · 2025-01-23T14:21:07.000-08:00
In this commit, we prevent error handling when the root injector is already destroyed. This may happen when the observable completes before emitting a value, which would trigger a `catchError` block that attempts to call `runInInjectionContext` on a destroyed injector. PR Close #59457
diff --git a/packages/router/src/navigation_transition.ts b/packages/router/src/navigation_transition.ts
@@ -8,6 +8,7 @@
 
 import {Location} from '@angular/common';
 import {
+  DestroyRef,
   EnvironmentInjector,
   inject,
   Injectable,
@@ -354,6 +355,7 @@ export class NavigationTransitions {
   readonly transitionAbortSubject = new Subject<Error>();
   private readonly configLoader = inject(RouterConfigLoader);
   private readonly environmentInjector = inject(EnvironmentInjector);
+  private readonly destroyRef = inject(DestroyRef);
   private readonly urlSerializer = inject(UrlSerializer);
   private readonly rootContexts = inject(ChildrenOutletContexts);
   private readonly location = inject(Location);
@@ -381,11 +383,16 @@ export class NavigationTransitions {
   /** @internal */
   rootComponentType: Type<any> | null = null;
 
+  private destroyed = false;
+
   constructor() {
     const onLoadStart = (r: Route) => this.events.next(new RouteConfigLoadStart(r));
     const onLoadEnd = (r: Route) => this.events.next(new RouteConfigLoadEnd(r));
     this.configLoader.onLoadEndListener = onLoadEnd;
     this.configLoader.onLoadStartListener = onLoadStart;
+    this.destroyRef.onDestroy(() => {
+      this.destroyed = true;
+    });
   }
 
   complete() {
@@ -831,6 +838,14 @@ export class NavigationTransitions {
             }
           }),
           catchError((e) => {
+            // If the application is already destroyed, the catch block should not
+            // execute anything in practice because other resources have already
+            // been released and destroyed.
+            if (this.destroyed) {
+              overallTransitionState.resolve(false);
+              return EMPTY;
+            }
+
             errored = true;
             /* This error type is issued during Redirect, and is handled as a
              * cancellation rather than an error. */
diff --git a/packages/router/test/integration.spec.ts b/packages/router/test/integration.spec.ts
@@ -25,6 +25,7 @@ import {
   ViewChildren,
   ɵConsole as Console,
   ɵNoopNgZone as NoopNgZone,
+  DestroyRef,
 } from '@angular/core';
 import {ComponentFixture, fakeAsync, inject, TestBed, tick} from '@angular/core/testing';
 import {By} from '@angular/platform-browser/src/dom/debug/by';
@@ -74,7 +75,7 @@ import {
   UrlTree,
 } from '@angular/router';
 import {RouterTestingHarness} from '@angular/router/testing';
-import {concat, EMPTY, firstValueFrom, Observable, Observer, of, Subscription} from 'rxjs';
+import {concat, EMPTY, firstValueFrom, Observable, Observer, of, Subject, Subscription} from 'rxjs';
 import {delay, filter, first, last, map, mapTo, takeWhile, tap} from 'rxjs/operators';
 
 import {
@@ -3619,6 +3620,49 @@ for (const browserAPI of ['navigation', 'history'] as const) {
     });
     describe('guards', () => {
       describe('CanActivate', () => {
+        describe('guard completes before emitting a value', () => {
+          @Injectable({providedIn: 'root'})
+          class CompletesBeforeEmitting {
+            private subject$ = new Subject<boolean>();
+
+            constructor(destroyRef: DestroyRef) {
+              destroyRef.onDestroy(() => this.subject$.complete());
+            }
+
+            // Note that this is a simple illustrative case of when an observable
+            // completes without emitting a value. In a real-world scenario, this
+            // might represent an HTTP request that never emits before the app is
+            // destroyed and then completes when the app is destroyed.
+            canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot) {
+              return this.subject$;
+            }
+          }
+
+          it('should not thrown an unhandled promise rejection', fakeAsync(
+            inject([Router], async (router: Router) => {
+              const fixture = createRoot(router, RootCmp);
+
+              const onUnhandledrejection = jasmine.createSpy();
+              window.addEventListener('unhandledrejection', onUnhandledrejection);
+
+              router.resetConfig([
+                {path: 'team/:id', component: TeamCmp, canActivate: [CompletesBeforeEmitting]},
+              ]);
+
+              router.navigateByUrl('/team/22');
+
+              // This was previously throwing an error `NG0205: Injector has already been destroyed`.
+              fixture.destroy();
+
+              // Wait until the event task is dispatched.
+              await new Promise((resolve) => setTimeout(resolve, 10));
+              window.removeEventListener('unhandledrejection', onUnhandledrejection);
+
+              expect(onUnhandledrejection).not.toHaveBeenCalled();
+            }),
+          ));
+        });
+
         describe('should not activate a route when CanActivate returns false', () => {
           beforeEach(() => {
             TestBed.configureTestingModule({
