fix: treat delete-thread 404 as success to free the delete dialog#375
Merged
Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #365 — the Delete Thread dialog trapped the user when deleting a thread that no longer existed server-side. The DELETE returned 404 (
NotFoundException), which propagated up throughRoomState.deleteThreadintoAsyncActionDialog, leaving the dialog open with red error text. Every retry repeated the same 404, and the stale entry was never removed from the sidebar.HTTP DELETE is idempotent: a 404 means the desired end state (thread gone server-side) is already true.
ThreadListState.deleteThreadnow catchesNotFoundExceptionand falls through to the existing local-list cleanup, so the dialog closes and the stale entry is dropped. All other exceptions (network, 500,AuthException) keep their existing behavior, so genuine failures still surface in the dialog.Changes
thread_list_state.dart): catchNotFoundExceptionindeleteThread, log it, and fall through to local cleanup.ThreadListStatelogging throughsoliplex_logging(soliplex.thread_list_state) instead ofdart:developer, matching thelobby_state/room_screenmigration. The twolevel: 900paths map to_logger.warning; the benign "keeping stale list" and new 404 paths map to_logger.info.Fixedentry under[Unreleased].Reviewed
Reviewed with Gemini and the PR-review agents (code, tests, errors, comments). Acted on two findings: added the observability log (was the only catch branch in the file without one) and the non-Loaded coverage test.
Test plan
flutter test test/modules/room/thread_list_state_test.dart— green (incl. existingdeleteThread propagates API error, which confirms non-404 errors still bubble)flutter analyze— zero warningsmarkdownlint-cli2on CHANGELOG — clean🤖 Generated with Claude Code