From 603fe86b878b05903b99a173475ea6823a10f629 Mon Sep 17 00:00:00 2001 From: Jaemin Jo <44039707+91jaeminjo@users.noreply.github.com> Date: Mon, 22 Jun 2026 16:48:18 -0500 Subject: [PATCH] fix: treat delete-thread 404 as success to free the delete dialog Deleting a thread that no longer exists server-side returned a 404 (NotFoundException), which propagated into AsyncActionDialog and left it open with an error on every retry while the stale entry stayed in the sidebar. DELETE is idempotent, so a 404 means the desired end state is already reached: catch NotFoundException, log it, and fall through to the existing local-list cleanup. Other exceptions keep propagating so genuine failures still surface. Route ThreadListState logging through soliplex_logging instead of dart:developer, matching the lobby_state/room_screen migration. Closes #365 --- CHANGELOG.md | 7 ++ lib/src/modules/room/thread_list_state.dart | 31 +++++--- test/modules/room/thread_list_state_test.dart | 78 +++++++++++++++++++ 3 files changed, 104 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c58e3426..401166f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,13 @@ Versions follow the `version+build` scheme from `pubspec.yaml`, bumped via - Auth: the consent notice terms are now selectable, so users can copy the text they're agreeing to. +### Fixed + +- Room: deleting a thread that no longer exists server-side no longer traps the + user in the Delete dialog. A 404 is now treated as success (DELETE is + idempotent — the thread is already gone), so the dialog closes and the stale + entry is removed from the sidebar. Other failures still surface in the dialog. + ## [0.90.0+60] - 2026-06-16 ### Added diff --git a/lib/src/modules/room/thread_list_state.dart b/lib/src/modules/room/thread_list_state.dart index 3c32dab9..ce0c23d8 100644 --- a/lib/src/modules/room/thread_list_state.dart +++ b/lib/src/modules/room/thread_list_state.dart @@ -1,10 +1,13 @@ import 'dart:async'; -import 'dart:developer' as dev; import 'package:soliplex_agent/soliplex_agent.dart'; +import 'package:soliplex_logging/soliplex_logging.dart'; import '../auth/auth_session.dart'; +final Logger _logger = + LogManager.instance.getLogger('soliplex.thread_list_state'); + sealed class ThreadListStatus {} class ThreadsLoading extends ThreadListStatus {} @@ -101,6 +104,17 @@ class ThreadListState { if (_isDisposed) return; try { await _connection.api.deleteThread(_roomId, threadId); + } on NotFoundException catch (error) { + // DELETE is idempotent: a 404 means the thread is already gone + // server-side, so the desired end state is reached. Fall through to + // the local cleanup that drops the stale entry from the sidebar. + // A 404 is mapped purely by status code, so log the resource details + // in case the route (not the thread) is what's missing. + _logger.info( + 'deleteThread got 404; treating as already-deleted and dropping ' + 'stale entry', + error: error, + ); } on AuthException catch (error) { _funnelAuthException(error, op: 'deleteThread'); return; @@ -196,13 +210,11 @@ class ThreadListState { } on PermissionDeniedException catch (error) { if (token.isCancelled) return; _cancelToken = null; - dev.log( + _logger.warning( _threads.value is ThreadsLoaded ? 'Thread refresh forbidden (403), keeping stale list' : 'Thread fetch forbidden (403)', error: error, - name: 'ThreadListState', - level: 900, ); if (_threads.value is! ThreadsLoaded) { _threads.value = ThreadsFailed(error); @@ -210,11 +222,9 @@ class ThreadListState { } on AuthException catch (error) { if (token.isCancelled) return; _cancelToken = null; - dev.log( + _logger.warning( 'Thread fetch hit AuthException; funneling to markSessionExpired', error: error, - name: 'ThreadListState', - level: 900, ); _auth.markSessionExpired(); } on Object catch (error) { @@ -222,10 +232,9 @@ class ThreadListState { _cancelToken = null; // Preserve existing loaded threads on refresh failure. if (_threads.value is ThreadsLoaded) { - dev.log( + _logger.info( 'Thread refresh failed, keeping stale list', error: error, - name: 'ThreadListState', ); } else { _threads.value = ThreadsFailed(error); @@ -234,11 +243,9 @@ class ThreadListState { } void _funnelAuthException(AuthException error, {required String op}) { - dev.log( + _logger.warning( '$op hit AuthException; funneling to markSessionExpired', error: error, - name: 'ThreadListState', - level: 900, ); _auth.markSessionExpired(); } diff --git a/test/modules/room/thread_list_state_test.dart b/test/modules/room/thread_list_state_test.dart index 9dc53ac9..558f0cf8 100644 --- a/test/modules/room/thread_list_state_test.dart +++ b/test/modules/room/thread_list_state_test.dart @@ -188,6 +188,44 @@ void main() { state.dispose(); }); + test('deleteThread treats NotFoundException as success and removes thread', + () async { + api.nextThreads = [ + ThreadInfo( + id: 'thread-1', + roomId: 'room-1', + name: 'First', + createdAt: DateTime(2026, 1, 1), + ), + ThreadInfo( + id: 'thread-2', + roomId: 'room-1', + name: 'Second', + createdAt: DateTime(2026, 3, 1), + ), + ]; + + final state = ThreadListState( + connection: connection, + roomId: 'room-1', + auth: _authInActiveSession(), + ); + await Future.delayed(Duration.zero); + + // The thread is already gone server-side: DELETE 404s. + api.nextDeleteThreadError = const NotFoundException(message: 'gone'); + + // Idempotent delete must not throw... + await state.deleteThread('thread-1'); + + // ...and the stale thread must be removed from the local list. + final loaded = state.threads.value as ThreadsLoaded; + expect(loaded.threads.length, 1); + expect(loaded.threads.single.id, 'thread-2'); + + state.dispose(); + }); + test('renameThread updates name and preserves description', () async { api.nextThreads = [ ThreadInfo( @@ -511,6 +549,46 @@ void main() { state.dispose(); }); + test( + 'deleteThread NotFoundException from non-loaded state still reconciles ' + 'via a fresh fetch', () async { + // Initial fetch fails: state ends up in ThreadsFailed. + api.nextThreadsError = Exception('slow network'); + + final state = ThreadListState( + connection: connection, + roomId: 'room-1', + auth: _authInActiveSession(), + ); + await Future.delayed(Duration.zero); + expect(state.threads.value, isA()); + + // The thread is already gone server-side (404), and there's no loaded + // baseline to drop it from — the swallow must still fall through to the + // reconciling fetch rather than returning early. Clear the fetch error + // and seed the post-delete server state for that fetch. + api.nextDeleteThreadError = const NotFoundException(message: 'gone'); + api.nextThreadsError = null; + api.nextThreads = [ + ThreadInfo( + id: 'thread-other', + roomId: 'room-1', + name: 'Other', + createdAt: DateTime(2026, 2, 1), + ), + ]; + + await state.deleteThread('thread-1'); + // deleteThread scheduled a fresh fetch; let it resolve. + await Future.delayed(Duration.zero); + + expect(api.deleteThreadCallCount, 1); + final loaded = state.threads.value as ThreadsLoaded; + expect(loaded.threads.single.id, 'thread-other'); + + state.dispose(); + }); + group('noteSpawnedThread', () { test('inserts thread into loaded list sorted by createdAt descending', () async {