Skip to content

[go_router] Fix redirect zone async error handling - #12836

Open
ktul15 wants to merge 1 commit into
flutter:mainfrom
ktul15:fix-go-router-redirect-zone
Open

[go_router] Fix redirect zone async error handling#12836
ktul15 wants to merge 1 commit into
flutter:mainfrom
ktul15:fix-go-router-redirect-zone

Conversation

@ktul15

@ktul15 ktul15 commented Sep 11, 2026

Copy link
Copy Markdown

Fixes flutter/flutter#192066

Summary

Replaces the guarded redirect zone with a value-only zone when running redirect callbacks.

Redirect callbacks still have access to the current router through zone values, so APIs such as GoRouter.of(context) and context.namedLocation(...)
continue to work during redirects. However, unrelated unawaited async errors started during a redirect are no longer caught by go_router and wrapped as
GoException.

Tests

  • fvm dart run ../../script/tool/bin/flutter_plugin_tools.dart format --packages go_router
  • fvm dart run ../../script/tool/bin/flutter_plugin_tools.dart analyze --packages go_router
  • fvm dart run ../../script/tool/bin/flutter_plugin_tools.dart dart-test --packages go_router
  • fvm dart run ../../script/tool/bin/flutter_plugin_tools.dart validate --packages go_router
  • fvm flutter test test/go_router_test.dart
  • fvm flutter test test/redirect_chain_test.dart

For the checklist, check the ones that are true:

  • I read the Contributor Guide...
  • I read the AI contribution guidelines...
  • I read the Tree Hygiene page...
  • I read and followed the relevant style guides and ran the auto-formatter.
  • I signed the CLA.
  • The title of the PR starts with the name of the package surrounded by square brackets...
  • I linked to at least one issue that this PR fixes in the description above.
  • I followed the version and CHANGELOG instructions...
  • I updated/added any relevant documentation...
  • I added new tests to check the change I am making...
  • All existing and new tests are passing.

@google-cla

google-cla Bot commented Sep 11, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request replaces runZonedGuarded with Zone.current.fork().run in RouteConfiguration to prevent unrelated async errors during redirects from being wrapped as GoException. It also adds a corresponding test case and a changelog entry. The reviewer noted that this change introduces a regression because synchronous exceptions thrown by the redirect callback are no longer wrapped in a GoException. They suggested wrapping the execution in a try-catch block to preserve the original error-wrapping behavior.

Comment on lines +625 to +627
return Zone.current
.fork(zoneValues: <Object?, Object?>{currentRouterKey: router})
.run<T>(callback);

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[go_router] An unrelated async failure that merely starts during a redirect is rethrown as a fatal GoException

1 participant