@tduncan / Cortana / commits / d148b20

fix: add mounted checks to server setup methods to prevent state mutation crashes after disposal #65

abdulmominsakib committed Aug 19, 2026 at 16:20 UTC d148b2043a8472e66f9562e141694e40ec6bd1d8
3 files changed +165
lib/features/onboarding/screens/onboarding_server_setup_screen.dart
+10
@@ -160,6 +160,14 @@ class _OnboardingServerSetupScreenState
160 Future<void> _testConnection() async {
161 if (_formKey.currentState?.validate() != true) return;
162
163 + // Defensive mounted check before any state mutation. Even though the
164 + // entry point here is a user tap (which implies mounted == true), a
165 + // stale callback could still trigger this method after the widget
166 + // has been disposed. Without this guard the first setState would
167 + // throw "Null check operator used on a null value" at
168 + // package:flutter/src/widgets/framework.dart from
169 + // `_element!.markNeedsBuild()`. See #65.
170 + if (!mounted) return;
171 setState(() {
172 _isTesting = true;
173 _testResult = null;
@@ -200,6 +208,8 @@ class _OnboardingServerSetupScreenState
208 Future<void> _saveAndContinue() async {
209 if (_formKey.currentState?.validate() != true) return;
210
211 + // Defensive mounted check before any state mutation. See #65.
212 + if (!mounted) return;
213 setState(() {
214 _isSaving = true;
215 });
lib/features/servers/views/add_server_screen.dart
+4
@@ -117,6 +117,8 @@ class _AddServerScreenState extends ConsumerState<AddServerScreen> {
117 Future<void> _testConnection() async {
118 if (_formKey.currentState?.validate() != true) return;
119
120 + // Defensive mounted check before any state mutation. See #65.
121 + if (!mounted) return;
122 setState(() {
123 _isTesting = true;
124 _testResult = null;
@@ -208,6 +210,8 @@ class _AddServerScreenState extends ConsumerState<AddServerScreen> {
210 Future<void> _saveServer() async {
211 if (_formKey.currentState?.validate() != true) return;
212
213 + // Defensive mounted check before any state mutation. See #65.
214 + if (!mounted) return;
215 setState(() {
216 _isSaving = true;
217 });
test/features/issue_crash_fixes_test.dart
+151
@@ -439,6 +439,157 @@ void main() {
439 expect(tester.takeException(), isNull);
440 });
441 });
442 +
443 + group('Issue #65: Server Setup Screens Unmounted setState finally-guard', () {
444 + testWidgets(
445 + 'OnboardingServerSetupScreen does not crash when testConnection completes after unmount',
446 + (tester) async {
447 + SharedPreferences.setMockInitialValues({});
448 + final prefs = await SharedPreferences.getInstance();
449 +
450 + // Use a Completer so we control when the network response resolves —
451 + // this lets us unmount the widget before the await resumes, then
452 + // resume the future and verify the `finally` setState is a no-op.
453 + final connectionCompleter = Completer<ResponseBody>();
454 + final mockDio = Dio();
455 + mockDio.httpClientAdapter = _MockHttpClientAdapter((options) {
456 + return connectionCompleter.future;
457 + });
458 + final mockApiService = ServerApiService(mockDio);
459 +
460 + final container = ProviderContainer(
461 + overrides: [
462 + sharedPreferencesProvider.overrideWithValue(prefs),
463 + settingsProvider.overrideWith(_TestSettingsNotifier.new),
464 + chatParamsProvider.overrideWithValue(ChatParameters.defaults()),
465 + serverApiServiceProvider.overrideWithValue(mockApiService),
466 + ],
467 + );
468 + addTearDown(container.dispose);
469 +
470 + await tester.pumpWidget(
471 + UncontrolledProviderScope(
472 + container: container,
473 + child: ShadTheme(
474 + data: AppTheme.lightShadTheme,
475 + child: const MaterialApp(
476 + locale: Locale('en'),
477 + localizationsDelegates: AppLocalizations.localizationsDelegates,
478 + supportedLocales: AppLocalizations.supportedLocales,
479 + home: OnboardingServerSetupScreen(
480 + selectedType: ServerType.lmStudio,
481 + ),
482 + ),
483 + ),
484 + ),
485 + );
486 + await tester.pumpAndSettle();
487 +
488 + expect(find.text('Test Connection'), findsOneWidget);
489 +
490 + // Kick off the test connection (this enters the try block and awaits).
491 + await tester.tap(find.text('Test Connection'));
492 + await tester.pump();
493 +
494 + // Unmount the screen *while* the await is pending. This simulates a
495 + // user navigating away (or any other reason for the widget to be
496 + // disposed) before the network response arrives.
497 + await tester.pumpWidget(
498 + UncontrolledProviderScope(
499 + container: container,
500 + child: const MaterialApp(
501 + home: Scaffold(body: SizedBox()),
502 + ),
503 + ),
504 + );
505 + await tester.pump();
506 +
507 + // Now resume the future — this should run the remaining setState
508 + // calls in the try, catch, and finally blocks. Without the mounted
509 + // guards this would crash with
510 + // "Null check operator used on a null value" at
511 + // package:flutter/src/widgets/framework.dart:1219 from
512 + // `_element!.markNeedsBuild()` (issue #65).
513 + connectionCompleter.complete(
514 + ResponseBody.fromString('{"models":[]}', 200),
515 + );
516 + await tester.pumpAndSettle();
517 +
518 + expect(tester.takeException(), isNull);
519 + });
520 +
521 + testWidgets(
522 + 'OnboardingServerSetupScreen does not crash when testConnection fails after unmount',
523 + (tester) async {
524 + SharedPreferences.setMockInitialValues({});
525 + final prefs = await SharedPreferences.getInstance();
526 +
527 + // Completer that returns a 500 response — exercises the failure
528 + // branch (isConnected == false) + the `finally` block while the
529 + // widget is already unmounted.
530 + final connectionCompleter = Completer<ResponseBody>();
531 + final mockDio = Dio();
532 + mockDio.httpClientAdapter = _MockHttpClientAdapter((options) {
533 + return connectionCompleter.future;
534 + });
535 + final mockApiService = ServerApiService(mockDio);
536 +
537 + final container = ProviderContainer(
538 + overrides: [
539 + sharedPreferencesProvider.overrideWithValue(prefs),
540 + settingsProvider.overrideWith(_TestSettingsNotifier.new),
541 + chatParamsProvider.overrideWithValue(ChatParameters.defaults()),
542 + serverApiServiceProvider.overrideWithValue(mockApiService),
543 + ],
544 + );
545 + addTearDown(container.dispose);
546 +
547 + await tester.pumpWidget(
548 + UncontrolledProviderScope(
549 + container: container,
550 + child: ShadTheme(
551 + data: AppTheme.lightShadTheme,
552 + child: const MaterialApp(
553 + locale: Locale('en'),
554 + localizationsDelegates: AppLocalizations.localizationsDelegates,
555 + supportedLocales: AppLocalizations.supportedLocales,
556 + home: OnboardingServerSetupScreen(
557 + selectedType: ServerType.openAICompatible,
558 + ),
559 + ),
560 + ),
561 + ),
562 + );
563 + await tester.pumpAndSettle();
564 +
565 + await tester.tap(find.text('Test Connection'));
566 + await tester.pump();
567 +
568 + // Unmount before completing the in-flight request.
569 + await tester.pumpWidget(
570 + UncontrolledProviderScope(
571 + container: container,
572 + child: const MaterialApp(
573 + home: Scaffold(body: SizedBox()),
574 + ),
575 + ),
576 + );
577 + await tester.pump();
578 +
579 + // Complete with a 500 response so the testConnection method returns
580 + // false (no exception thrown). This drives both the try branch
581 + // (isConnected == false) and the finally branch (sets
582 + // _isTesting = false) on the now-disposed state. Without the
583 + // mounted guard, the finally setState would crash with
584 + // "Null check operator used on a null value" (#65).
585 + connectionCompleter.complete(
586 + ResponseBody.fromString('{"error":"server error"}', 500),
587 + );
588 + await tester.pumpAndSettle();
589 +
590 + expect(tester.takeException(), isNull);
591 + });
592 + });
593 }
594
595 class _StubActiveServerNotifier extends ActiveServerNotifier {