Skip to content

Improve async error handling#109

Open
callebtc wants to merge 2 commits intomainfrom
issue-91-async-error-handling
Open

Improve async error handling#109
callebtc wants to merge 2 commits intomainfrom
issue-91-async-error-handling

Conversation

@callebtc
Copy link
Collaborator

@callebtc callebtc commented Nov 29, 2025

Summary

  • adds defensive try/catch blocks to wallet init/rebuild so failures are surfaced via a new WalletErrorListener
  • ModernPOSActivity subscribes to wallet errors and surfaces them in-app
  • MintsSettingsActivity gets consistent async error handling (load, refresh, add mint, stale refresh) with new localized strings
  • RestoreWalletActivity now handles Nostr backup fetch and wallet restore errors with localized messages and logging
  • LightningMintHandler surfaces WebSocket/polling failures via status updates and guards mint() with try/catch
  • BalanceCheckActivity already provided user-facing errors; error handling remains intact

Testing

  • ./gradlew assembleDebug

@callebtc
Copy link
Collaborator Author

maybe ACK

@callebtc
Copy link
Collaborator Author

callebtc commented Dec 1, 2025

Review (goose)

Effectiveness: 6/10

  • Good start surfacing errors from wallet init/restore and guarding some Lightning flows
  • Missing coverage for existing wallet instances created before the listener is attached
  • Mint list still swallows balance/load errors – new strings are never used

Code Style: 5/10

  • Listener is registered inside onResume so concurrent screens would clobber each other; needs lifecycle-safe owner
  • No multi-listener support; setErrorListener replaces previous observers silently
  • Error strings added but not wired up; several try/catch blocks only log but don’t surface messages

Safety & Robustness: 4/10

  • CashuWalletManager can emit errors before ModernPOSActivity registers the listener → user never sees them
  • setErrorListener is global/static and not thread-safe; other activities can null it unexpectedly
  • LightningMintHandler status updates are hard-coded English strings, bypassing localization & UX patterns

Suggestions:

  1. Move wallet error reporting to Application scope (e.g., shared Flow/LiveData) so multiple screens can observe without stomping each other, and register before init runs.
  2. Actually surface the new strings when loading mints/balances fails (currently they’re never shown) and keep the UI responsive.
  3. Localize Lightning status messages and avoid spamming toasts; consider in-view status chips like the rest of the POS UI.
  4. Consider wrapping async calls (mint balances, mint info fetch) in a helper that consolidates logging + user-facing errors with retry affordances.

Given the remaining edge cases and race conditions, I’d hold merging until these issues are addressed.

@ye0man ye0man added this to Numo Jan 15, 2026
@github-project-automation github-project-automation bot moved this to Backlog in Numo Jan 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

1 participant