[GH-ISSUE #7659] Add consistent error handling to all four provider reset handlers in CreateAccountModal #52269

Open
opened 2026-04-30 20:32:25 -05:00 by GiteaMirror · 0 comments
Owner

Originally created by @coderabbitai[bot] on GitHub (Apr 29, 2026).
Original GitHub issue: https://github.com/actualbudget/actual/issues/7659

Summary

The four bank-sync provider reset handlers in packages/desktop-client/src/components/modals/CreateAccountModal.tsxonGoCardlessReset, onSimpleFinReset, onPluggyAiReset, and onEnableBankingReset — all chain secret-set IPC calls without try/catch and are invoked via void, meaning any rejection is silently dropped with no user feedback and an ambiguous UI state.

Steps to Reproduce / Context

Flagged during review of PR #7345 (Enable Banking integration). The Enable Banking handler (onEnableBankingReset) uses await Promise.all([...]) so it can reject, while the GoCardless/SimpleFIN/PluggyAI handlers chain void send(...).then(...) — all four share the same missing-error-handling issue.

Proposed Fix

Wrap each reset handler in a try/catch, only update setup state on success, and surface an error notification (e.g. via addNotification) on failure so the user knows the reset did not complete.

References

Originally created by @coderabbitai[bot] on GitHub (Apr 29, 2026). Original GitHub issue: https://github.com/actualbudget/actual/issues/7659 ## Summary The four bank-sync provider reset handlers in `packages/desktop-client/src/components/modals/CreateAccountModal.tsx` — `onGoCardlessReset`, `onSimpleFinReset`, `onPluggyAiReset`, and `onEnableBankingReset` — all chain `secret-set` IPC calls without `try/catch` and are invoked via `void`, meaning any rejection is silently dropped with no user feedback and an ambiguous UI state. ## Steps to Reproduce / Context Flagged during review of PR #7345 (Enable Banking integration). The Enable Banking handler (`onEnableBankingReset`) uses `await Promise.all([...])` so it can reject, while the GoCardless/SimpleFIN/PluggyAI handlers chain `void send(...).then(...)` — all four share the same missing-error-handling issue. ## Proposed Fix Wrap each reset handler in a `try/catch`, only update setup state on success, and surface an error notification (e.g. via `addNotification`) on failure so the user knows the reset did not complete. ## References - PR: https://github.com/actualbudget/actual/pull/7345 - Review comment: https://github.com/actualbudget/actual/pull/7345#discussion_r3159466250 - Requested by: @AurelDemiri
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/actual#52269