diff --git a/goldens/public-api/router/index.md b/goldens/public-api/router/index.md index 70a817c58e0d5e..c6d6291d5a00b0 100644 --- a/goldens/public-api/router/index.md +++ b/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'; } diff --git a/packages/router/src/navigation_transition.ts b/packages/router/src/navigation_transition.ts index 87d27c0c1aa1b4..b9eb2086c2a3f5 100644 --- a/packages/router/src/navigation_transition.ts +++ b/packages/router/src/navigation_transition.ts @@ -736,7 +736,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; diff --git a/packages/router/src/router.ts b/packages/router/src/router.ts index 9dbac92f1021cd..79d73a0423d3a9 100644 --- a/packages/router/src/router.ts +++ b/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 * diff --git a/packages/router/src/router_config.ts b/packages/router/src/router_config.ts index 09542d0d510bbd..451d1506ab3efd 100644 --- a/packages/router/src/router_config.ts +++ b/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; diff --git a/packages/router/test/computed_state_restoration.spec.ts b/packages/router/test/computed_state_restoration.spec.ts index fbc4abc93bd800..63979a472e8f31 100644 --- a/packages/router/test/computed_state_restoration.spec.ts +++ b/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})); diff --git a/packages/router/testing/test/router_testing_harness.spec.ts b/packages/router/testing/test/router_testing_harness.spec.ts index 48305cb6b75ae6..8192255ef1da23 100644 --- a/packages/router/testing/test/router_testing_harness.spec.ts +++ b/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 () => {