fix(demo): catch unhandled wallet connection rejections and surface error in UI - #118
Conversation
| try { | ||
| setWallet(await connectWallet()); | ||
| const state = await connectWallet(); | ||
| setWallet(state); |
There was a problem hiding this comment.
This catch never fires for the case it's meant to handle. connectWallet() in lib/wallet.ts already returns { status: "disconnected" } when requestAccess() reports access.error (the user rejects the Freighter popup), it never throws for that. So a real rejection just resolves normally, error stays null, and the UI reverts to the plain button with no explanation, the exact silent failure #94 describes. The fix needs to happen in connectWallet()/resolveConnectedState() itself: surface the error result instead of collapsing it to disconnected, or have this hook check wallet.status after the await rather than relying on a throw that never happens.
| <button className="wallet-button" onClick={connect} disabled={connecting}> | ||
| {connecting ? "Connecting..." : "Connect wallet"} | ||
| </button> | ||
| <div className="wallet-button-container"> |
There was a problem hiding this comment.
wallet-button-container and wallet-error (line 34) aren't defined anywhere in App.css or index.css, so once error is actually set, it renders with default browser styling and undefined layout relative to the button.
|
Updated! connectWallet and resolveConnectedState now throw on wallet error/rejection so useWallet properly catches and sets the error message. Also added the missing .wallet-button-container and .wallet-error styles to App.css |
collinsezedike
left a comment
collinsezedike
left a comment
There was a problem hiding this comment.
Both original findings are fixed correctly, connectWallet() now throws on rejection and the catch actually fires, and the CSS classes are defined. One new thing this fix introduces.
| const [connecting, setConnecting] = useState(false); | ||
| const [error, setError] = useState<string | null>(null); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
detectWallet() is called here with no .catch(), but resolveConnectedState() (which detectWallet() calls internally) now throws instead of returning a disconnected state, per this PR's fix in lib/wallet.ts. A user with a wallet connected and allowed, but whose address retrieval fails for any reason, previously saw this resolve gracefully to { status: "disconnected" }. Now it throws, and since there's no catch here, that becomes an unhandled promise rejection on page load instead. Please add a .catch() here too, same pattern as connect()'s try/catch, or have detectWallet() itself catch and fall back to disconnected internally.
|
Updated! Handled the fallback in both |
collinsezedike
left a comment
collinsezedike
left a comment
There was a problem hiding this comment.
The wallet-connect fix itself is sound, but this PR also adds a stray file at the repo root with a garbled, non-ASCII filename (a fragment of an unrelated commit message plus a private-use Unicode character). Its content is raw terminal output, ANSI escape codes included, from a different diff about MAX_ANTI_SNIPE_HARD_MAX_SECS/DEPLOYMENT_V2.md. This looks like an accidental paste-as-file and is unrelated to this PR's wallet fix. Please delete it.
|
Cleaned up! Removed the stray accidental file from the repo root and dropped the unneeded |
collinsezedike
left a comment
collinsezedike
left a comment
There was a problem hiding this comment.
@Santia2004 thank you for the contribution. This looks good, there is nothing to flag beyond a couple of trivial nits inline.
| </div> | ||
| ); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
Nit: missing trailing newline at end of file.
| return { wallet, connecting, connect }; | ||
| } | ||
| return { wallet, connecting, error, connect }; | ||
| } No newline at end of file |
There was a problem hiding this comment.
Nit: missing trailing newline at end of file.
| export function shortenAddress(address: string): string { | ||
| return `${address.slice(0, 4)}...${address.slice(-4)}`; | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
Nit: missing trailing newline at end of file.
2 participants
Closes #94
Changes
connectWallet()call insideuseWallet.tswith atry/catchblock to handle rejected promises.errorstate fromuseWallet.WalletButton.tsxwhen connection fails.