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')); + }, + ); + }); }); }