Skip to content

refactor(legacy_seed_view): sanitize user-facing error messages - #2549

Draft
wired-pasteque wants to merge 3 commits into
developfrom
refactor-errors-legacy-seed-view
Draft

refactor(legacy_seed_view): sanitize user-facing error messages#2549
wired-pasteque wants to merge 3 commits into
developfrom
refactor-errors-legacy-seed-view

Conversation

@wired-pasteque

Copy link
Copy Markdown
Collaborator

No description provided.

@wired-pasteque wired-pasteque self-assigned this Jul 31, 2026
@claude

This comment was marked as outdated.

@wired-pasteque

Copy link
Copy Markdown
Collaborator Author

@claude, check the fixes, regarding finding 2: out of scope - one feature per PR

@claude

claude Bot commented Aug 3, 2026

Copy link
Copy Markdown

Claude finished @wired-pasteque's task in 2m 18s —— View job


Fix verification — PR #2549

  • Read the two fix commits (d28907dc2, 5433ab907)
  • Verify finding 1: Error subtypes no longer strand the UI on a spinner
  • Verify minor finding: refetch loop on empty-seeds success
  • Acknowledge finding 2 as out of scope
  • Post verdict

Both fixes are correct — nothing further needed. ✅

Finding 1 (Error throwables → infinite spinner): fixed. get_legacy_seeds_usecase.dart:32 now ends with a bare catch (e, st), so TypeError/HiveError from corrupt legacy data become LegacySeedViewUnexpectedFailure instead of an unhandled rejection. The catch clause keeps the sanitization contract (logs only e.runtimeType, never the payload), and the new test throws ArgumentError (an Error, not Exception) and asserts both the failure mapping and that no raw message is retained — exactly the regression case. I also confirmed both log calls match the Logger API (warning takes a positional message, severe a named one).

Minor finding (refetch loop on empty success): fixed. The fetch trigger moved out of BlocBuilder.builder into initState (legacy_seed_view_screen.dart:17-24), so an empty-seeds success no longer re-fires on every rebuild. Details checked: the cubit is provided by BlocProvider above the screen (settings_router.dart:324), so context.read in initState resolves; and fetchOldSeeds() emits loading: true synchronously before its first await, so the first frame shows the spinner rather than a flash of the "no seeds" message.

Finding 2 (core GetOldSeedsUsecase logs the raw exception): acknowledged as out of scope per your call — one feature per PR. Worth a follow-up ticket since the raw-e log in the core usecase is the remaining path where key-material-bearing exception text reaches the TSV log and Sentry, but nothing to do here.

One optional nit, pre-existing and not from these fixes: if the user pops the screen while a fetch is in flight, the cubit is closed and the pending emit in fetchOldSeeds will throw a StateError (emit-after-close). A guard like if (isClosed) return; before the post-await emits would silence it, but it's cosmetic and fine to leave.

Note: Flutter isn't installed in this runner, so I verified statically (diffs, the Logger/Result APIs, provider wiring) rather than running the test suite.

--- | Branch

@ethicnology

Copy link
Copy Markdown
Member

Conflict management required plz @wired-pasteque

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants