fix(router): Avoid unhandled promise rejections of navigations by def…

…ault

This is the most sane thing to do given that
applications very rarely handle the promise rejection and, as a
result, would get "unhandled promise rejection" console logs.
The vast majority of applications would not be affected by this
change so omitting a migration seems reasonable. Instead,
applications that rely on rejection can specifically opt-in to the
old behavior.

BREAKING CHANGE: Navigation promises will no longer be rejected when a
navigation error occurs. The error is still reported to the error
handler and the events as a `NavigationError` but the promise will
instead resolve to `false` as with any other failed navigation. This
resolves issues that come from unhandled promise rejections, as the
navigation promise is rarely used and likely unhandled if it is
rejected. Tests or production code that rely on the rejection can opt
back in by using setting `resolveNavigationPromiseOnError` to `false` in
`RouterModule.forRoot` or `withRouterConfig` of `provideRouter`.

packages/router/src/navigation_transition.ts

@@ -907,19 +907,10 @@ export class NavigationTransitions {
                   throw e;
                 }
               } catch (ee) {
-                // TODO(atscott): consider flipping the default behavior of
-                // resolveNavigationPromiseOnError to be `resolve(false)` when
-                // undefined. This is the most sane thing to do given that
-                // applications very rarely handle the promise rejection and, as a
-                // result, would get "unhandled promise rejection" console logs.
-                // The vast majority of applications would not be affected by this
-                // change so omitting a migration seems reasonable. Instead,
-                // applications that rely on rejection can specifically opt-in to the
-                // old behavior.
-                if (this.options.resolveNavigationPromiseOnError) {
-                  overallTransitionState.resolve(false);
-                } else {
+                if (this.options.resolveNavigationPromiseOnError === false) {
                   overallTransitionState.reject(ee);
+                } else {
+                  overallTransitionState.resolve(false);
                 }
               }
             }

packages/router/src/router_config.ts

@@ -113,11 +113,11 @@ export interface RouterConfigOptions {
   defaultQueryParamsHandling?: QueryParamsHandling;
 
   /**
-   * When `true`, the `Promise` will instead resolve with `false`, as it does with other failed
-   * navigations (for example, when guards are rejected).
-
-   * Otherwise the `Promise` returned by the Router's navigation with be rejected
+   * When `false` the `Promise` returned by the Router's navigation with be rejected
    * if an error occurs.
+   *
+   * Otherwise, the `Promise` will instead resolve with `false`, as it does with other failed
+   * navigations (for example, when guards are rejected).
    */
   resolveNavigationPromiseOnError?: boolean;
 }

packages/router/test/integration.spec.ts

@@ -1861,10 +1861,8 @@ for (const browserAPI of ['navigation', 'history'] as const) {
         const recordedEvents: Event[] = [];
         router.events.forEach((e) => recordedEvents.push(e));
 
-        let e: any;
-        router.navigateByUrl('/invalid').catch((_) => (e = _));
+        router.navigateByUrl('/invalid');
         advance(fixture);
-        expect(e.message).toContain('Cannot match any routes');
 
         router.navigateByUrl('/user/fedor');
         advance(fixture);
@@ -2072,10 +2070,8 @@ for (const browserAPI of ['navigation', 'history'] as const) {
 
         // Try navigating to a component which throws an error during activation.
         ConditionalThrowingCmp.throwError = true;
-        expect(() => {
-          router.navigateByUrl('/throwing');
-          advance(fixture);
-        }).toThrow();
+        router.navigateByUrl('/throwing');
+        advance(fixture);
         expect(location.path()).toEqual('');
         expect(fixture.nativeElement.innerHTML).not.toContain('throwing');
 
@@ -2212,11 +2208,9 @@ for (const browserAPI of ['navigation', 'history'] as const) {
 
         router.resetConfig([{path: 'simple', component: SimpleCmp}]);
 
-        router.navigateByUrl('/invalid');
-        expect(() => advance(fixture)).toThrow();
-
-        router.navigateByUrl('/invalid2');
-        expect(() => advance(fixture)).toThrow();
+        const p = router.navigateByUrl('/invalid');
+        advance(fixture);
+        expectAsync(p).toBeResolvedTo(false);
       }),
     ));
 
@@ -2495,8 +2489,7 @@ for (const browserAPI of ['navigation', 'history'] as const) {
           const recordedEvents: any[] = [];
           router.events.subscribe((e) => e instanceof RouterEvent && recordedEvents.push(e));
 
-          let e: any = null;
-          router.navigateByUrl('/simple')!.catch((error) => (e = error));
+          router.navigateByUrl('/simple');
           advance(fixture);
 
           expectEvents(recordedEvents, [
@@ -2507,8 +2500,6 @@ for (const browserAPI of ['navigation', 'history'] as const) {
             [ResolveStart, '/simple'],
             [NavigationError, '/simple'],
           ]);
-
-          expect(e).toEqual('error');
         }),
       ));
 
