Skip to content

Commit 769e56b

Browse files
davidmiglozdavidmigloz
authored
[go_router] Fix Block.then() re-entrancy when triggered by refreshListenable (#11136)
Fixes flutter/flutter#183012 `Block.then(() => router.go(...))` and `Allow(then: () => router.go(...))` callbacks in `onEnter` silently lose their navigation when triggered by `refreshListenable`. The `router.go()` inside the callback runs synchronously during `handleTopOnEnter` processing, triggering a re-entrant `_processRouteInformation` whose result is dropped due to transaction token churn in Flutter's `Router`. **Reproduction repo**: https://github.com/davidmigloz/flutter_block_then_bug **Live demo**: https://davidmigloz.github.io/flutter_block_then_bug/ ### Root Cause In `parser.dart`, `handleTopOnEnter` executes the `then` callback via `await Future<void>.sync(callback)` (line 533). When the callback calls `router.go('/login')`, it synchronously triggers `notifyListeners()` → `_processRouteInformation` while the outer parse is still in-flight. Flutter's `Router` mints a new transaction token for the re-entrant parse and silently discards the result. ### Fix Replace `await Future<void>.sync(callback)` with `scheduleMicrotask(() async { await callback(); })`. This defers the callback to the microtask queue, ensuring `router.go()` runs after the current parse has completed and Flutter's `Router` has committed the result. This is the minimal deferral (before any timer/frame), and the behavior change (callbacks fire asynchronously instead of synchronously) is consistent with the documentation ("Executed after the decision is committed"). ### Tests - **5 regression tests** (`on_enter_test.dart`): `Block.then(router.go)` + `refreshListenable`, `Block.then(router.goNamed)` variant, rapid `refreshListenable` emissions, `Allow.then(router.go)` variant, error propagation after deferral. - **Full suite**: 395 tests pass, 0 regressions (2 skipped, pre-existing). ## Pre-Review Checklist [^1]: This PR uses `pending_changelogs/` for versioning and changelog, following the go_router batch release process.
1 parent b04f3e5 commit 769e56b

File tree

3 files changed

+291
-14
lines changed

3 files changed

+291
-14
lines changed

‎packages/go_router/lib/src/parser.dart‎

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -529,20 +529,28 @@ class _OnEnterHandler {
529529
}
530530

531531
if (callback != null) {
532-
try {
533-
await Future<void>.sync(callback);
534-
} catch (error, stack) {
535-
// Log error but don't crash - navigation already committed
536-
log('Error in then callback: $error');
537-
FlutterError.reportError(
538-
FlutterErrorDetails(
539-
exception: error,
540-
stack: stack,
541-
library: 'go_router',
542-
context: ErrorDescription('while executing then callback'),
543-
),
544-
);
545-
}
532+
// Defer the callback to a microtask so that router.go() (or
533+
// similar) inside it does not trigger a re-entrant
534+
// _processRouteInformation while the current parse is still
535+
// in-flight. Without this, Flutter's Router mints a new
536+
// transaction token for the re-entrant parse and silently
537+
// discards the result due to token churn.
538+
scheduleMicrotask(() async {
539+
try {
540+
await callback();
541+
} catch (error, stack) {
542+
// Log error but don't crash - navigation already committed
543+
log('Error in then callback: $error');
544+
FlutterError.reportError(
545+
FlutterErrorDetails(
546+
exception: error,
547+
stack: stack,
548+
library: 'go_router',
549+
context: ErrorDescription('while executing then callback'),
550+
),
551+
);
552+
}
553+
});
546554
}
547555

548556
return matchList;

‎packages/go_router/pending_changelogs/fix_block_then_reentrance.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
changelog: |
2+
- Fixes `Block.then()` and `Allow.then()` navigation callbacks being silently lost when triggered by `refreshListenable` due to re-entrant route processing.
3+
version: patch

‎packages/go_router/test/on_enter_test.dart‎

Lines changed: 266 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1582,5 +1582,271 @@ void main() {
15821582
expect(find.text('Home'), findsOneWidget);
15831583
},
15841584
);
1585+
1586+
group('with refreshListenable', () {
1587+
testWidgets(
1588+
'Block.then(router.go) navigates after refreshListenable fires',
1589+
(WidgetTester tester) async {
1590+
final isAuthenticated = ValueNotifier<bool>(true);
1591+
addTearDown(isAuthenticated.dispose);
1592+
1593+
router = GoRouter(
1594+
initialLocation: '/home',
1595+
refreshListenable: isAuthenticated,
1596+
onEnter:
1597+
(
1598+
BuildContext context,
1599+
GoRouterState current,
1600+
GoRouterState next,
1601+
GoRouter goRouter,
1602+
) {
1603+
// Public routes — always allow
1604+
if (next.uri.path == '/login') {
1605+
return const Allow();
1606+
}
1607+
1608+
// Protected routes — require auth
1609+
if (!isAuthenticated.value) {
1610+
return Block.then(() => goRouter.go('/login'));
1611+
}
1612+
return const Allow();
1613+
},
1614+
routes: <RouteBase>[
1615+
GoRoute(
1616+
path: '/home',
1617+
builder: (_, __) =>
1618+
const Scaffold(body: Center(child: Text('Home'))),
1619+
),
1620+
GoRoute(
1621+
path: '/login',
1622+
builder: (_, __) =>
1623+
const Scaffold(body: Center(child: Text('Login'))),
1624+
),
1625+
],
1626+
);
1627+
1628+
await tester.pumpWidget(MaterialApp.router(routerConfig: router));
1629+
await tester.pumpAndSettle();
1630+
expect(find.text('Home'), findsOneWidget);
1631+
1632+
// Toggle auth off — refreshListenable fires, guard blocks and
1633+
// calls router.go('/login') in Block.then callback.
1634+
isAuthenticated.value = false;
1635+
await tester.pumpAndSettle();
1636+
1637+
// The callback navigation must commit.
1638+
expect(router.state.uri.path, equals('/login'));
1639+
expect(find.text('Login'), findsOneWidget);
1640+
},
1641+
);
1642+
1643+
testWidgets(
1644+
'Block.then(router.goNamed) navigates after refreshListenable fires',
1645+
(WidgetTester tester) async {
1646+
final isAuthenticated = ValueNotifier<bool>(true);
1647+
addTearDown(isAuthenticated.dispose);
1648+
1649+
router = GoRouter(
1650+
initialLocation: '/home',
1651+
refreshListenable: isAuthenticated,
1652+
onEnter:
1653+
(
1654+
BuildContext context,
1655+
GoRouterState current,
1656+
GoRouterState next,
1657+
GoRouter goRouter,
1658+
) {
1659+
if (next.uri.path == '/login') {
1660+
return const Allow();
1661+
}
1662+
1663+
if (!isAuthenticated.value) {
1664+
return Block.then(() => goRouter.goNamed('login'));
1665+
}
1666+
return const Allow();
1667+
},
1668+
routes: <RouteBase>[
1669+
GoRoute(
1670+
path: '/home',
1671+
builder: (_, __) =>
1672+
const Scaffold(body: Center(child: Text('Home'))),
1673+
),
1674+
GoRoute(
1675+
path: '/login',
1676+
name: 'login',
1677+
builder: (_, __) =>
1678+
const Scaffold(body: Center(child: Text('Login'))),
1679+
),
1680+
],
1681+
);
1682+
1683+
await tester.pumpWidget(MaterialApp.router(routerConfig: router));
1684+
await tester.pumpAndSettle();
1685+
expect(find.text('Home'), findsOneWidget);
1686+
1687+
isAuthenticated.value = false;
1688+
await tester.pumpAndSettle();
1689+
1690+
expect(router.state.uri.path, equals('/login'));
1691+
expect(find.text('Login'), findsOneWidget);
1692+
},
1693+
);
1694+
1695+
testWidgets(
1696+
'Block.then(router.go) navigates after multiple rapid refreshListenable emissions',
1697+
(WidgetTester tester) async {
1698+
final authState = ValueNotifier<int>(0);
1699+
addTearDown(authState.dispose);
1700+
1701+
router = GoRouter(
1702+
initialLocation: '/home',
1703+
refreshListenable: authState,
1704+
onEnter:
1705+
(
1706+
BuildContext context,
1707+
GoRouterState current,
1708+
GoRouterState next,
1709+
GoRouter goRouter,
1710+
) {
1711+
if (next.uri.path == '/login') {
1712+
return const Allow();
1713+
}
1714+
1715+
if (authState.value > 2) {
1716+
return Block.then(() => goRouter.go('/login'));
1717+
}
1718+
return const Allow();
1719+
},
1720+
routes: <RouteBase>[
1721+
GoRoute(
1722+
path: '/home',
1723+
builder: (_, __) =>
1724+
const Scaffold(body: Center(child: Text('Home'))),
1725+
),
1726+
GoRoute(
1727+
path: '/login',
1728+
builder: (_, __) =>
1729+
const Scaffold(body: Center(child: Text('Login'))),
1730+
),
1731+
],
1732+
);
1733+
1734+
await tester.pumpWidget(MaterialApp.router(routerConfig: router));
1735+
await tester.pumpAndSettle();
1736+
expect(find.text('Home'), findsOneWidget);
1737+
1738+
// Rapid emissions simulating CombineLatestStream
1739+
authState.value = 1;
1740+
authState.value = 2;
1741+
authState.value = 3; // triggers Block.then
1742+
await tester.pumpAndSettle();
1743+
1744+
expect(router.state.uri.path, equals('/login'));
1745+
expect(find.text('Login'), findsOneWidget);
1746+
},
1747+
);
1748+
1749+
testWidgets(
1750+
'Allow.then(router.go) navigates after refreshListenable fires',
1751+
(WidgetTester tester) async {
1752+
final shouldRedirect = ValueNotifier<bool>(false);
1753+
addTearDown(shouldRedirect.dispose);
1754+
1755+
router = GoRouter(
1756+
initialLocation: '/home',
1757+
refreshListenable: shouldRedirect,
1758+
onEnter:
1759+
(
1760+
BuildContext context,
1761+
GoRouterState current,
1762+
GoRouterState next,
1763+
GoRouter goRouter,
1764+
) {
1765+
if (next.uri.path == '/dashboard') {
1766+
return const Allow();
1767+
}
1768+
1769+
if (shouldRedirect.value && next.uri.path == '/home') {
1770+
return Allow(then: () => goRouter.go('/dashboard'));
1771+
}
1772+
return const Allow();
1773+
},
1774+
routes: <RouteBase>[
1775+
GoRoute(
1776+
path: '/home',
1777+
builder: (_, __) =>
1778+
const Scaffold(body: Center(child: Text('Home'))),
1779+
),
1780+
GoRoute(
1781+
path: '/dashboard',
1782+
builder: (_, __) =>
1783+
const Scaffold(body: Center(child: Text('Dashboard'))),
1784+
),
1785+
],
1786+
);
1787+
1788+
await tester.pumpWidget(MaterialApp.router(routerConfig: router));
1789+
await tester.pumpAndSettle();
1790+
expect(find.text('Home'), findsOneWidget);
1791+
1792+
shouldRedirect.value = true;
1793+
await tester.pumpAndSettle();
1794+
1795+
expect(router.state.uri.path, equals('/dashboard'));
1796+
expect(find.text('Dashboard'), findsOneWidget);
1797+
},
1798+
);
1799+
1800+
testWidgets(
1801+
'Block.then error is reported after refreshListenable fires',
1802+
(WidgetTester tester) async {
1803+
final trigger = ValueNotifier<bool>(false);
1804+
addTearDown(trigger.dispose);
1805+
1806+
FlutterErrorDetails? reported;
1807+
final void Function(FlutterErrorDetails)? oldHandler =
1808+
FlutterError.onError;
1809+
FlutterError.onError = (FlutterErrorDetails details) {
1810+
reported = details;
1811+
};
1812+
addTearDown(() => FlutterError.onError = oldHandler);
1813+
1814+
router = GoRouter(
1815+
initialLocation: '/home',
1816+
refreshListenable: trigger,
1817+
onEnter:
1818+
(
1819+
BuildContext context,
1820+
GoRouterState current,
1821+
GoRouterState next,
1822+
GoRouter goRouter,
1823+
) {
1824+
if (trigger.value && next.uri.path == '/home') {
1825+
return Block.then(() => throw StateError('callback error'));
1826+
}
1827+
return const Allow();
1828+
},
1829+
routes: <RouteBase>[
1830+
GoRoute(
1831+
path: '/home',
1832+
builder: (_, __) =>
1833+
const Scaffold(body: Center(child: Text('Home'))),
1834+
),
1835+
],
1836+
);
1837+
1838+
await tester.pumpWidget(MaterialApp.router(routerConfig: router));
1839+
await tester.pumpAndSettle();
1840+
expect(find.text('Home'), findsOneWidget);
1841+
1842+
trigger.value = true;
1843+
await tester.pumpAndSettle();
1844+
1845+
// Error should be reported (not swallowed)
1846+
expect(reported, isNotNull);
1847+
expect(reported!.exception.toString(), contains('callback error'));
1848+
},
1849+
);
1850+
});
15851851
});
15861852
}

0 commit comments

Comments
 (0)
⚡