diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md index 5abdfcd2f0d..5ed3009f964 100644 --- a/packages/go_router/CHANGELOG.md +++ b/packages/go_router/CHANGELOG.md @@ -1,3 +1,8 @@ +## 16.2.5 + +- Fixes `GoRouter.of(context)` access inside redirect callbacks by providing router access through Zone-based context tracking. +- Adds support for using context extension methods (e.g., `context.namedLocation()`, `context.go()`) within redirect callbacks. + ## 16.2.4 - Fix Android Cold Start deep link with empty path losing scheme and authority. diff --git a/packages/go_router/lib/src/configuration.dart b/packages/go_router/lib/src/configuration.dart index 178736637c3..683c88f8ed7 100644 --- a/packages/go_router/lib/src/configuration.dart +++ b/packages/go_router/lib/src/configuration.dart @@ -10,6 +10,7 @@ import 'package:flutter/widgets.dart'; import 'logging.dart'; import 'match.dart'; +import 'misc/constants.dart'; import 'misc/errors.dart'; import 'path_utils.dart'; import 'route.dart'; @@ -29,6 +30,7 @@ class RouteConfiguration { this._routingConfig, { required this.navigatorKey, this.extraCodec, + this.router, }) { _onRoutingTableChanged(); _routingConfig.addListener(_onRoutingTableChanged); @@ -265,6 +267,11 @@ class RouteConfiguration { /// example. final Codec? extraCodec; + /// The GoRouter instance that owns this configuration. + /// + /// This is used to provide access to the router during redirects. + final GoRouter? router; + final Map _nameToPath = {}; /// Looks up the url location by a [GoRoute]'s name. @@ -430,23 +437,50 @@ class RouteConfiguration { } return true; }); - final FutureOr routeLevelRedirectResult = - _getRouteLevelRedirect(context, prevMatchList, routeMatches, 0); - if (routeLevelRedirectResult is String?) { - return processRouteLevelRedirect(routeLevelRedirectResult); + try { + final FutureOr routeLevelRedirectResult = + _getRouteLevelRedirect(context, prevMatchList, routeMatches, 0); + + if (routeLevelRedirectResult is String?) { + return processRouteLevelRedirect(routeLevelRedirectResult); + } + return routeLevelRedirectResult + .then(processRouteLevelRedirect) + .catchError((Object error) { + final GoException goException = + error is GoException + ? error + : GoException( + 'Exception during route redirect: $error', + ); + return _errorRouteMatchList( + prevMatchList.uri, + goException, + extra: prevMatchList.extra, + ); + }); + } catch (exception) { + final GoException goException = + exception is GoException + ? exception + : GoException('Exception during route redirect: $exception'); + return _errorRouteMatchList( + prevMatchList.uri, + goException, + extra: prevMatchList.extra, + ); } - return routeLevelRedirectResult.then( - processRouteLevelRedirect, - ); } redirectHistory.add(prevMatchList); // Check for top-level redirect - final FutureOr topRedirectResult = _routingConfig.value.redirect( - context, - buildTopLevelGoRouterState(prevMatchList), - ); + final FutureOr topRedirectResult = _runInRouterZone(() { + return _routingConfig.value.redirect( + context, + buildTopLevelGoRouterState(prevMatchList), + ); + }); if (topRedirectResult is String?) { return processTopLevelRedirect(topRedirectResult); @@ -479,14 +513,33 @@ class RouteConfiguration { currentCheckIndex + 1, ); final RouteBase route = match.route; - final FutureOr routeRedirectResult = route.redirect!.call( - context, - match.buildState(this, matchList), - ); - if (routeRedirectResult is String?) { - return processRouteRedirect(routeRedirectResult); + try { + final FutureOr routeRedirectResult = _runInRouterZone(() { + return route.redirect!.call(context, match.buildState(this, matchList)); + }); + if (routeRedirectResult is String?) { + return processRouteRedirect(routeRedirectResult); + } + return routeRedirectResult.then(processRouteRedirect).catchError( + (Object error) { + // Convert any exception during async route redirect to a GoException + final GoException goException = + error is GoException + ? error + : GoException('Exception during route redirect: $error'); + // Throw the GoException to be caught by the redirect handling chain + throw goException; + }, + ); + } catch (exception) { + // Convert any exception during route redirect to a GoException + final GoException goException = + exception is GoException + ? exception + : GoException('Exception during route redirect: $exception'); + // Throw the GoException to be caught by the redirect handling chain + throw goException; } - return routeRedirectResult.then(processRouteRedirect); } RouteMatchList _getNewMatches( @@ -498,9 +551,13 @@ class RouteConfiguration { final RouteMatchList newMatch = findMatch(Uri.parse(newLocation)); _addRedirect(redirectHistory, newMatch, previousLocation); return newMatch; - } on GoException catch (e) { - log('Redirection exception: ${e.message}'); - return _errorRouteMatchList(previousLocation, e); + } catch (exception) { + final GoException goException = + exception is GoException + ? exception + : GoException('Exception during redirect: $exception'); + log('Redirection exception: ${goException.message}'); + return _errorRouteMatchList(previousLocation, goException); } } @@ -536,6 +593,39 @@ class RouteConfiguration { .join(' => '); } + /// Runs the given function in a Zone with the router context for redirects. + T _runInRouterZone(T Function() callback) { + if (router == null) { + return callback(); + } + + T? result; + bool errorOccurred = false; + + runZonedGuarded( + () { + result = callback(); + }, + (Object error, StackTrace stack) { + errorOccurred = true; + // Convert any exception during redirect to a GoException and rethrow + final GoException goException = + error is GoException + ? error + : GoException('Exception during redirect: $error'); + throw goException; + }, + zoneValues: {currentRouterKey: router}, + ); + + if (errorOccurred) { + // This should not be reached since we rethrow in the error handler + throw GoException('Unexpected error in router zone'); + } + + return result as T; + } + /// Get the location for the provided route. /// /// Builds the absolute path for the route, by concatenating the paths of the diff --git a/packages/go_router/lib/src/misc/constants.dart b/packages/go_router/lib/src/misc/constants.dart new file mode 100644 index 00000000000..e871c02182b --- /dev/null +++ b/packages/go_router/lib/src/misc/constants.dart @@ -0,0 +1,9 @@ +// Copyright 2013 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:meta/meta.dart'; + +/// Symbol used as a Zone key to track the current GoRouter during redirects. +@internal +const Symbol currentRouterKey = #goRouterRedirectContext; diff --git a/packages/go_router/lib/src/misc/extensions.dart b/packages/go_router/lib/src/misc/extensions.dart index 67de494a87e..4c06c6da9ee 100644 --- a/packages/go_router/lib/src/misc/extensions.dart +++ b/packages/go_router/lib/src/misc/extensions.dart @@ -10,8 +10,6 @@ import '../router.dart'; /// context.go('/'); extension GoRouterHelper on BuildContext { /// Get a location from route name and parameters. - /// - /// This method can't be called during redirects. String namedLocation( String name, { Map pathParameters = const {}, diff --git a/packages/go_router/lib/src/parser.dart b/packages/go_router/lib/src/parser.dart index eb2e8a5af84..020a5bbb657 100644 --- a/packages/go_router/lib/src/parser.dart +++ b/packages/go_router/lib/src/parser.dart @@ -12,6 +12,7 @@ import 'configuration.dart'; import 'information_provider.dart'; import 'logging.dart'; import 'match.dart'; +import 'misc/errors.dart'; import 'router.dart'; /// The function signature of [GoRouteInformationParser.onParserException]. @@ -170,15 +171,47 @@ class GoRouteInformationParser extends RouteInformationParser { BuildContext context, RouteMatchList routeMatch, ) { - final FutureOr redirectedFuture = configuration.redirect( - context, - routeMatch, - redirectHistory: [], - ); - if (redirectedFuture is RouteMatchList) { - return SynchronousFuture(redirectedFuture); + try { + final FutureOr redirectedFuture = configuration.redirect( + context, + routeMatch, + redirectHistory: [], + ); + if (redirectedFuture is RouteMatchList) { + return SynchronousFuture(redirectedFuture); + } + return redirectedFuture.catchError((Object error) { + // Convert any exception during redirect to a GoException + final GoException goException = + error is GoException + ? error + : GoException('Exception during redirect: $error'); + // Return an error match list instead of throwing + return RouteMatchList( + matches: const [], + extra: routeMatch.extra, + error: goException, + uri: routeMatch.uri, + pathParameters: const {}, + ); + }); + } catch (exception) { + // Convert any exception during redirect to a GoException + final GoException goException = + exception is GoException + ? exception + : GoException('Exception during redirect: $exception'); + // Return an error match list instead of throwing + return SynchronousFuture( + RouteMatchList( + matches: const [], + extra: routeMatch.extra, + error: goException, + uri: routeMatch.uri, + pathParameters: const {}, + ), + ); } - return redirectedFuture; } RouteMatchList _updateRouteMatchList( diff --git a/packages/go_router/lib/src/router.dart b/packages/go_router/lib/src/router.dart index 370e52d2a28..cd76b594c39 100644 --- a/packages/go_router/lib/src/router.dart +++ b/packages/go_router/lib/src/router.dart @@ -13,6 +13,7 @@ import 'delegate.dart'; import 'information_provider.dart'; import 'logging.dart'; import 'match.dart'; +import 'misc/constants.dart'; import 'misc/inherited_router.dart'; import 'parser.dart'; import 'route.dart'; @@ -209,6 +210,7 @@ class GoRouter implements RouterConfig { _routingConfig, navigatorKey: navigatorKey, extraCodec: extraCodec, + router: this, ); final ParserExceptionHandler? parserExceptionHandler; @@ -537,24 +539,27 @@ class GoRouter implements RouterConfig { } /// Find the current GoRouter in the widget tree. - /// - /// This method throws when it is called during redirects. static GoRouter of(BuildContext context) { - final GoRouter? inherited = maybeOf(context); - assert(inherited != null, 'No GoRouter found in context'); - return inherited!; + final GoRouter? router = maybeOf(context); + if (router == null) { + throw FlutterError('No GoRouter found in context'); + } + return router; } /// The current GoRouter in the widget tree, if any. - /// - /// This method returns null when it is called during redirects. static GoRouter? maybeOf(BuildContext context) { final InheritedGoRouter? inherited = context .getElementForInheritedWidgetOfExactType() ?.widget as InheritedGoRouter?; - return inherited?.goRouter; + if (inherited != null) { + return inherited.goRouter; + } + + // Check if we're in a redirect context + return Zone.current[currentRouterKey] as GoRouter?; } /// Disposes resource created by this object. diff --git a/packages/go_router/pubspec.yaml b/packages/go_router/pubspec.yaml index 5f5517bb3a5..b474000e38b 100644 --- a/packages/go_router/pubspec.yaml +++ b/packages/go_router/pubspec.yaml @@ -1,7 +1,7 @@ name: go_router description: A declarative router for Flutter based on Navigation 2 supporting deep linking, data-driven routes and more -version: 16.2.4 +version: 16.2.5 repository: https://github.com/flutter/packages/tree/main/packages/go_router issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+go_router%22 diff --git a/packages/go_router/test/exception_handling_test.dart b/packages/go_router/test/exception_handling_test.dart index a3e65aaeddd..2aca1f9f3ca 100644 --- a/packages/go_router/test/exception_handling_test.dart +++ b/packages/go_router/test/exception_handling_test.dart @@ -123,5 +123,118 @@ void main() { await tester.pumpAndSettle(); expect(find.text('home'), findsOneWidget); }); + + testWidgets('can catch errors thrown in redirect callbacks', ( + WidgetTester tester, + ) async { + bool exceptionCaught = false; + String? errorMessage; + + final GoRouter router = await createRouter( + [ + GoRoute( + path: '/', + builder: (_, GoRouterState state) => const Text('home'), + ), + GoRoute( + path: '/error-page', + builder: + (_, GoRouterState state) => + Text('error handled: ${state.extra}'), + ), + GoRoute( + path: '/trigger-error', + builder: + (_, GoRouterState state) => const Text('should not reach here'), + ), + ], + tester, + redirect: (BuildContext context, GoRouterState state) { + if (state.matchedLocation == '/trigger-error') { + // Simulate an error in redirect callback + throw Exception('Redirect error occurred'); + } + return null; + }, + onException: ( + BuildContext context, + GoRouterState state, + GoRouter router, + ) { + exceptionCaught = true; + errorMessage = 'Caught exception for ${state.uri}'; + router.go('/error-page', extra: errorMessage); + }, + ); + + expect(find.text('home'), findsOneWidget); + expect(exceptionCaught, isFalse); + + // Navigate to a route that will trigger an error in the redirect callback + router.go('/trigger-error'); + await tester.pumpAndSettle(); + + // Verify the exception was caught and handled + expect(exceptionCaught, isTrue); + expect(errorMessage, isNotNull); + expect( + find.text('error handled: Caught exception for /trigger-error'), + findsOneWidget, + ); + expect(find.text('should not reach here'), findsNothing); + }); + + testWidgets('can catch non-GoException errors thrown in redirect callbacks', ( + WidgetTester tester, + ) async { + bool exceptionCaught = false; + + final GoRouter router = await createRouter( + [ + GoRoute( + path: '/', + builder: (_, GoRouterState state) => const Text('home'), + ), + GoRoute( + path: '/error-page', + builder: + (_, GoRouterState state) => const Text('generic error handled'), + ), + GoRoute( + path: '/trigger-runtime-error', + builder: + (_, GoRouterState state) => const Text('should not reach here'), + ), + ], + tester, + redirect: (BuildContext context, GoRouterState state) { + if (state.matchedLocation == '/trigger-runtime-error') { + // Simulate a runtime error (not GoException) + throw StateError('Runtime error in redirect'); + } + return null; + }, + onException: ( + BuildContext context, + GoRouterState state, + GoRouter router, + ) { + exceptionCaught = true; + router.go('/error-page'); + }, + ); + + expect(find.text('home'), findsOneWidget); + expect(exceptionCaught, isFalse); + + // Navigate to a route that will trigger a runtime error in the redirect callback + router.go('/trigger-runtime-error'); + await tester.pumpAndSettle(); + + // Verify the exception was caught and handled + expect(exceptionCaught, isTrue); + expect(find.text('generic error handled'), findsOneWidget); + expect(find.text('should not reach here'), findsNothing); + }); }); } diff --git a/packages/go_router/test/go_router_test.dart b/packages/go_router/test/go_router_test.dart index 7a3372b6490..0b4dc08e6fa 100644 --- a/packages/go_router/test/go_router_test.dart +++ b/packages/go_router/test/go_router_test.dart @@ -2389,6 +2389,94 @@ void main() { expect(redirected, isTrue); }); + testWidgets('error thrown during redirect can be caught by onException', ( + WidgetTester tester, + ) async { + bool exceptionCaught = false; + final List routes = [ + GoRoute( + path: '/', + builder: + (BuildContext context, GoRouterState state) => const HomeScreen(), + ), + GoRoute( + path: '/login', + builder: + (BuildContext context, GoRouterState state) => + const LoginScreen(), + ), + GoRoute( + path: '/trigger-error', + builder: + (BuildContext context, GoRouterState state) => + const Text('should not reach here'), + ), + ]; + + final GoRouter router = await createRouter( + routes, + tester, + redirect: (BuildContext context, GoRouterState state) { + if (state.matchedLocation == '/trigger-error') { + throw Exception('Redirect error'); + } + return null; + }, + onException: ( + BuildContext context, + GoRouterState state, + GoRouter router, + ) { + exceptionCaught = true; + }, + ); + + expect(find.byType(HomeScreen), findsOneWidget); + expect(exceptionCaught, isFalse); + + // Navigate to a route that will trigger an error in the redirect callback + router.go('/trigger-error'); + await tester.pumpAndSettle(); + + // Verify the exception was caught + expect(exceptionCaught, isTrue); + // Should stay on the home screen since onException didn't navigate anywhere + expect(find.byType(HomeScreen), findsOneWidget); + expect(find.text('should not reach here'), findsNothing); + }); + + testWidgets('context extension methods work in redirects', ( + WidgetTester tester, + ) async { + String? capturedNamedLocation; + final List routes = [ + GoRoute( + path: '/', + name: 'home', + builder: + (BuildContext context, GoRouterState state) => const HomeScreen(), + ), + GoRoute( + path: '/login', + name: 'login', + builder: + (BuildContext context, GoRouterState state) => + const LoginScreen(), + ), + ]; + + await createRouter( + routes, + tester, + redirect: (BuildContext context, GoRouterState state) { + capturedNamedLocation = context.namedLocation('login'); + return state.matchedLocation == '/login' ? null : '/login'; + }, + ); + + expect(capturedNamedLocation, '/login'); + }); + testWidgets('redirect can redirect to same path', ( WidgetTester tester, ) async {