Skip to content

Commit 637f07c

Browse files
atscottatscott
committed
fix(router): Align RouterModule.forRoot errorHandler with provider error handler
This change aligns the behavior of the error handler in the `ExtraOptions` of `RouterModule.forRoot` with the error handler in `withNavigationErrorHandler`. The changes are: * Slightly different timing: The handler is called before the `NavigationError` emits * Runs in the injection context, meaning it is more configurable at the config location rather than needing to assign the value to the `Router.errorHandler` later to get access to injectables * Can now return `RedirectCommand` to recover from the error and redirect without emitting `NavigationError` * No longer allows arbitrarily overriding return value of the navigation promise BREAKING CHANGE: The `Router.errorHandler` property has been removed. Adding an error handler should be configured in either `withNavigationErrorHandler` with `provideRouter` or the `errorHandler` property in the extra options of `RouterModule.forRoot`. In addition, the error handler cannot be used to change the return value of the router navigation promise or prevent it from rejecting. Instead, if you want to prevent the promise from rejecting, use `resolveNavigationPromiseOnError`.
1 parent 9109f31 commit 637f07c

7 files changed

Lines changed: 51 additions & 69 deletions

File tree

‎goldens/public-api/router/index.api.md‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -298,8 +298,7 @@ export interface ExtraOptions extends InMemoryScrollingOptions, RouterConfigOpti
298298
bindToComponentInputs?: boolean;
299299
enableTracing?: boolean;
300300
enableViewTransitions?: boolean;
301-
// @deprecated
302-
errorHandler?: (error: any) => any;
301+
errorHandler?: (error: any) => RedirectCommand | any;
303302
initialNavigation?: InitialNavigation;
304303
preloadingStrategy?: any;
305304
scrollOffset?: [number, number] | (() => [number, number]);
@@ -705,8 +704,6 @@ export class Router {
705704
config: Routes;
706705
createUrlTree(commands: any[], navigationExtras?: UrlCreationOptions): UrlTree;
707706
dispose(): void;
708-
// @deprecated
709-
errorHandler: (error: any) => any;
710707
get events(): Observable<Event_2>;
711708
getCurrentNavigation(): Navigation | null;
712709
initialNavigation(): void;

‎packages/core/test/bundling/router/bundle.golden_symbols.json‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1103,9 +1103,6 @@
11031103
{
11041104
"name": "defaultErrorFactory"
11051105
},
1106-
{
1107-
"name": "defaultErrorHandler2"
1108-
},
11091106
{
11101107
"name": "defaultIfEmpty"
11111108
},

‎packages/router/src/navigation_transition.ts‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -328,10 +328,6 @@ export interface NavigationTransition {
328328
*/
329329
interface InternalRouterInterface {
330330
config: Routes;
331-
// All of these are public API of router interface and can change during runtime because they are
332-
// writeable. Ideally, these would be removed and the values retrieved instead from the values
333-
// available in DI.
334-
errorHandler: (error: any) => any;
335331
navigated: boolean;
336332
routeReuseStrategy: RouteReuseStrategy;
337333
onSameUrlNavigation: 'reload' | 'ignore';
@@ -893,10 +889,7 @@ export class NavigationTransitions {
893889
);
894890
} else {
895891
this.events.next(navigationError);
896-
// TODO(atscott): remove deprecation on errorHandler in RouterModule.forRoot and change behavior to provide NAVIGATION_ERROR_HANDLER
897-
// Note: Still remove public `Router.errorHandler` property, as this is supposed to be configured in DI.
898-
const errorHandlerResult = router.errorHandler(e);
899-
overallTransitionState.resolve(!!errorHandlerResult);
892+
throw e;
900893
}
901894
} catch (ee) {
902895
// TODO(atscott): consider flipping the default behavior of

‎packages/router/src/router.ts‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -142,15 +142,6 @@ export class Router {
142142
return this.stateManager.getRouterState();
143143
}
144144

145-
/**
146-
* A handler for navigation errors in this NgModule.
147-
*
148-
* @deprecated Subscribe to the `Router` events and watch for `NavigationError` instead.
149-
* `provideRouter` has the `withNavigationErrorHandler` feature to make this easier.
150-
* @see {@link withNavigationErrorHandler}
151-
*/
152-
errorHandler: (error: any) => any = this.options.errorHandler || defaultErrorHandler;
153-
154145
/**
155146
* True if at least one navigation event has occurred,
156147
* false otherwise.

‎packages/router/src/router_config.ts‎

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,23 +8,7 @@
88

99
import {InjectionToken} from '@angular/core';
1010

11-
import {OnSameUrlNavigation, QueryParamsHandling} from './models';
12-
13-
/**
14-
* Error handler that is invoked when a navigation error occurs.
15-
*
16-
* If the handler returns a value, the navigation Promise is resolved with this value.
17-
* If the handler throws an exception, the navigation Promise is rejected with
18-
* the exception.
19-
*
20-
* @publicApi
21-
* @deprecated Subscribe to the `Router` events and watch for `NavigationError` instead.
22-
* If the ErrorHandler is used to prevent unhandled promise rejections when navigation
23-
* errors occur, use the `resolveNavigationPromiseOnError` option instead.
24-
*
25-
* @see RouterConfigOptions
26-
*/
27-
export type ErrorHandler = (error: any) => any;
11+
import {OnSameUrlNavigation, QueryParamsHandling, RedirectCommand} from './models';
2812

2913
/**
3014
* Allowed values in an `ExtraOptions` object that configure
@@ -246,13 +230,9 @@ export interface ExtraOptions extends InMemoryScrollingOptions, RouterConfigOpti
246230
* If the handler returns a value, the navigation Promise is resolved with this value.
247231
* If the handler throws an exception, the navigation Promise is rejected with the exception.
248232
*
249-
* @deprecated Subscribe to the `Router` events and watch for `NavigationError` instead.
250-
* If the ErrorHandler is used to prevent unhandled promise rejections when navigation
251-
* errors occur, use the `resolveNavigationPromiseOnError` option instead.
252-
*
253233
* @see RouterConfigOptions
254234
*/
255-
errorHandler?: (error: any) => any;
235+
errorHandler?: (error: any) => RedirectCommand | any;
256236

257237
/**
258238
* Configures a preloading strategy.

‎packages/router/src/router_module.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import {RouterLinkActive} from './directives/router_link_active';
3434
import {RouterOutlet} from './directives/router_outlet';
3535
import {RuntimeErrorCode} from './errors';
3636
import {Routes} from './models';
37-
import {NavigationTransitions} from './navigation_transition';
37+
import {NAVIGATION_ERROR_HANDLER, NavigationTransitions} from './navigation_transition';
3838
import {
3939
getBootstrapListener,
4040
rootRoute,
@@ -148,6 +148,12 @@ export class RouterModule {
148148
useFactory: provideForRootGuard,
149149
deps: [[Router, new Optional(), new SkipSelf()]],
150150
},
151+
config?.errorHandler
152+
? {
153+
provide: NAVIGATION_ERROR_HANDLER,
154+
useValue: config.errorHandler,
155+
}
156+
: [],
151157
{provide: ROUTER_CONFIGURATION, useValue: config ? config : {}},
152158
config?.useHash ? provideHashLocationStrategy() : providePathLocationStrategy(),
153159
provideRouterScroller(),

‎packages/router/test/integration.spec.ts‎

Lines changed: 40 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1897,6 +1897,46 @@ for (const browserAPI of ['navigation', 'history'] as const) {
18971897
expect(TestBed.inject(Handler).handlerCalled).toBeTrue();
18981898
});
18991899

1900+
it('can redirect from error handler with RouterModule.forRoot', async () => {
1901+
TestBed.configureTestingModule({
1902+
imports: [
1903+
RouterModule.forRoot(
1904+
[
1905+
{
1906+
path: 'throw',
1907+
canMatch: [
1908+
() => {
1909+
throw new Error('');
1910+
},
1911+
],
1912+
component: BlankCmp,
1913+
},
1914+
{path: 'error', component: BlankCmp},
1915+
],
1916+
{
1917+
resolveNavigationPromiseOnError: true,
1918+
errorHandler: () => new RedirectCommand(coreInject(Router).parseUrl('/error')),
1919+
},
1920+
),
1921+
],
1922+
});
1923+
const router = TestBed.inject(Router);
1924+
let emitNavigationError = false;
1925+
let emitNavigationCancelWithRedirect = false;
1926+
router.events.subscribe((e) => {
1927+
if (e instanceof NavigationError) {
1928+
emitNavigationError = true;
1929+
}
1930+
if (e instanceof NavigationCancel && e.code === NavigationCancellationCode.Redirect) {
1931+
emitNavigationCancelWithRedirect = true;
1932+
}
1933+
});
1934+
await router.navigateByUrl('/throw');
1935+
expect(router.url).toEqual('/error');
1936+
expect(emitNavigationError).toBe(false);
1937+
expect(emitNavigationCancelWithRedirect).toBe(true);
1938+
});
1939+
19001940
it('can redirect from error handler', async () => {
19011941
TestBed.configureTestingModule({
19021942
providers: [
@@ -2134,28 +2174,6 @@ for (const browserAPI of ['navigation', 'history'] as const) {
21342174
expect(locationUrlBeforeEmittingError).toEqual('/simple');
21352175
}));
21362176

2137-
it('should support custom error handlers', fakeAsync(
2138-
inject([Router], (router: Router) => {
2139-
router.errorHandler = (error) => 'resolvedValue';
2140-
const fixture = createRoot(router, RootCmp);
2141-
2142-
router.resetConfig([{path: 'user/:name', component: UserCmp}]);
2143-
2144-
const recordedEvents: any[] = [];
2145-
router.events.forEach((e) => recordedEvents.push(e));
2146-
2147-
let e: any;
2148-
router.navigateByUrl('/invalid')!.then((_) => (e = _));
2149-
advance(fixture);
2150-
expect(e).toEqual(true);
2151-
2152-
expectEvents(recordedEvents, [
2153-
[NavigationStart, '/invalid'],
2154-
[NavigationError, '/invalid'],
2155-
]);
2156-
}),
2157-
));
2158-
21592177
it('should recover from malformed uri errors', fakeAsync(
21602178
inject([Router, Location], (router: Router, location: Location) => {
21612179
router.resetConfig([{path: 'simple', component: SimpleCmp}]);

0 commit comments

Comments
 (0)