@tduncan / Cortana / commits / 6a01540

fix: add mounted guards to connection tests and safe form validation in server setup (#64)

- Guard _testConnection and _saveAndContinue against calling setState when unmounted in OnboardingServerSetupScreen and AddServerScreen. - Use null-safe form validation (?.validate() != true) across onboarding, add server, persona creation, and cloud sync screens. - Use safe port parsing with default port fallbacks in _buildServer. - Add regression tests in test/features/issue_crash_fixes_test.dart. Closes #64

abdulmominsakib committed Aug 16, 2026 at 18:20 UTC 6a015406306d8bd49e45688c8b289dad3d9ed2ac
5 files changed +217 -54
lib/features/cloud_sync/views/cloud_sync_screen.dart
+1 -1
@@ -76,7 +76,7 @@ class _CloudSyncScreenState extends ConsumerState<CloudSyncScreen> {
76 );
77
78 Future<void> _run(Future<void> Function() action) async {
79 - if (!_formKey.currentState!.validate()) return;
79 + if (_formKey.currentState?.validate() != true) return;
80 setState(() => _working = true);
81 try {
82 await action();
lib/features/onboarding/screens/onboarding_server_setup_screen.dart
+30 -17
@@ -98,9 +98,17 @@ class _OnboardingServerSetupScreenState
98 final host = requiresCloudConfig
99 ? AppConstants.ollamaCloudBaseUrl
100 : _hostController.text.trim();
101 + final defaultPort = switch (widget.selectedType) {
102 + ServerType.lmStudio => AppConstants.lmStudioDefaultPort,
103 + ServerType.openAICompatible => AppConstants.openAICompatibleDefaultPort,
104 + ServerType.ollama => AppConstants.ollamaDefaultPort,
105 + ServerType.ollamaCloud => AppConstants.ollamaCloudDefaultPort,
106 + ServerType.openRouter => 443,
107 + ServerType.onDevice => 0,
108 + };
109 final port = requiresCloudConfig
110 ? AppConstants.ollamaCloudDefaultPort
103 - : int.parse(_portController.text.trim());
111 + : (int.tryParse(_portController.text.trim()) ?? defaultPort);
112
113 return Server(
114 id: DateTime.now().millisecondsSinceEpoch.toString(),
@@ -126,31 +134,31 @@ class _OnboardingServerSetupScreenState
134
135 String? _validateHost(String? value) {
136 if (_isCloudProvider) return null;
129 - final l10n = AppLocalizations.of(context)!;
137 + final l10n = AppLocalizations.of(context);
138 if (value == null || value.trim().isEmpty) {
131 - return l10n.host_required;
139 + return l10n?.host_required ?? 'Host is required';
140 }
141 if (parseServerAddressInput(value) == null) {
134 - return l10n.host_valid;
142 + return l10n?.host_valid ?? 'Enter a valid host';
143 }
144 return null;
145 }
146
147 String? _validatePort(String? value) {
148 if (_isCloudProvider) return null;
141 - final l10n = AppLocalizations.of(context)!;
149 + final l10n = AppLocalizations.of(context);
150 if (value == null || value.trim().isEmpty) {
143 - return l10n.port_required;
151 + return l10n?.port_required ?? 'Port is required';
152 }
153 final port = int.tryParse(value.trim());
154 if (port == null || port < 1 || port > 65535) {
147 - return l10n.port_invalid;
155 + return l10n?.port_invalid ?? 'Enter a valid port number (1-65535)';
156 }
157 return null;
158 }
159
160 Future<void> _testConnection() async {
153 - if (!_formKey.currentState!.validate()) return;
161 + if (_formKey.currentState?.validate() != true) return;
162
163 setState(() {
164 _isTesting = true;
@@ -158,35 +166,39 @@ class _OnboardingServerSetupScreenState
166 _testSuccess = false;
167 });
168
161 - final l10n = AppLocalizations.of(context)!;
169 + final l10n = AppLocalizations.of(context);
170 final apiService = ref.read(serverApiServiceProvider);
171 final testServer = _buildServer();
172
173 try {
174 final isConnected = await apiService.testConnection(testServer);
175 + if (!mounted) return;
176 setState(() {
177 _testSuccess = isConnected;
178 _testResult = isConnected
170 - ? l10n.connection_successful
171 - : l10n.connection_failed;
179 + ? (l10n?.connection_successful ?? 'Connection successful')
180 + : (l10n?.connection_failed ?? 'Connection failed');
181 });
182 if (isConnected) {
183 invalidateAvailableModelsCache(testServer.id);
184 }
185 } catch (e) {
186 + if (!mounted) return;
187 setState(() {
188 _testSuccess = false;
179 - _testResult = l10n.error_with_message(e.toString());
189 + _testResult = l10n?.error_with_message(e.toString()) ?? 'Error: $e';
190 });
191 } finally {
182 - setState(() {
183 - _isTesting = false;
184 - });
192 + if (mounted) {
193 + setState(() {
194 + _isTesting = false;
195 + });
196 + }
197 }
198 }
199
200 Future<void> _saveAndContinue() async {
189 - if (!_formKey.currentState!.validate()) return;
201 + if (_formKey.currentState?.validate() != true) return;
202
203 setState(() {
204 _isSaving = true;
@@ -207,7 +219,8 @@ class _OnboardingServerSetupScreenState
219 ScaffoldMessenger.of(context).showSnackBar(
220 SnackBar(
221 content: Text(
210 - AppLocalizations.of(context)!.error_with_message(e.toString()),
222 + AppLocalizations.of(context)?.error_with_message(e.toString()) ??
223 + 'Error: $e',
224 ),
225 backgroundColor: Colors.red,
226 ),
lib/features/personas/views/create_persona_screen.dart
+1 -1
@@ -99,7 +99,7 @@ class _CreatePersonaScreenState extends ConsumerState<CreatePersonaScreen> {
99 }
100
101 Future<void> _save() async {
102 - if (!_formKey.currentState!.validate()) return;
102 + if (_formKey.currentState?.validate() != true) return;
103
104 setState(() => _isSaving = true);
105
lib/features/servers/views/add_server_screen.dart
+33 -18
@@ -115,7 +115,7 @@ class _AddServerScreenState extends ConsumerState<AddServerScreen> {
115 }
116
117 Future<void> _testConnection() async {
118 - if (!_formKey.currentState!.validate()) return;
118 + if (_formKey.currentState?.validate() != true) return;
119
120 setState(() {
121 _isTesting = true;
@@ -128,25 +128,29 @@ class _AddServerScreenState extends ConsumerState<AddServerScreen> {
128
129 try {
130 final isConnected = await apiService.testConnection(testServer);
131 + if (!mounted) return;
132 setState(() {
133 _testResult = isConnected
133 - ? AppLocalizations.of(context)!.connection_successful
134 - : AppLocalizations.of(context)!.connection_failed;
134 + ? AppLocalizations.of(context)?.connection_successful ?? 'Connection successful'
135 + : AppLocalizations.of(context)?.connection_failed ?? 'Connection failed';
136 });
137 if (isConnected) {
138 invalidateAvailableModelsCache(testServer.id);
139 }
140 } catch (e) {
141 + if (!mounted) return;
142 setState(() {
143 _testResult = AppLocalizations.of(
144 context,
143 - )!.error_with_message(e.toString());
145 + )?.error_with_message(e.toString()) ?? 'Error: $e';
146 });
147 } finally {
146 - setState(() {
147 - _isTesting = false;
148 - });
149 - _scrollToBottom();
148 + if (mounted) {
149 + setState(() {
150 + _isTesting = false;
151 + });
152 + _scrollToBottom();
153 + }
154 }
155 }
156
@@ -162,9 +166,17 @@ class _AddServerScreenState extends ConsumerState<AddServerScreen> {
166 final host = _selectedType == ServerType.ollamaCloud
167 ? AppConstants.ollamaCloudBaseUrl
168 : rawHost;
169 + final defaultPort = switch (_selectedType) {
170 + ServerType.lmStudio => AppConstants.lmStudioDefaultPort,
171 + ServerType.openAICompatible => AppConstants.openAICompatibleDefaultPort,
172 + ServerType.ollama => AppConstants.ollamaDefaultPort,
173 + ServerType.ollamaCloud => AppConstants.ollamaCloudDefaultPort,
174 + ServerType.openRouter => 443,
175 + ServerType.onDevice => 0,
176 + };
177 final port = _selectedType == ServerType.ollamaCloud
178 ? AppConstants.ollamaCloudDefaultPort
167 - : int.parse(rawPort);
179 + : (int.tryParse(rawPort) ?? defaultPort);
180
181 return Server(
182 id:
@@ -194,7 +206,7 @@ class _AddServerScreenState extends ConsumerState<AddServerScreen> {
206 }
207
208 Future<void> _saveServer() async {
197 - if (!_formKey.currentState!.validate()) return;
209 + if (_formKey.currentState?.validate() != true) return;
210
211 setState(() {
212 _isSaving = true;
@@ -215,21 +227,24 @@ class _AddServerScreenState extends ConsumerState<AddServerScreen> {
227 invalidateAvailableModelsCache(server.id);
228
229 if (mounted) {
218 - final l10n = AppLocalizations.of(context)!;
230 + final l10n = AppLocalizations.of(context);
231 context.pop();
220 - ScaffoldMessenger.of(context).showSnackBar(
221 - SnackBar(
222 - content: Text(_isEditing ? l10n.server_updated : l10n.server_added),
223 - duration: const Duration(seconds: 2),
224 - ),
225 - );
232 + if (l10n != null) {
233 + ScaffoldMessenger.of(context).showSnackBar(
234 + SnackBar(
235 + content: Text(_isEditing ? l10n.server_updated : l10n.server_added),
236 + duration: const Duration(seconds: 2),
237 + ),
238 + );
239 + }
240 }
241 } catch (e) {
242 if (mounted) {
243 ScaffoldMessenger.of(context).showSnackBar(
244 SnackBar(
245 content: Text(
232 - AppLocalizations.of(context)!.error_with_message(e.toString()),
246 + AppLocalizations.of(context)?.error_with_message(e.toString()) ??
247 + 'Error: $e',
248 ),
249 backgroundColor: Colors.red,
250 ),
test/features/issue_crash_fixes_test.dart
+152 -17
@@ -21,6 +21,8 @@ import 'package:localmind/features/models/data/models/model_info.dart';
21 import 'package:localmind/features/servers/data/models/server.dart';
22 import 'package:localmind/features/servers/data/server_api_service.dart';
23 import 'package:localmind/features/servers/providers/server_providers.dart';
24 +import 'package:localmind/features/onboarding/screens/onboarding_server_setup_screen.dart';
25 +import 'package:localmind/features/servers/views/add_server_screen.dart';
26 import 'package:localmind/features/settings/data/models/app_settings.dart';
27 import 'package:localmind/l10n/app_localizations.dart';
28 import 'package:shadcn_ui/shadcn_ui.dart';
@@ -40,23 +42,24 @@ void main() {
42 if (tempFile.existsSync()) tempFile.deleteSync();
43 });
44
43 - final isPreview = true;
44 - final source = AudioSource.file(
45 - tempFile.path,
46 - tag: MediaItem(
47 - id: 'tts_chunk_test_0',
48 - album: isPreview ? 'Voice Preview' : 'LocalMind TTS',
49 - title: 'Hello world',
50 - artist: 'Piper TTS',
51 - ),
52 - );
45 + for (final isPreview in [true, false]) {
46 + final source = AudioSource.file(
47 + tempFile.path,
48 + tag: MediaItem(
49 + id: 'tts_chunk_test_$isPreview',
50 + album: isPreview ? 'Voice Preview' : 'LocalMind TTS',
51 + title: 'Hello world',
52 + artist: 'Piper TTS',
53 + ),
54 + );
55
54 - // Verify tag is non-null and is a MediaItem (avoiding "type 'Null' is not a subtype of type 'MediaItem'")
55 - expect(source.tag, isNotNull);
56 - expect(source.tag, isA<MediaItem>());
57 - final mediaItem = source.tag as MediaItem;
58 - expect(mediaItem.album, 'Voice Preview');
59 - expect(mediaItem.title, 'Hello world');
56 + // Verify tag is non-null and is a MediaItem (avoiding "type 'Null' is not a subtype of type 'MediaItem'")
57 + expect(source.tag, isNotNull);
58 + expect(source.tag, isA<MediaItem>());
59 + final mediaItem = source.tag as MediaItem;
60 + expect(mediaItem.album, isPreview ? 'Voice Preview' : 'LocalMind TTS');
61 + expect(mediaItem.title, 'Hello world');
62 + }
63 });
64 });
65
@@ -304,6 +307,138 @@ void main() {
307 expect(tester.takeException(), isNull);
308 });
309 });
310 +
311 + group('Issue #64: Server Setup Screens Unmounted testConnection Guard', () {
312 + testWidgets('OnboardingServerSetupScreen does not crash when unmounted during testConnection', (tester) async {
313 + SharedPreferences.setMockInitialValues({});
314 + final prefs = await SharedPreferences.getInstance();
315 +
316 + final connectionCompleter = Completer<ResponseBody>();
317 + final mockDio = Dio();
318 + mockDio.httpClientAdapter = _MockHttpClientAdapter((options) {
319 + return connectionCompleter.future;
320 + });
321 + final mockApiService = ServerApiService(mockDio);
322 +
323 + final container = ProviderContainer(
324 + overrides: [
325 + sharedPreferencesProvider.overrideWithValue(prefs),
326 + settingsProvider.overrideWith(_TestSettingsNotifier.new),
327 + chatParamsProvider.overrideWithValue(ChatParameters.defaults()),
328 + serverApiServiceProvider.overrideWithValue(mockApiService),
329 + ],
330 + );
331 + addTearDown(container.dispose);
332 +
333 + await tester.pumpWidget(
334 + UncontrolledProviderScope(
335 + container: container,
336 + child: ShadTheme(
337 + data: AppTheme.lightShadTheme,
338 + child: const MaterialApp(
339 + locale: Locale('en'),
340 + localizationsDelegates: AppLocalizations.localizationsDelegates,
341 + supportedLocales: AppLocalizations.supportedLocales,
342 + home: OnboardingServerSetupScreen(
343 + selectedType: ServerType.lmStudio,
344 + ),
345 + ),
346 + ),
347 + ),
348 + );
349 + await tester.pumpAndSettle();
350 +
351 + expect(find.text('Test Connection'), findsOneWidget);
352 +
353 + // Tap Test Connection
354 + await tester.tap(find.text('Test Connection'));
355 + await tester.pump();
356 +
357 + // Immediately unmount OnboardingServerSetupScreen while test is in flight
358 + await tester.pumpWidget(
359 + UncontrolledProviderScope(
360 + container: container,
361 + child: const MaterialApp(
362 + home: Scaffold(body: SizedBox()),
363 + ),
364 + ),
365 + );
366 + await tester.pump();
367 +
368 + // Now complete the network response
369 + connectionCompleter.complete(
370 + ResponseBody.fromString('{"models":[]}', 200),
371 + );
372 + await tester.pumpAndSettle();
373 +
374 + // Ensure no Null check operator or setState exception was thrown
375 + expect(tester.takeException(), isNull);
376 + });
377 +
378 + testWidgets('AddServerScreen does not crash when unmounted during testConnection', (tester) async {
379 + SharedPreferences.setMockInitialValues({});
380 + final prefs = await SharedPreferences.getInstance();
381 +
382 + final connectionCompleter = Completer<ResponseBody>();
383 + final mockDio = Dio();
384 + mockDio.httpClientAdapter = _MockHttpClientAdapter((options) {
385 + return connectionCompleter.future;
386 + });
387 + final mockApiService = ServerApiService(mockDio);
388 +
389 + final container = ProviderContainer(
390 + overrides: [
391 + sharedPreferencesProvider.overrideWithValue(prefs),
392 + settingsProvider.overrideWith(_TestSettingsNotifier.new),
393 + chatParamsProvider.overrideWithValue(ChatParameters.defaults()),
394 + serverApiServiceProvider.overrideWithValue(mockApiService),
395 + ],
396 + );
397 + addTearDown(container.dispose);
398 +
399 + await tester.pumpWidget(
400 + UncontrolledProviderScope(
401 + container: container,
402 + child: ShadTheme(
403 + data: AppTheme.lightShadTheme,
404 + child: const MaterialApp(
405 + locale: Locale('en'),
406 + localizationsDelegates: AppLocalizations.localizationsDelegates,
407 + supportedLocales: AppLocalizations.supportedLocales,
408 + home: AddServerScreen(),
409 + ),
410 + ),
411 + ),
412 + );
413 + await tester.pumpAndSettle();
414 +
415 + expect(find.text('Test Connection'), findsOneWidget);
416 +
417 + // Tap Test Connection
418 + await tester.tap(find.text('Test Connection'));
419 + await tester.pump();
420 +
421 + // Immediately unmount AddServerScreen while test is in flight
422 + await tester.pumpWidget(
423 + UncontrolledProviderScope(
424 + container: container,
425 + child: const MaterialApp(
426 + home: Scaffold(body: SizedBox()),
427 + ),
428 + ),
429 + );
430 + await tester.pump();
431 +
432 + // Now complete the network response
433 + connectionCompleter.complete(
434 + ResponseBody.fromString('{"models":[]}', 200),
435 + );
436 + await tester.pumpAndSettle();
437 +
438 + // Ensure no Null check operator or setState exception was thrown
439 + expect(tester.takeException(), isNull);
440 + });
441 + });
442 }
443
444 class _StubActiveServerNotifier extends ActiveServerNotifier {
@@ -330,7 +465,7 @@ class _StubConnectedStatusNotifier extends ConnectionStatusNotifier {
465 }
466
467 class _MockHttpClientAdapter implements HttpClientAdapter {
333 - final ResponseBody Function(RequestOptions options) handler;
468 + final FutureOr<ResponseBody> Function(RequestOptions options) handler;
469
470 _MockHttpClientAdapter(this.handler);
471