From e0ea78e538b256ee1b66e9e6ea68dcf0b4023e15 Mon Sep 17 00:00:00 2001 From: David Miguel Date: Fri, 27 Feb 2026 16:42:18 +0100 Subject: [PATCH] [go_router] Fix Block.then() re-entrancy when triggered by refreshListenable `Block.then(() => router.go(...))` callbacks silently lose their navigation when triggered by `refreshListenable`. The `router.go()` inside the callback runs synchronously during `handleTopOnEnter`, triggering a re-entrant `_processRouteInformation` whose result is dropped due to transaction token churn in Flutter's Router. Fix: wrap the `then` callback in `scheduleMicrotask()` so it runs after the current parse completes and Flutter's Router has committed the result. This is consistent with the `then` documentation which states the callback is "executed after the decision is committed". Adds 5 regression tests covering Block.then + refreshListenable, goNamed variant, rapid emissions, Allow.then, and error propagation. Fixes https://github.com/flutter/flutter/issues/183012 --- packages/go_router/lib/src/parser.dart | 36 ++- .../fix_block_then_reentrance.yaml | 3 + packages/go_router/test/on_enter_test.dart | 266 ++++++++++++++++++ 3 files changed, 291 insertions(+), 14 deletions(-) create mode 100644 packages/go_router/pending_changelogs/fix_block_then_reentrance.yaml diff --git a/packages/go_router/lib/src/parser.dart b/packages/go_router/lib/src/parser.dart index eb7a43b4b9d2..c900b02d8570 100644 --- a/packages/go_router/lib/src/parser.dart +++ b/packages/go_router/lib/src/parser.dart @@ -529,20 +529,28 @@ class _OnEnterHandler { } if (callback != null) { - try { - await Future.sync(callback); - } catch (error, stack) { - // Log error but don't crash - navigation already committed - log('Error in then callback: $error'); - FlutterError.reportError( - FlutterErrorDetails( - exception: error, - stack: stack, - library: 'go_router', - context: ErrorDescription('while executing then callback'), - ), - ); - } + // Defer the callback to a microtask so that router.go() (or + // similar) inside it does not trigger a re-entrant + // _processRouteInformation while the current parse is still + // in-flight. Without this, Flutter's Router mints a new + // transaction token for the re-entrant parse and silently + // discards the result due to token churn. + scheduleMicrotask(() async { + try { + await callback(); + } catch (error, stack) { + // Log error but don't crash - navigation already committed + log('Error in then callback: $error'); + FlutterError.reportError( + FlutterErrorDetails( + exception: error, + stack: stack, + library: 'go_router', + context: ErrorDescription('while executing then callback'), + ), + ); + } + }); } return matchList; diff --git a/packages/go_router/pending_changelogs/fix_block_then_reentrance.yaml b/packages/go_router/pending_changelogs/fix_block_then_reentrance.yaml new file mode 100644 index 000000000000..c0c0dcc6b0c2 --- /dev/null +++ b/packages/go_router/pending_changelogs/fix_block_then_reentrance.yaml @@ -0,0 +1,3 @@ +changelog: | + - Fixes `Block.then()` and `Allow.then()` navigation callbacks being silently lost when triggered by `refreshListenable` due to re-entrant route processing. +version: patch diff --git a/packages/go_router/test/on_enter_test.dart b/packages/go_router/test/on_enter_test.dart index 0656b16bfaf1..c9408ea4183d 100644 --- a/packages/go_router/test/on_enter_test.dart +++ b/packages/go_router/test/on_enter_test.dart @@ -1582,5 +1582,271 @@ void main() { expect(find.text('Home'), findsOneWidget); }, ); + + group('with refreshListenable', () { + testWidgets( + 'Block.then(router.go) navigates after refreshListenable fires', + (WidgetTester tester) async { + final isAuthenticated = ValueNotifier(true); + addTearDown(isAuthenticated.dispose); + + router = GoRouter( + initialLocation: '/home', + refreshListenable: isAuthenticated, + onEnter: + ( + BuildContext context, + GoRouterState current, + GoRouterState next, + GoRouter goRouter, + ) { + // Public routes — always allow + if (next.uri.path == '/login') { + return const Allow(); + } + + // Protected routes — require auth + if (!isAuthenticated.value) { + return Block.then(() => goRouter.go('/login')); + } + return const Allow(); + }, + routes: [ + GoRoute( + path: '/home', + builder: (_, __) => + const Scaffold(body: Center(child: Text('Home'))), + ), + GoRoute( + path: '/login', + builder: (_, __) => + const Scaffold(body: Center(child: Text('Login'))), + ), + ], + ); + + await tester.pumpWidget(MaterialApp.router(routerConfig: router)); + await tester.pumpAndSettle(); + expect(find.text('Home'), findsOneWidget); + + // Toggle auth off — refreshListenable fires, guard blocks and + // calls router.go('/login') in Block.then callback. + isAuthenticated.value = false; + await tester.pumpAndSettle(); + + // The callback navigation must commit. + expect(router.state.uri.path, equals('/login')); + expect(find.text('Login'), findsOneWidget); + }, + ); + + testWidgets( + 'Block.then(router.goNamed) navigates after refreshListenable fires', + (WidgetTester tester) async { + final isAuthenticated = ValueNotifier(true); + addTearDown(isAuthenticated.dispose); + + router = GoRouter( + initialLocation: '/home', + refreshListenable: isAuthenticated, + onEnter: + ( + BuildContext context, + GoRouterState current, + GoRouterState next, + GoRouter goRouter, + ) { + if (next.uri.path == '/login') { + return const Allow(); + } + + if (!isAuthenticated.value) { + return Block.then(() => goRouter.goNamed('login')); + } + return const Allow(); + }, + routes: [ + GoRoute( + path: '/home', + builder: (_, __) => + const Scaffold(body: Center(child: Text('Home'))), + ), + GoRoute( + path: '/login', + name: 'login', + builder: (_, __) => + const Scaffold(body: Center(child: Text('Login'))), + ), + ], + ); + + await tester.pumpWidget(MaterialApp.router(routerConfig: router)); + await tester.pumpAndSettle(); + expect(find.text('Home'), findsOneWidget); + + isAuthenticated.value = false; + await tester.pumpAndSettle(); + + expect(router.state.uri.path, equals('/login')); + expect(find.text('Login'), findsOneWidget); + }, + ); + + testWidgets( + 'Block.then(router.go) navigates after multiple rapid refreshListenable emissions', + (WidgetTester tester) async { + final authState = ValueNotifier(0); + addTearDown(authState.dispose); + + router = GoRouter( + initialLocation: '/home', + refreshListenable: authState, + onEnter: + ( + BuildContext context, + GoRouterState current, + GoRouterState next, + GoRouter goRouter, + ) { + if (next.uri.path == '/login') { + return const Allow(); + } + + if (authState.value > 2) { + return Block.then(() => goRouter.go('/login')); + } + return const Allow(); + }, + routes: [ + GoRoute( + path: '/home', + builder: (_, __) => + const Scaffold(body: Center(child: Text('Home'))), + ), + GoRoute( + path: '/login', + builder: (_, __) => + const Scaffold(body: Center(child: Text('Login'))), + ), + ], + ); + + await tester.pumpWidget(MaterialApp.router(routerConfig: router)); + await tester.pumpAndSettle(); + expect(find.text('Home'), findsOneWidget); + + // Rapid emissions simulating CombineLatestStream + authState.value = 1; + authState.value = 2; + authState.value = 3; // triggers Block.then + await tester.pumpAndSettle(); + + expect(router.state.uri.path, equals('/login')); + expect(find.text('Login'), findsOneWidget); + }, + ); + + testWidgets( + 'Allow.then(router.go) navigates after refreshListenable fires', + (WidgetTester tester) async { + final shouldRedirect = ValueNotifier(false); + addTearDown(shouldRedirect.dispose); + + router = GoRouter( + initialLocation: '/home', + refreshListenable: shouldRedirect, + onEnter: + ( + BuildContext context, + GoRouterState current, + GoRouterState next, + GoRouter goRouter, + ) { + if (next.uri.path == '/dashboard') { + return const Allow(); + } + + if (shouldRedirect.value && next.uri.path == '/home') { + return Allow(then: () => goRouter.go('/dashboard')); + } + return const Allow(); + }, + routes: [ + GoRoute( + path: '/home', + builder: (_, __) => + const Scaffold(body: Center(child: Text('Home'))), + ), + GoRoute( + path: '/dashboard', + builder: (_, __) => + const Scaffold(body: Center(child: Text('Dashboard'))), + ), + ], + ); + + await tester.pumpWidget(MaterialApp.router(routerConfig: router)); + await tester.pumpAndSettle(); + expect(find.text('Home'), findsOneWidget); + + shouldRedirect.value = true; + await tester.pumpAndSettle(); + + expect(router.state.uri.path, equals('/dashboard')); + expect(find.text('Dashboard'), findsOneWidget); + }, + ); + + testWidgets( + 'Block.then error is reported after refreshListenable fires', + (WidgetTester tester) async { + final trigger = ValueNotifier(false); + addTearDown(trigger.dispose); + + FlutterErrorDetails? reported; + final void Function(FlutterErrorDetails)? oldHandler = + FlutterError.onError; + FlutterError.onError = (FlutterErrorDetails details) { + reported = details; + }; + addTearDown(() => FlutterError.onError = oldHandler); + + router = GoRouter( + initialLocation: '/home', + refreshListenable: trigger, + onEnter: + ( + BuildContext context, + GoRouterState current, + GoRouterState next, + GoRouter goRouter, + ) { + if (trigger.value && next.uri.path == '/home') { + return Block.then(() => throw StateError('callback error')); + } + return const Allow(); + }, + routes: [ + GoRoute( + path: '/home', + builder: (_, __) => + const Scaffold(body: Center(child: Text('Home'))), + ), + ], + ); + + await tester.pumpWidget(MaterialApp.router(routerConfig: router)); + await tester.pumpAndSettle(); + expect(find.text('Home'), findsOneWidget); + + trigger.value = true; + await tester.pumpAndSettle(); + + // Error should be reported (not swallowed) + expect(reported, isNotNull); + expect(reported!.exception.toString(), contains('callback error')); + }, + ); + }); }); }