fix: WALLET-1364 — surface a failed export-keys window open - #1428
Open
ost-ptk wants to merge 1 commit into
Open
fix: WALLET-1364 — surface a failed export-keys window open#1428ost-ptk wants to merge 1 commit into
ost-ptk wants to merge 1 commit into
Conversation
Clicking "Download account keys" when windows.create rejects produced no window and no feedback whatsoever — the catch only wrote to the background console. The comment justifying that silence was wrong, so it is deleted rather than softened: it claimed there is no popup left to show the error in, but the dispatch comes from the popup navigation menu, whose handler closes the menu OVERLAY rather than the window, and the popup mounts SagaErrorBanner. When the window fails to open nothing takes focus, so the popup is still there to render the banner. The surfaced message is a static sentence — appEvents is broadcast to every UI replica and the banner renders source and message verbatim.
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
1 participant
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.
Ticket: https://make-software.atlassian.net/browse/WALLET-1364 (P1 item 3)
The bug
The user clicks "Download account keys" in the popup navigation menu, a saga opens a separate export window, and if
windows.createrejects the catch only writes to the background console. No window, no message, no feedback of any kind — the click simply does nothing.The comment sitting in that catch justified the silence and was factually wrong, which is how the gap survived review. It claimed "there is no popup left to show it in". Both halves of that were checked against the code and are false:
closeNavigationMenu()— that only navigates router state to hide the menu overlay, with no window API call at all;SagaErrorBannerunconditionally (src/apps/popup/index.tsx).When
windows.createrejects, no new window takes focus, so the popup is still open and perfectly able to render the error.The fix
yield put(sagaError({ source: 'openExportKeysWindowSaga', … }))alongside the existingconsole.error, and the incorrect comment is deleted rather than softened — the reasoning it stated was false, so leaving a softened version would preserve the misleading premise.The surfaced message is a short static sentence with no error text, no paths and no user data:
appEventsis broadcast to every UI replica and the banner renders{source}: {message}verbatim.Verification
npm run ci-checkpasses (406 tests). The new test discriminates: removing theput(sagaError(...))makes it fail while the other five in the file — including the early-return-on-existing-window case and the watcher test — keep passing.