Skip to content

refactor(status_check): sanitize user-facing error messages - #2501

Draft
wired-pasteque wants to merge 5 commits into
developfrom
refactor-errors-status-check
Draft

refactor(status_check): sanitize user-facing error messages#2501
wired-pasteque wants to merge 5 commits into
developfrom
refactor-errors-status-check

Conversation

@wired-pasteque

Copy link
Copy Markdown
Collaborator

No description provided.

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

This comment was marked as outdated.

@wired-pasteque

Copy link
Copy Markdown
Collaborator Author

@claude, review the fixes

@claude

claude Bot commented Jul 23, 2026

Copy link
Copy Markdown

Claude finished @wired-pasteque's task in 1m 43s —— View job


Re-review — fixes for both prior findings

  • Read the fix commits (0d87fd999, 288dd36da) against the prior review findings
  • Verify finding 1: narrowed on NoWalletsFoundException scope
  • Verify finding 2: stale-results indication on failed refresh
  • Check new l10n key coverage across all locales and test updates
  • Post review summary

Both findings are fixed correctly. No new issues — LGTM.

Finding 1 (misattributed/silently-swallowed NoWalletsFoundException) — fixed. The catch is now a nested try wrapping only the _getWalletsUsecase.execute() call (lib/features/status_check/domain/check_service_status_usecase.dart:33-37), so a NoWalletsFoundException escaping _checkAllServiceStatusUsecase.execute() falls through to the generic catch-all, which logs via log.severe and returns StatusCheckUnexpectedFailure. The new test at test/features/status_check/domain/check_service_status_usecase_test.dart:118-139 covers exactly this misattribution scenario, and the mock setup matches the real NoWalletsFoundException(super.message) constructor.

Finding 2 (stale results shown under the error banner) — fixed. lib/features/status_check/service_status_page.dart:94-107 now renders a "Results may be outdated" line (warning color, small font, next to the last-checked timestamp) when state.failure != null && serviceStatus.lastChecked != null. The condition is right: it only appears when there are previous results to be stale about — a failure on the very first check (no lastChecked) shows just the error banner. context.appColors.warning exists in both light and dark themes (lib/core/themes/colors.dart:143,187).

L10n coverage. statusCheckResultsMayBeOutdated is present in all 27 locale files (verified with an inverse grep — zero files missing the key), with @-metadata in app_en.arb. Generated l10n output is gitignored, so no stale-codegen risk.

CI note. The "Analyze and Test" workflow for the fix commits is still in progress at review time; I couldn't run flutter analyze/tests in this environment, so compilation confirmation comes from that run. The code reads correct by inspection.


@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