Skip to content

Commit

Permalink
feat(router): Add router configuration to resolve navigation promise …
Browse files Browse the repository at this point in the history
…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.

fixes #48902
  • Loading branch information
atscott committed Aug 31, 2023
1 parent 240b7cd commit 50f7850
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 17 deletions.
6 changes: 5 additions & 1 deletion packages/router/src/navigation_transition.ts
Expand Up @@ -702,7 +702,11 @@ export class NavigationTransitions {
try {
overallTransitionState.resolve(router.errorHandler(e));
} catch (ee) {
overallTransitionState.reject(ee);
if (this.options.resolveNavigationPromiseOnError) {
overallTransitionState.resolve(false);
} else {
overallTransitionState.reject(ee);
}
}
}
return EMPTY;
Expand Down
6 changes: 3 additions & 3 deletions packages/router/src/router.ts
Expand Up @@ -681,9 +681,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
*
Expand Down
9 changes: 9 additions & 0 deletions packages/router/src/router_config.ts
Expand Up @@ -107,6 +107,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;
}

/**
Expand Down
12 changes: 7 additions & 5 deletions packages/router/test/computed_state_restoration.spec.ts
Expand Up @@ -109,7 +109,11 @@ describe('`restoredState#ɵrouterPageId`', () => {
canLoad: ['alwaysFalse']
}
],
withRouterConfig({urlUpdateStrategy, canceledNavigationResolution: 'computed'})),
withRouterConfig({
urlUpdateStrategy,
canceledNavigationResolution: 'computed',
resolveNavigationPromiseOnError: true,
})),
]
});
const router = TestBed.inject(Router);
Expand Down Expand Up @@ -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}));

Expand Down
23 changes: 15 additions & 8 deletions packages/router/testing/test/router_testing_harness.spec.ts
Expand Up @@ -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([])]});
Expand Down Expand Up @@ -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 () => {
Expand Down

0 comments on commit 50f7850

Please sign in to comment.