[go_router] Fix assertion failure when onException fires on initial navigation#12216
Draft
davidmigloz wants to merge 2 commits into
Draft
[go_router] Fix assertion failure when onException fires on initial navigation#12216davidmigloz wants to merge 2 commits into
davidmigloz wants to merge 2 commits into
Conversation
…avigation When onException fires during the initial navigation (e.g., an initial deep link blocked by onEnter returning Block.stop()), routerDelegate.currentConfiguration is empty. Returning it directly from parserExceptionHandler causes GoRouterDelegate.setNewRoutePath to hit `assert(configuration.isNotEmpty || configuration.isError)`. Fall back to the error match list (isError=true) when the current configuration is empty, satisfying the assertion while any recovery navigation queued from onException is processed. Additionally, defer the onParserException call itself to a microtask on that same initial-navigation, blocked-with-no-prior-route path. Calling it synchronously let the app crash-prevention fix land, but a router.go() made synchronously from onException was silently discarded: Flutter's Router mints a new intent token for the re-entrant parse while the initial parse is still in-flight, dropping the outer result. The deferral is scoped to that one parser.dart call site so ordinary initial navigation exceptions (e.g. a plain unmatched route) keep calling onException synchronously, as existing tests rely on.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When
onExceptionis provided and fires during the initial navigation (e.g. a deep-linked route blocked byonEnterreturningBlock.stop()),parserExceptionHandlerinrouter.dartreturnsrouterDelegate.currentConfiguration. On the initial navigation nothing has committed yet, so that configuration is empty, andGoRouterDelegate.setNewRoutePathcrashes onassert(configuration.isNotEmpty || configuration.isError).Crash sequence:
initialLocationpointing at a route.onEnterreturnsBlock.stop()for it.RouteMatchList(isError == true) and invokesonException.parserExceptionHandlersynchronously returnsrouterDelegate.currentConfiguration— still empty.setNewRoutePathassertsconfiguration.isNotEmpty || configuration.isError— neither holds — crash.The fix has two parts:
router.dart: fall back to the error match list when the current configuration is invalid — neither non-empty norisError— satisfying the assertion'sisErrorbranch.parser.dart: in theonCanNotEnterno-prior-route branch, defer theonParserExceptioncall to a microtask (the same pattern as the existing deferral for theonEnterthencallback nearby). Without this, a recovery navigation made synchronously insideonException— e.g.router.go('/fallback'), as in the issue's repro — is silently discarded by theRouter's intent-token churn, leaving the app parked on the error state. Deferring past the in-flight parse lets synchronous recovery work the wayonExceptionnormally does.Behavior is unchanged everywhere else: non-initial navigations return the non-empty
currentConfigurationas before, the plain unmatched-initial-route path keeps its fully synchronousonExceptioncall (existing redirect tests depend on it, and that path never drops the re-entrant navigation), and withoutonExceptionneither code path is reached.Tests in
exception_handling_test.dart:router.go('/fallback')synchronously from insideonExceptionand asserts full recovery (fallback rendered, protected absent,router.state.uri.path == '/fallback'); a second drives the app-side deferred-via-scheduleMicrotaskpattern. Verified red/green for each half: without therouter.dartfallback the delegate assertion fires atdelegate.dart:221; without theparser.dartdeferral the synchronous recovery is silently dropped and the fallback never renders.router.dartfallback is also covered independently of the deferred path. An unmatched initial navigation with a non-navigatingonExceptionnow renders the default error screen; before the fix the app just stayed blank, because the empty current configuration hitsetNewRoutePath's equality early-return. And a second unmatched navigation no longer replaces the first installed error configuration: the fallback preserves any valid current configuration, whether non-empty orisError.Fixes flutter/flutter#189582
Pre-Review Checklist
[shared_preferences]///).Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2