Skip to content
Open
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
27 changes: 3 additions & 24 deletions packages/go_router/lib/src/configuration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -622,30 +622,9 @@ class RouteConfiguration {
return callback();
}

T? result;
var errorOccurred = false;

runZonedGuarded<void>(
() {
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: <Object?, Object?>{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;
return Zone.current
.fork(zoneValues: <Object?, Object?>{currentRouterKey: router})
.run<T>(callback);
Comment on lines +625 to +627

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

By removing runZonedGuarded and directly returning the result of Zone.run, synchronous exceptions thrown by the redirect callback are no longer wrapped in a GoException (e.g., Exception during redirect: <error>). This is a regression in error handling behavior for synchronous redirect errors.

To preserve the wrapping of synchronous exceptions while still avoiding the issue with unrelated async errors, we can wrap the call in a try-catch block.

    try {
      return Zone.current
          .fork(zoneValues: <Object?, Object?>{currentRouterKey: router})
          .run<T>(callback);
    } catch (error) {
      throw error is GoException
          ? error
          : GoException('Exception during redirect: $error');
    }

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_runInRouterZone is only called from the top-level and route-level redirect handlers, and both call sites already wrap synchronous
exceptions in GoException. The top-level path wraps with Exception during redirect, while the route-level path wraps with Exception during route redirect.

The change here keeps _runInRouterZone focused on providing the router zone value only, while preserving synchronous and returned-Future redirect error
handling at the existing call sites. The added regression test covers the unrelated unawaited async error case from the issue.

}

/// Get the location for the provided route.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
changelog: |
- Fixes unrelated async errors started during redirects being wrapped as `GoException`.
version: patch
32 changes: 32 additions & 0 deletions packages/go_router/test/go_router_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2063,6 +2063,38 @@ void main() {
expect(find.text('should not reach here'), findsNothing);
});

testWidgets('unrelated async errors started during redirect are not wrapped', (
WidgetTester tester,
) async {
final Object expectedError = StateError('background failure');
Object? caughtError;

await runZonedGuarded<Future<void>>(
() async {
await createRouter(
<RouteBase>[
GoRoute(
path: '/',
builder: (BuildContext context, GoRouterState state) => const HomeScreen(),
),
],
tester,
redirect: (BuildContext context, GoRouterState state) {
Future<void>.error(expectedError);
return null;
},
);

await tester.pump();
},
(Object error, StackTrace stackTrace) {
caughtError = error;
},
);

expect(caughtError, same(expectedError));
});

testWidgets('context extension methods work in redirects', (WidgetTester tester) async {
String? capturedNamedLocation;
final routes = <GoRoute>[
Expand Down
Loading