@@ -2523,11 +2514,9 @@ for (const browserAPI of ['navigation', 'history'] as const) {
           const recordedEvents: any[] = [];
           router.events.subscribe((e) => e instanceof RouterEvent && recordedEvents.push(e));
 
-          let e: any = 'some value';
-          router.navigateByUrl('/simple').catch((error) => (e = error));
+          const p = router.navigateByUrl('/simple');
           advance(fixture);
-
-          expect(e).toEqual(null);
+          expectAsync(p).toBeResolvedTo(false);
         }),
       ));
 
@@ -2708,12 +2697,7 @@ for (const browserAPI of ['navigation', 'history'] as const) {
         router.resetConfig([
           {path: 'throwing', resolve: {thrower: ThrowingResolver}, component: BlankCmp},
         ]);
-        try {
-          await router.navigateByUrl('/throwing');
-          fail('navigation should throw');
-        } catch (e: unknown) {
-          expect((e as Error).message).toEqual(errorMessage);
-        }
+        await router.navigateByUrl('/throwing');
 
         expect(caughtError).toBeDefined();
         expect(caughtError?.target).toBeDefined();
@@ -6797,10 +6781,15 @@ for (const browserAPI of ['navigation', 'history'] as const) {
 
           router.resetConfig([{path: 'lazy', loadChildren: () => LoadedModule}]);
 
-          let recordedError: any = null;
-          router.navigateByUrl('/lazy/loaded')!.catch((err) => (recordedError = err));
+          let recordedError: NavigationError | undefined;
+          router.events.subscribe((e) => {
+            if (e instanceof NavigationError) {
+              recordedError = e;
+            }
+          });
+          router.navigateByUrl('/lazy/loaded');
           advance(fixture);
-          expect(recordedError.message).toContain(`NG04007`);
+          expect(recordedError!.error.message).toContain(`NG04007`);
         }),
       ));
 
@@ -7119,11 +7108,16 @@ for (const browserAPI of ['navigation', 'history'] as const) {
         const fixture = createRoot(router, RootCmp);
         router.resetConfig([{path: 'lazy', loadChildren: () => LoadedModule}]);
 
-        let recordedError: any = null;
-        router.navigateByUrl('/lazy/loaded').catch((err) => (recordedError = err));
+        let recordedError: NavigationError | undefined;
+        router.events.subscribe((e) => {
+          if (e instanceof NavigationError) {
+            recordedError = e;
+          }
+        });
+        router.navigateByUrl('/lazy/loaded');
         advance(fixture);
 
-        expect(recordedError.message).toContain(
+        expect(recordedError!.error.message).toContain(
           `Invalid configuration of route 'lazy/loaded'. One of the following must be provided: component, loadComponent, redirectTo, children or loadChildren`,
         );
       }));

packages/router/test/standalone.spec.ts

@@ -9,7 +9,7 @@
 import {Component, Injectable, NgModule} from '@angular/core';
 import {ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing';
 import {By} from '@angular/platform-browser';
-import {provideRoutes, Router, RouterModule, ROUTES} from '@angular/router';
+import {NavigationError, provideRoutes, Router, RouterModule, ROUTES} from '@angular/router';
 
 @Component({template: '<div>simple standalone</div>'})
 export class SimpleStandaloneComponent {}
@@ -62,8 +62,15 @@ describe('standalone in Router API', () => {
       const root = TestBed.createComponent(RootCmp);
 
       const router = TestBed.inject(Router);
+      let caughtError: NavigationError | undefined;
+      router.events.subscribe((e) => {
+        if (e instanceof NavigationError) {
+          caughtError = e;
+        }
+      });
       router.navigateByUrl('/lazy/notstandalone');
-      expect(() => advance(root)).toThrowError(
+      advance(root);
+      expect(caughtError!.error.message).toMatch(
         /.*lazy\/notstandalone.*component must be standalone/,
       );
     }));
@@ -363,8 +370,15 @@ describe('standalone in Router API', () => {
       });
 
       const root = TestBed.createComponent(RootCmp);
+      let caughtError: NavigationError | undefined;
+      TestBed.inject(Router).events.subscribe((e) => {
+        if (e instanceof NavigationError) {
+          caughtError = e;
+        }
+      });
       TestBed.inject(Router).navigateByUrl('/home');
-      expect(() => advance(root)).toThrowError(/.*home.*component must be standalone/);
+      advance(root);
+      expect(caughtError?.error.message).toMatch(/.*home.*component must be standalone/);
     }));
 
     it('throws error when loadComponent is used with a module', fakeAsync(() => {
@@ -382,9 +396,16 @@ describe('standalone in Router API', () => {
         ],
       });
 
+      let caughtError: NavigationError | undefined;
+      TestBed.inject(Router).events.subscribe((e) => {
+        if (e instanceof NavigationError) {
+          caughtError = e;
+        }
+      });
       const root = TestBed.createComponent(RootCmp);
       TestBed.inject(Router).navigateByUrl('/home');
-      expect(() => advance(root)).toThrowError(/.*home.*Use 'loadChildren' instead/);
+      advance(root);
+      expect(caughtError?.error.message).toMatch(/.*home.*Use 'loadChildren' instead/);
     }));
   });
   describe('default export unwrapping', () => {