From 5cea2f7e7a5a3550a6f990de709ce489ace02f8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Sasovsky?= Date: Wed, 30 Jul 2025 16:47:28 -0300 Subject: [PATCH 1/8] feat: access GoRouter.of from redirect methods --- packages/go_router/lib/src/configuration.dart | 57 ++++++++++++------- .../go_router/lib/src/misc/extensions.dart | 2 - packages/go_router/lib/src/router.dart | 30 ++++++++-- packages/go_router/test/go_router_test.dart | 55 ++++++++++++++++++ 4 files changed, 117 insertions(+), 27 deletions(-) diff --git a/packages/go_router/lib/src/configuration.dart b/packages/go_router/lib/src/configuration.dart index a730500b6b0..38652ec9b2d 100644 --- a/packages/go_router/lib/src/configuration.dart +++ b/packages/go_router/lib/src/configuration.dart @@ -16,6 +16,9 @@ import 'route.dart'; import 'router.dart'; import 'state.dart'; +/// Symbol used as a Zone key to track the current GoRouter during redirects. +const Symbol _currentRouterKey = #goRouterRedirectContext; + /// The signature of the redirect callback. typedef GoRouterRedirect = FutureOr Function( BuildContext context, GoRouterState state); @@ -29,6 +32,7 @@ class RouteConfiguration { this._routingConfig, { required this.navigatorKey, this.extraCodec, + this.router, }) { _onRoutingTableChanged(); _routingConfig.addListener(_onRoutingTableChanged); @@ -232,22 +236,17 @@ class RouteConfiguration { /// The limit for the number of consecutive redirects. int get redirectLimit => _routingConfig.value.redirectLimit; - /// The global key for top level navigator. + /// The global key for the root navigator. final GlobalKey navigatorKey; - /// The codec used to encode and decode extra into a serializable format. - /// - /// When navigating using [GoRouter.go] or [GoRouter.push], one can provide - /// an `extra` parameter along with it. If the extra contains complex data, - /// consider provide a codec for serializing and deserializing the extra data. - /// - /// See also: - /// * [Navigation](https://pub.dev/documentation/go_router/latest/topics/Navigation-topic.html) - /// topic. - /// * [extra_codec](https://github.com/flutter/packages/blob/main/packages/go_router/example/lib/extra_codec.dart) - /// example. + /// The codec used to encode and decode the extra parameter. See + /// [GoRoute.builder] and [GoRoute.pageBuilder] for more info. 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. @@ -416,10 +415,12 @@ class RouteConfiguration { 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); @@ -448,10 +449,12 @@ class RouteConfiguration { _getRouteLevelRedirect( context, matchList, routeMatches, currentCheckIndex + 1); final RouteBase route = match.route; - final FutureOr routeRedirectResult = route.redirect!.call( - context, - match.buildState(this, matchList), - ); + final FutureOr routeRedirectResult = _runInRouterZone(() { + return route.redirect!.call( + context, + match.buildState(this, matchList), + ); + }); if (routeRedirectResult is String?) { return processRouteRedirect(routeRedirectResult); } @@ -508,6 +511,20 @@ 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(); + } + + return runZoned( + callback, + zoneValues: { + _currentRouterKey: router, + }, + ); + } + /// 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/extensions.dart b/packages/go_router/lib/src/misc/extensions.dart index 3c390c94598..f8cca2249d3 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/router.dart b/packages/go_router/lib/src/router.dart index 9962dd2d449..31e8fa3ef4d 100644 --- a/packages/go_router/lib/src/router.dart +++ b/packages/go_router/lib/src/router.dart @@ -18,6 +18,9 @@ import 'parser.dart'; import 'route.dart'; import 'state.dart'; +/// Symbol used as a Zone key to track the current GoRouter during redirects. +const Symbol _currentRouterKey = #goRouterRedirectContext; + /// The function signature of [GoRouter.onException]. /// /// Use `state.error` to access the exception. @@ -206,6 +209,7 @@ class GoRouter implements RouterConfig { _routingConfig, navigatorKey: navigatorKey, extraCodec: extraCodec, + router: this, ); final ParserExceptionHandler? parserExceptionHandler; @@ -519,21 +523,37 @@ class GoRouter implements RouterConfig { /// Find the current GoRouter in the widget tree. /// - /// This method throws when it is called during redirects. + /// This method can now be called during redirects. static GoRouter of(BuildContext context) { final GoRouter? inherited = maybeOf(context); - assert(inherited != null, 'No GoRouter found in context'); - return inherited!; + if (inherited != null) { + return inherited; + } + + // Check if we're in a redirect context + final GoRouter? redirectRouter = + Zone.current[_currentRouterKey] as GoRouter?; + if (redirectRouter != null) { + return redirectRouter; + } + + throw FlutterError('No GoRouter found in context'); } /// The current GoRouter in the widget tree, if any. /// - /// This method returns null when it is called during redirects. + /// This method can now return a router even 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/test/go_router_test.dart b/packages/go_router/test/go_router_test.dart index 5d6b142c313..e8d27639d08 100644 --- a/packages/go_router/test/go_router_test.dart +++ b/packages/go_router/test/go_router_test.dart @@ -2238,6 +2238,61 @@ void main() { expect(redirected, isTrue); }); + testWidgets('GoRouter.of(context) should work in redirects', + (WidgetTester tester) async { + GoRouter? capturedRouter; + final List routes = [ + GoRoute( + path: '/', + builder: (BuildContext context, GoRouterState state) => + const HomeScreen(), + ), + GoRoute( + path: '/login', + builder: (BuildContext context, GoRouterState state) => + const LoginScreen(), + ), + ]; + + final GoRouter router = await createRouter(routes, tester, + redirect: (BuildContext context, GoRouterState state) { + // This should not throw an exception + capturedRouter = GoRouter.of(context); + return state.matchedLocation == '/login' ? null : '/login'; + }); + + expect(capturedRouter, isNotNull); + expect(capturedRouter, equals(router)); + }); + + testWidgets('Context extension methods should 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) { + // This should not throw an exception + capturedNamedLocation = context.namedLocation('login'); + return state.matchedLocation == '/login' ? null : '/login'; + }); + + expect(capturedNamedLocation, '/login'); + }); + testWidgets('redirect can redirect to same path', (WidgetTester tester) async { final List routes = [ From 05b94b72b92d981f5acceef1cf43771cbd048332 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Sasovsky?= Date: Wed, 30 Jul 2025 16:54:34 -0300 Subject: [PATCH 2/8] refactor: update documentation for navigatorKey and extraCodec in RouteConfiguration --- packages/go_router/lib/src/configuration.dart | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/go_router/lib/src/configuration.dart b/packages/go_router/lib/src/configuration.dart index 38652ec9b2d..2293e88b37e 100644 --- a/packages/go_router/lib/src/configuration.dart +++ b/packages/go_router/lib/src/configuration.dart @@ -236,11 +236,20 @@ class RouteConfiguration { /// The limit for the number of consecutive redirects. int get redirectLimit => _routingConfig.value.redirectLimit; - /// The global key for the root navigator. + /// The global key for top level navigator. final GlobalKey navigatorKey; - /// The codec used to encode and decode the extra parameter. See - /// [GoRoute.builder] and [GoRoute.pageBuilder] for more info. + /// The codec used to encode and decode extra into a serializable format. + /// + /// When navigating using [GoRouter.go] or [GoRouter.push], one can provide + /// an `extra` parameter along with it. If the extra contains complex data, + /// consider provide a codec for serializing and deserializing the extra data. + /// + /// See also: + /// * [Navigation](https://pub.dev/documentation/go_router/latest/topics/Navigation-topic.html) + /// topic. + /// * [extra_codec](https://github.com/flutter/packages/blob/main/packages/go_router/example/lib/extra_codec.dart) + /// example. final Codec? extraCodec; /// The GoRouter instance that owns this configuration. From c93570b61f1a88275279158fdf47b1c589c34885 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Sasovsky?= Date: Wed, 30 Jul 2025 17:00:53 -0300 Subject: [PATCH 3/8] refactor: replace _currentRouterKey with currentRouterKey constant for redirect context tracking --- packages/go_router/lib/src/configuration.dart | 6 ++--- .../go_router/lib/src/misc/constants.dart | 5 +++++ packages/go_router/lib/src/router.dart | 22 +++++-------------- 3 files changed, 13 insertions(+), 20 deletions(-) create mode 100644 packages/go_router/lib/src/misc/constants.dart diff --git a/packages/go_router/lib/src/configuration.dart b/packages/go_router/lib/src/configuration.dart index 2293e88b37e..ba61d475fd6 100644 --- a/packages/go_router/lib/src/configuration.dart +++ b/packages/go_router/lib/src/configuration.dart @@ -10,15 +10,13 @@ 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'; import 'router.dart'; import 'state.dart'; -/// Symbol used as a Zone key to track the current GoRouter during redirects. -const Symbol _currentRouterKey = #goRouterRedirectContext; - /// The signature of the redirect callback. typedef GoRouterRedirect = FutureOr Function( BuildContext context, GoRouterState state); @@ -529,7 +527,7 @@ class RouteConfiguration { return runZoned( callback, zoneValues: { - _currentRouterKey: router, + currentRouterKey: router, }, ); } 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..2a89463c3c2 --- /dev/null +++ b/packages/go_router/lib/src/misc/constants.dart @@ -0,0 +1,5 @@ +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/router.dart b/packages/go_router/lib/src/router.dart index 31e8fa3ef4d..305a620899d 100644 --- a/packages/go_router/lib/src/router.dart +++ b/packages/go_router/lib/src/router.dart @@ -13,14 +13,12 @@ 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'; import 'state.dart'; -/// Symbol used as a Zone key to track the current GoRouter during redirects. -const Symbol _currentRouterKey = #goRouterRedirectContext; - /// The function signature of [GoRouter.onException]. /// /// Use `state.error` to access the exception. @@ -525,19 +523,11 @@ class GoRouter implements RouterConfig { /// /// This method can now be called during redirects. static GoRouter of(BuildContext context) { - final GoRouter? inherited = maybeOf(context); - if (inherited != null) { - return inherited; - } - - // Check if we're in a redirect context - final GoRouter? redirectRouter = - Zone.current[_currentRouterKey] as GoRouter?; - if (redirectRouter != null) { - return redirectRouter; + final GoRouter? router = maybeOf(context); + if (router == null) { + throw FlutterError('No GoRouter found in context'); } - - throw FlutterError('No GoRouter found in context'); + return router; } /// The current GoRouter in the widget tree, if any. @@ -553,7 +543,7 @@ class GoRouter implements RouterConfig { } // Check if we're in a redirect context - return Zone.current[_currentRouterKey] as GoRouter?; + return Zone.current[currentRouterKey] as GoRouter?; } /// Disposes resource created by this object. From 858ea74467f70b9fb8509181e6ee6a002158f817 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Sasovsky?= Date: Thu, 31 Jul 2025 13:25:06 -0300 Subject: [PATCH 4/8] chore: release version 16.0.1 with fixes for redirect callbacks and context extension methods --- packages/go_router/CHANGELOG.md | 5 +++++ packages/go_router/pubspec.yaml | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md index b4f32dc31ed..3641c1a489f 100644 --- a/packages/go_router/CHANGELOG.md +++ b/packages/go_router/CHANGELOG.md @@ -1,3 +1,8 @@ +## 16.0.1 + +- 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.0.0 - **BREAKING CHANGE** diff --git a/packages/go_router/pubspec.yaml b/packages/go_router/pubspec.yaml index 3debeeaf158..6a3256aa7dd 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.0.0 +version: 16.0.1 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 From 61bc8ffc3a9ab59b68ec78198e2175f652054dae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Sasovsky?= Date: Thu, 31 Jul 2025 13:37:14 -0300 Subject: [PATCH 5/8] refactor: update documentation for GoRouter context methods to clarify usage during redirects --- packages/go_router/lib/src/router.dart | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/go_router/lib/src/router.dart b/packages/go_router/lib/src/router.dart index 305a620899d..6f2d85d2977 100644 --- a/packages/go_router/lib/src/router.dart +++ b/packages/go_router/lib/src/router.dart @@ -520,8 +520,6 @@ class GoRouter implements RouterConfig { } /// Find the current GoRouter in the widget tree. - /// - /// This method can now be called during redirects. static GoRouter of(BuildContext context) { final GoRouter? router = maybeOf(context); if (router == null) { @@ -531,8 +529,6 @@ class GoRouter implements RouterConfig { } /// The current GoRouter in the widget tree, if any. - /// - /// This method can now return a router even during redirects. static GoRouter? maybeOf(BuildContext context) { final InheritedGoRouter? inherited = context .getElementForInheritedWidgetOfExactType() From 6309387aa5e54708e77f2cde636843acec479d49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Sasovsky?= Date: Fri, 1 Aug 2025 17:21:11 -0300 Subject: [PATCH 6/8] feat: implement error handling in router zone for GoRouter --- packages/go_router/lib/src/configuration.dart | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/packages/go_router/lib/src/configuration.dart b/packages/go_router/lib/src/configuration.dart index ba61d475fd6..e34958916e9 100644 --- a/packages/go_router/lib/src/configuration.dart +++ b/packages/go_router/lib/src/configuration.dart @@ -251,6 +251,7 @@ class RouteConfiguration { final Codec? extraCodec; /// The GoRouter instance that owns this configuration. + /// /// This is used to provide access to the router during redirects. final GoRouter? router; @@ -529,6 +530,28 @@ class RouteConfiguration { zoneValues: { currentRouterKey: router, }, + zoneSpecification: ZoneSpecification( + handleUncaughtError: ( + Zone zone, + ZoneDelegate zoneDelegate, + Zone zone2, + Object error, + StackTrace stackTrace, + ) { + // Handle errors by routing them to GoRouter's error handling mechanisms + if (error is GoException) { + // For GoException, we can let it propagate as it's already handled + // by the existing error handling in redirect methods + throw error; + } else { + // For other errors, we should route them to the router's error handling + // This will be handled by GoRouter.onException or error builders + log('Error in router zone: $error'); + // Convert the error to a GoException for proper error handling + throw GoException('error in router zone: $error'); + } + }, + ), ); } From fec0356dcd91fcef12b540cf40b1b04ee9a752f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Sasovsky?= Date: Fri, 1 Aug 2025 23:14:15 -0300 Subject: [PATCH 7/8] refactor: replace runZoned with runZonedGuarded for improved error handling in router zone --- packages/go_router/lib/src/configuration.dart | 40 ++++++++----------- 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/packages/go_router/lib/src/configuration.dart b/packages/go_router/lib/src/configuration.dart index e34958916e9..78d29ff76fd 100644 --- a/packages/go_router/lib/src/configuration.dart +++ b/packages/go_router/lib/src/configuration.dart @@ -525,34 +525,26 @@ class RouteConfiguration { return callback(); } - return runZoned( + return runZonedGuarded( callback, + (Object error, StackTrace stackTrace) { + // Handle errors by routing them to GoRouter's error handling mechanisms + if (error is GoException) { + // For GoException, we can let it propagate as it's already handled + // by the existing error handling in redirect methods + throw error; + } else { + // For other errors, we should route them to the router's error handling + // This will be handled by GoRouter.onException or error builders + log('Error in router zone: $error'); + // Convert the error to a GoException for proper error handling + throw GoException('error in router zone: $error'); + } + }, zoneValues: { currentRouterKey: router, }, - zoneSpecification: ZoneSpecification( - handleUncaughtError: ( - Zone zone, - ZoneDelegate zoneDelegate, - Zone zone2, - Object error, - StackTrace stackTrace, - ) { - // Handle errors by routing them to GoRouter's error handling mechanisms - if (error is GoException) { - // For GoException, we can let it propagate as it's already handled - // by the existing error handling in redirect methods - throw error; - } else { - // For other errors, we should route them to the router's error handling - // This will be handled by GoRouter.onException or error builders - log('Error in router zone: $error'); - // Convert the error to a GoException for proper error handling - throw GoException('error in router zone: $error'); - } - }, - ), - ); + )!; } /// Get the location for the provided route. From 2cdaa913e57faa6f928bf6730bef79297c478ef0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Sasovsky?= Date: Tue, 5 Aug 2025 00:39:13 -0300 Subject: [PATCH 8/8] feat: enhance error handling in redirect callbacks and tests for GoRouter --- packages/go_router/lib/src/configuration.dart | 111 ++++++++++++------ packages/go_router/lib/src/parser.dart | 39 +++++- .../test/exception_handling_test.dart | 93 +++++++++++++++ packages/go_router/test/go_router_test.dart | 44 +++++-- 4 files changed, 237 insertions(+), 50 deletions(-) diff --git a/packages/go_router/lib/src/configuration.dart b/packages/go_router/lib/src/configuration.dart index 78d29ff76fd..9426103859b 100644 --- a/packages/go_router/lib/src/configuration.dart +++ b/packages/go_router/lib/src/configuration.dart @@ -411,14 +411,30 @@ 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); @@ -457,16 +473,34 @@ class RouteConfiguration { _getRouteLevelRedirect( context, matchList, routeMatches, currentCheckIndex + 1); final RouteBase route = match.route; - final FutureOr routeRedirectResult = _runInRouterZone(() { - return 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( @@ -478,9 +512,12 @@ 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); } } @@ -525,26 +562,32 @@ class RouteConfiguration { return callback(); } - return runZonedGuarded( - callback, - (Object error, StackTrace stackTrace) { - // Handle errors by routing them to GoRouter's error handling mechanisms - if (error is GoException) { - // For GoException, we can let it propagate as it's already handled - // by the existing error handling in redirect methods - throw error; - } else { - // For other errors, we should route them to the router's error handling - // This will be handled by GoRouter.onException or error builders - log('Error in router zone: $error'); - // Convert the error to a GoException for proper error handling - throw GoException('error in router zone: $error'); - } + 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. diff --git a/packages/go_router/lib/src/parser.dart b/packages/go_router/lib/src/parser.dart index d1981898a8b..7d67351e8a2 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]. @@ -160,12 +161,40 @@ class GoRouteInformationParser extends RouteInformationParser { Future _redirect( 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/test/exception_handling_test.dart b/packages/go_router/test/exception_handling_test.dart index d8ad3c3f565..4fb140be985 100644 --- a/packages/go_router/test/exception_handling_test.dart +++ b/packages/go_router/test/exception_handling_test.dart @@ -108,5 +108,98 @@ 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 e8d27639d08..93a861a603f 100644 --- a/packages/go_router/test/go_router_test.dart +++ b/packages/go_router/test/go_router_test.dart @@ -2238,9 +2238,9 @@ void main() { expect(redirected, isTrue); }); - testWidgets('GoRouter.of(context) should work in redirects', + testWidgets('error thrown during redirect can be caught by onException', (WidgetTester tester) async { - GoRouter? capturedRouter; + bool exceptionCaught = false; final List routes = [ GoRoute( path: '/', @@ -2252,19 +2252,41 @@ void main() { 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) { - // This should not throw an exception - capturedRouter = GoRouter.of(context); - return state.matchedLocation == '/login' ? null : '/login'; - }); + 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(capturedRouter, isNotNull); - expect(capturedRouter, equals(router)); - }); + 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 should work in redirects', (WidgetTester tester) async { String? capturedNamedLocation;