Skip to content

Commit

Permalink
fix(router): prevent error handling when injector is destroyed (#59457)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
arturovt authored and alxhub committed Jan 23, 2025
1 parent 6ce8ed7 commit c7b6e11
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 1 deletion.
15 changes: 15 additions & 0 deletions packages/router/src/navigation_transition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import {Location} from '@angular/common';
import {
DestroyRef,
EnvironmentInjector,
inject,
Injectable,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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. */
Expand Down
46 changes: 45 additions & 1 deletion packages/router/test/integration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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({
Expand Down

0 comments on commit c7b6e11

Please sign in to comment.