Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions packages/go_router/lib/src/parser.dart
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,19 @@ class GoRouteInformationParser extends RouteInformationParser<RouteMatchList> {
),
extra: infoState.extra,
);
final RouteMatchList resolved = onParserException != null
? onParserException!(context, blocked)
: blocked;
return SynchronousFuture<RouteMatchList>(resolved);
if (onParserException != null) {
// Defer past this still-in-flight initial parse, so a recovery
// navigation (e.g. router.go()) made synchronously from
// onParserException/onException isn't discarded by the Router's
// intent-token churn (same reason as the microtask deferral
// below for the onEnter `then` callback).
scheduleMicrotask(() {
if (context.mounted) {
onParserException!(context, blocked);
}
});
}
return SynchronousFuture<RouteMatchList>(blocked);
},
);
}
Expand Down
11 changes: 9 additions & 2 deletions packages/go_router/lib/src/router.dart
Original file line number Diff line number Diff line change
Expand Up @@ -271,8 +271,15 @@ class GoRouter implements RouterConfig<RouteMatchList> {
if (onException != null) {
parserExceptionHandler = (BuildContext context, RouteMatchList routeMatchList) {
onException(context, configuration.buildTopLevelGoRouterState(routeMatchList), this);
// Avoid updating GoRouterDelegate if onException is provided.
return routerDelegate.currentConfiguration;
// Prefer the current configuration whenever it is valid (non-empty
// or an already-installed error configuration), to avoid updating
// GoRouterDelegate with the error match list. Fall back to the
// error match list only when nothing has committed yet
// (e.g., initial deep link blocked by onEnter). This prevents
// an assertion failure in GoRouterDelegate.setNewRoutePath
// which requires configuration.isNotEmpty || configuration.isError.
final RouteMatchList current = routerDelegate.currentConfiguration;
return current.isNotEmpty || current.isError ? current : routeMatchList;
};
} else {
parserExceptionHandler = null;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
changelog: |
- Fixes an assertion failure when `onException` fires during a blocked initial navigation, and defers the call so a recovery navigation made inside it is no longer silently discarded
version: patch
112 changes: 112 additions & 0 deletions packages/go_router/test/exception_handling_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:async';

import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:go_router/go_router.dart';
Expand Down Expand Up @@ -195,5 +197,115 @@ void main() {
expect(find.text('generic error handled'), findsOneWidget);
expect(find.text('should not reach here'), findsNothing);
});

testWidgets('recovers to the fallback route when onException defers router.go '
'during a blocked initial navigation', (WidgetTester tester) async {
final router = GoRouter(
initialLocation: '/protected',
onEnter: (_, GoRouterState current, GoRouterState next, GoRouter goRouter) {
if (next.matchedLocation == '/protected') {
return const Block.stop();
}
return const Allow();
},
onException: (_, GoRouterState state, GoRouter router) {
// Deferred to a microtask by the app itself: this should still
// work now that go_router also defers its own call to
// onException on the initial-navigation path (the sibling test
// below covers a router.go() called synchronously from
// onException).
scheduleMicrotask(() => router.go('/fallback'));
},
routes: <RouteBase>[
GoRoute(path: '/protected', builder: (_, _) => const Text('protected')),
GoRoute(path: '/fallback', builder: (_, _) => const Text('fallback')),
],
);
addTearDown(router.dispose);

await tester.pumpWidget(MaterialApp.router(routerConfig: router));
await tester.pumpAndSettle();

expect(tester.takeException(), isNull);
expect(find.text('fallback'), findsOneWidget);
expect(find.text('protected'), findsNothing);
expect(router.state.uri.path, '/fallback');
});

testWidgets('recovers to the fallback route when onException calls router.go '
'synchronously during a blocked initial navigation', (WidgetTester tester) async {
final router = GoRouter(
initialLocation: '/protected',
onEnter: (_, GoRouterState current, GoRouterState next, GoRouter goRouter) {
if (next.matchedLocation == '/protected') {
return const Block.stop();
}
return const Allow();
},
onException: (_, GoRouterState state, GoRouter router) {
router.go('/fallback');
},
routes: <RouteBase>[
GoRoute(path: '/protected', builder: (_, _) => const Text('protected')),
GoRoute(path: '/fallback', builder: (_, _) => const Text('fallback')),
],
);
addTearDown(router.dispose);

await tester.pumpWidget(MaterialApp.router(routerConfig: router));
await tester.pumpAndSettle();

expect(tester.takeException(), isNull);
expect(find.text('fallback'), findsOneWidget);
expect(find.text('protected'), findsNothing);
expect(router.state.uri.path, '/fallback');
});

testWidgets('renders the default error screen for an unmatched initial navigation '
'when onException does not navigate', (WidgetTester tester) async {
var exceptionCaught = false;

await createRouter(
<RouteBase>[GoRoute(path: '/', builder: (_, GoRouterState state) => const Text('home'))],
tester,
initialLocation: '/unmatched-route',
onException: (_, _, _) {
exceptionCaught = true;
},
);

expect(tester.takeException(), isNull);
expect(exceptionCaught, isTrue);
expect(find.text('Page Not Found'), findsOneWidget);
});

testWidgets('preserves the first installed error configuration when a second '
'unmatched navigation also has onException not navigate', (WidgetTester tester) async {
final visited = <String>[];

final GoRouter router = await createRouter(
<RouteBase>[GoRoute(path: '/', builder: (_, GoRouterState state) => const Text('home'))],
tester,
initialLocation: '/first-unmatched',
onException: (_, GoRouterState state, _) {
visited.add(state.uri.toString());
},
);

expect(tester.takeException(), isNull);
expect(visited, <String>['/first-unmatched']);
expect(router.routerDelegate.currentConfiguration.uri.toString(), '/first-unmatched');

router.go('/second-unmatched');
await tester.pumpAndSettle();

// onException fires again for the second exception, but the
// already-installed error configuration from the first is
// preserved instead of being replaced by the second.
expect(tester.takeException(), isNull);
expect(visited, <String>['/first-unmatched', '/second-unmatched']);
expect(router.routerDelegate.currentConfiguration.uri.toString(), '/first-unmatched');
expect(find.text('Page Not Found'), findsOneWidget);
});
});
}