From 0de856e929d5c62454373b4b49ccfb5974fe6de5 Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Fri, 12 Jun 2026 17:12:03 +0200 Subject: [PATCH] fix(settings): refuse overlapping runAsync at runtime --- .../settings/async_form_state_mixin.dart | 32 +++++------ .../settings/async_form_state_mixin_test.dart | 53 +++++++++++++++++++ 2 files changed, 70 insertions(+), 15 deletions(-) create mode 100644 test/screens/settings/async_form_state_mixin_test.dart diff --git a/lib/screens/settings/async_form_state_mixin.dart b/lib/screens/settings/async_form_state_mixin.dart index 45b70df8..aa3ceb5e 100644 --- a/lib/screens/settings/async_form_state_mixin.dart +++ b/lib/screens/settings/async_form_state_mixin.dart @@ -1,5 +1,7 @@ import 'package:flutter/widgets.dart'; +import '../../utils/app_logger.dart'; + /// Mixin for stateful screens that wrap their async work in a busy + error /// scaffolding. Exposes [busy] and [errorText] state plus a [runAsync] helper /// that clears the prior error, sets busy, runs the body, captures any @@ -13,7 +15,9 @@ import 'package:flutter/widgets.dart'; /// [runAsync] calls must be sequenced, never overlapped: there is a single /// busy flag, so the first call to finish clears it while the other is still /// running. Don't start a second runAsync (or fire one with `unawaited`) -/// while another that can still apply state is in flight — debug-asserted. +/// while another that can still apply state is in flight — debug-asserted, +/// and the overlapping call is refused (returns null) in release so it can't +/// corrupt the busy flag. /// Overlapping a *stale* run whose [runAsync]'s `shouldApplyState` has gone /// false (e.g. a cancelled poll unwinding) is fine; it no longer touches busy. mixin AsyncFormStateMixin on State { @@ -47,16 +51,17 @@ mixin AsyncFormStateMixin on State { }) async { bool canApplyState() => mounted && (shouldApplyState?.call() ?? true); if (!canApplyState()) return null; - assert( - !_activeRunAsyncGuards.any((stillApplies) => stillApplies()), - 'runAsync overlapped with another runAsync that can still apply state: ' - 'the first to finish clears busy out from under the other. Sequence the ' - 'calls (await the first) instead of nesting or unawaiting them.', - ); - assert(() { - _activeRunAsyncGuards.add(canApplyState); - return true; - }()); + if (_activeRunAsyncGuards.any((stillApplies) => stillApplies())) { + assert( + false, + 'runAsync overlapped with another runAsync that can still apply state: ' + 'the first to finish clears busy out from under the other. Sequence the ' + 'calls (await the first) instead of nesting or unawaiting them.', + ); + appLogger.w('runAsync overlapped with an active run; ignoring this call'); + return null; + } + _activeRunAsyncGuards.add(canApplyState); setState(() { _busy = true; _errorText = null; @@ -69,10 +74,7 @@ mixin AsyncFormStateMixin on State { } return null; } finally { - assert(() { - _activeRunAsyncGuards.remove(canApplyState); - return true; - }()); + _activeRunAsyncGuards.remove(canApplyState); if (canApplyState()) setState(() => _busy = false); } } diff --git a/test/screens/settings/async_form_state_mixin_test.dart b/test/screens/settings/async_form_state_mixin_test.dart new file mode 100644 index 00000000..e301ade4 --- /dev/null +++ b/test/screens/settings/async_form_state_mixin_test.dart @@ -0,0 +1,53 @@ +import 'dart:async'; + +import 'package:flutter/widgets.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:plezy/screens/settings/async_form_state_mixin.dart'; + +class _Host extends StatefulWidget { + const _Host(); + + @override + State<_Host> createState() => _HostState(); +} + +class _HostState extends State<_Host> with AsyncFormStateMixin<_Host> { + @override + Widget build(BuildContext context) => const SizedBox.shrink(); +} + +void main() { + testWidgets('sequential runAsync calls manage busy/error state', (tester) async { + await tester.pumpWidget(const _Host()); + final state = tester.state<_HostState>(find.byType(_Host)); + + final result = await state.runAsync(() async => 42); + expect(result, 42); + expect(state.busy, isFalse); + expect(state.errorText, isNull); + + final failed = await state.runAsync(() async => throw Exception('boom'), errorMapper: (_) => 'mapped'); + expect(failed, isNull); + expect(state.errorText, 'mapped'); + expect(state.busy, isFalse); + }); + + testWidgets('overlapping runAsync is refused', (tester) async { + await tester.pumpWidget(const _Host()); + final state = tester.state<_HostState>(find.byType(_Host)); + + final gate = Completer(); + final first = state.runAsync(() => gate.future); + await tester.pump(); + expect(state.busy, isTrue); + + // Debug builds assert (loud failure during development); release builds + // refuse the call with null so the busy flag can't be corrupted. + await expectLater(state.runAsync(() async => 1), throwsAssertionError); + expect(state.busy, isTrue, reason: 'the in-flight run is unaffected'); + + gate.complete(7); + expect(await first, 7); + expect(state.busy, isFalse); + }); +}