feat(router): Add router configuration to resolve navigation promise …

…on error

With the deprecation of the configurable errorHandler in the Router, there is a missing
use-case to prevent the navigation promise from rejecting on an error. This rejection
results in unhandled promise rejections. This commit allows developers to instruct
the router to instead resolve the navigation promise with 'false', which matches
the behavior of other failed navigations.

Resolving the Promise would be the ideal default behavior. It is rare
that any code handles the navigation Promise at all and even more rare
that the Promise rejection is caught. Updating the default value for
this option should be considered for an upcoming major version.

fixes #48902

goldens/public-api/router/index.md

@@ -733,6 +733,7 @@ export interface RouterConfigOptions {
     canceledNavigationResolution?: 'replace' | 'computed';
     onSameUrlNavigation?: OnSameUrlNavigation;
     paramsInheritanceStrategy?: 'emptyOnly' | 'always';
+    resolveNavigationPromiseOnError?: boolean;
     urlUpdateStrategy?: 'deferred' | 'eager';
 }
 

packages/router/src/navigation_transition.ts

@@ -718,7 +718,20 @@ export class NavigationTransitions {
                              try {
                                overallTransitionState.resolve(router.errorHandler(e));
                              } catch (ee) {
-                               overallTransitionState.reject(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 {
+                                 overallTransitionState.reject(ee);
+                               }
                              }
                            }
                            return EMPTY;

packages/router/src/router.ts

@@ -498,9 +498,9 @@ export class Router {
    * @param extras An options object that determines how the URL should be constructed or
    *     interpreted.
    *
-   * @returns A Promise that resolves to `true` when navigation succeeds, to `false` when navigation
-   *     fails,
-   * or is rejected on error.
+   * @returns A Promise that resolves to `true` when navigation succeeds, or `false` when navigation
+   *     fails. The Promise is rejected when an error occurs if `resolveNavigationPromiseOnError` is
+   * not `true`.
    *
    * @usageNotes
    *

packages/router/src/router_config.ts

@@ -21,6 +21,10 @@ import {UrlSerializer, UrlTree} from './url_tree';
  *
  * @publicApi
  * @deprecated Subscribe to the `Router` events and watch for `NavigationError` instead.
+ *   If the ErrorHandler is used to prevent unhandled promise rejections when navigation
+ *   errors occur, use the `resolveNavigationPromiseOnError` option instead.
+ *
+ * @see RouterConfigOptions
  */
 export type ErrorHandler = (error: any) => any;
 
@@ -110,6 +114,15 @@ export interface RouterConfigOptions {
    * showing an error message with the URL that failed.
    */
   urlUpdateStrategy?: 'deferred'|'eager';
+
+  /**
+   * 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
+   * if an error occurs.
+   */
+  resolveNavigationPromiseOnError?: boolean;
 }
 
 /**
@@ -222,6 +235,10 @@ export interface ExtraOptions extends InMemoryScrollingOptions, RouterConfigOpti
    * If the handler throws an exception, the navigation Promise is rejected with the exception.
    *
    * @deprecated Subscribe to the `Router` events and watch for `NavigationError` instead.
+   *   If the ErrorHandler is used to prevent unhandled promise rejections when navigation
+   *   errors occur, use the `resolveNavigationPromiseOnError` option instead.
+   *
+   * @see RouterConfigOptions
    */
   errorHandler?: (error: any) => any;
 

packages/router/test/computed_state_restoration.spec.ts

@@ -109,7 +109,11 @@ describe('`restoredState#ɵrouterPageId`', () => {
                 canLoad: ['alwaysFalse']
               }
             ],
-            withRouterConfig({urlUpdateStrategy, canceledNavigationResolution: 'computed'})),
+            withRouterConfig({
+              urlUpdateStrategy,
+              canceledNavigationResolution: 'computed',
+              resolveNavigationPromiseOnError: true,
+            })),
       ]
     });
     const router = TestBed.inject(Router);
@@ -477,10 +481,8 @@ describe('`restoredState#ɵrouterPageId`', () => {
 
          TestBed.inject(ThrowingCanActivateGuard).throw = true;
 
-         expect(() => {
-           location.back();
-           advance(fixture);
-         }).toThrow();
+         location.back();
+         advance(fixture);
          expect(location.path()).toEqual('/second');
          expect(location.getState()).toEqual(jasmine.objectContaining({ɵrouterPageId: 2}));
 

packages/router/testing/test/router_testing_harness.spec.ts

@@ -14,6 +14,8 @@ import {RouterTestingHarness} from '@angular/router/testing';
 import {of} from 'rxjs';
 import {delay} from 'rxjs/operators';
 
+import {withRouterConfig} from '../../src/provide_router';
+
 describe('navigateForTest', () => {
   it('gives null for the activatedComponent when no routes are configured', async () => {
     TestBed.configureTestingModule({providers: [provideRouter([])]});
@@ -52,15 +54,20 @@ describe('navigateForTest', () => {
 
   it('throws error if routing throws', async () => {
     TestBed.configureTestingModule({
-      providers: [provideRouter([{
-        path: '',
-        canActivate: [() => {
-          throw new Error('oh no');
-        }],
-        children: []
-      }])]
+      providers: [provideRouter(
+          [
+            {
+              path: 'e',
+              canActivate: [() => {
+                throw new Error('oh no');
+              }],
+              children: []
+            },
+          ],
+          withRouterConfig({resolveNavigationPromiseOnError: true}))]
     });
-    await expectAsync(RouterTestingHarness.create('/')).toBeRejected();
+    const harness = await RouterTestingHarness.create();
+    await expectAsync(harness.navigateByUrl('e')).toBeResolvedTo(null);
   });
 
   it('can observe param changes on routed component with second navigation', async () => {