diff --git a/.changeset/quiet-wallet-recovery.md b/.changeset/quiet-wallet-recovery.md new file mode 100644 index 00000000000..3877c549d70 --- /dev/null +++ b/.changeset/quiet-wallet-recovery.md @@ -0,0 +1,5 @@ +--- +"@clerk/mosaic": patch +--- + +Show a clear cancellation message for rejected wallet requests in UserProfile, and explain when a request is still pending in the wallet. Ignore late wallet responses after closing the profile so reopening starts a fresh connection flow. diff --git a/packages/mosaic/AGENTS.md b/packages/mosaic/AGENTS.md index 04fbe3de9e7..268b691f002 100644 --- a/packages/mosaic/AGENTS.md +++ b/packages/mosaic/AGENTS.md @@ -66,7 +66,7 @@ A flow lives in `src/features//`. The **model** (`*.model.ts(x)`) is th Most problems a feature hits already have a shared answer. Find the line for your problem and use it. Do not write a local version, even a small one. A PR that adds, renames or changes the signature of a shared helper in `src/hooks/`, `src/utils/`, `src/blocks/`, `src/components/form/` or `src/machine/` updates this list, and any doc that names it, in the same PR. -- An async action from a button or row: pending, an error, one lock → `usePendingAction` (`src/hooks/`) +- An async action from a button or row: pending, an error, one lock, unmount cancellation → `usePendingAction` (`src/hooks/`) - A form: values, validation, submit, field errors, success UI → `useForm` (`src/components/form/`), with `validateAsync` for server checks and `onSuccess` for mounted success UI - A `
` element, without the browser's validation popup → `Form` (`src/primitives/form/`) - A model save whose Clerk errors should land on form fields → `save` (`src/utils/errors.ts`) in the model, or reject with `FormSubmitError` diff --git a/packages/mosaic/docs/controllers.md b/packages/mosaic/docs/controllers.md index d8ccf117e9d..ec0609b347b 100644 --- a/packages/mosaic/docs/controllers.md +++ b/packages/mosaic/docs/controllers.md @@ -45,6 +45,7 @@ return { - `run` resolves `true` on success, so the caller can close a dialog or move focus. A cancelled reverification resolves `false`, shows no error and releases the lock. - To hold pending through an OAuth redirect, await a delay inside the action (`user-profile-connected-accounts-section.controller.ts`). +- The action receives an `AbortSignal` that aborts on unmount. Completions after unmount cannot update the hook's state. Aborting the signal does not stop an underlying provider request or undo a server mutation. Check the signal after external waits and before side effects. `reset()` only clears errors. - It is not for a form (`useForm`), a confirm step (`useConfirmController`), an action whose result drives other state (a machine), or a check that reruns as the user types (`useDebouncedAsync`, or a field's `validateAsync`). For a direct action in `ActionMenu`, opt in with `status: 'idle'` and set it to `'pending'` in the click event while the request runs and no dialog is open yet; the menu closes on the next render unless an action is pending. While pending, the menu reserves room for a spinner, disables its actions and blocks dismissal. It closes when the status returns to `'idle'`. Leave `status` undefined for ordinary actions that close through the menu's own click. diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-evm-recovery.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-evm-recovery.feature.test.tsx new file mode 100644 index 00000000000..a41ff9d46dd --- /dev/null +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-evm-recovery.feature.test.tsx @@ -0,0 +1,142 @@ +import { act, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { deferred } from '../../../../__tests__/async'; +import { serveFapi } from '../../../../__tests__/feature/fake-fapi'; +import { openMenu } from '../../../../__tests__/feature/menu'; +import { renderWithClerk } from '../../../../__tests__/feature/render'; +import { UserProfileWeb3WalletsSection } from '../user-profile-web3-wallets-section'; +import { signedInWallets } from './user-profile-web3-wallets.fixtures'; + +const request = vi.fn<(params: { method: string }) => Promise>(); +const provider = { request }; +const announceOKX = () => + window.dispatchEvent( + new CustomEvent('eip6963:announceProvider', { + detail: { info: { name: 'OKX Wallet', uuid: 'recovery-okx', icon: '', rdns: 'com.okx.wallet' }, provider }, + }), + ); + +afterEach(() => window.removeEventListener('eip6963:requestProvider', announceOKX)); + +describe.each([ + ['Base', 'web3_base_signature', '@base-org/account'], + ['Coinbase Wallet', 'web3_coinbase_wallet_signature', '@coinbase/wallet-sdk'], + ['OKX Wallet', 'web3_okx_wallet_signature', null], +] as const)('%s recovery', (name, strategy, moduleName) => { + async function setup() { + vi.stubGlobal('ethereum', { + request: () => { + throw new Error('The selected wallet must not use the fallback Ethereum provider.'); + }, + }); + const fapi = serveFapi(signedInWallets([strategy])); + const view = await renderWithClerk(); + Object.defineProperty(view.clerk, '__internal_moduleManager', { + value: { + import: (module: string) => { + expect(module).toBe(moduleName); + return Promise.resolve({ + createBaseAccountSDK: () => ({ getProvider: () => provider }), + createCoinbaseWalletSDK: () => ({ getProvider: () => provider }), + }); + }, + }, + }); + if (moduleName === null) { + window.addEventListener('eip6963:requestProvider', announceOKX); + announceOKX(); + } + return { fapi, view, user: userEvent.setup() }; + } + + it.each(['eth_requestAccounts', 'personal_sign'] as const)( + 'recovers from cancellation during %s and verifies on retry', + async method => { + request.mockImplementation(({ method: requested }) => { + if (requested === method) { + return Promise.reject(Object.assign(new Error('User rejected the request.'), { code: 4001 })); + } + return Promise.resolve(['0x1234567890abcdef']); + }); + const { fapi, user } = await setup(); + await user.click(await screen.findByRole('button', { name: `Connect ${name}` })); + await waitFor(() => + expect(screen.getByRole('alert')).toHaveTextContent('Wallet connection was cancelled. Please try again.'), + ); + request.mockImplementation(({ method: requested }) => + Promise.resolve(requested === 'personal_sign' ? 'signature' : ['0x1234567890abcdef']), + ); + if (method === 'personal_sign') { + await openMenu(user, screen.getByRole('button', { name: `Manage ${name}` })); + await user.click(screen.getByRole('menuitem', { name: 'Verify' })); + } else { + expect(fapi.client.sessions[0]?.user.web3_wallets).toHaveLength(0); + await user.click(screen.getByRole('button', { name: `Connect ${name}` })); + } + await waitFor(() => expect(fapi.client.sessions[0]?.user.web3_wallets[0]?.verification?.status).toBe('verified')); + expect(fapi.client.sessions[0]?.user.web3_wallets).toHaveLength(1); + expect(screen.queryByRole('alert')).not.toBeInTheDocument(); + }, + ); + + it('explains a pending provider request and releases its busy state', async () => { + request.mockRejectedValue({ code: -32002 }); + const { user } = await setup(); + await user.click(await screen.findByRole('button', { name: `Connect ${name}` })); + await waitFor(() => + expect(screen.getByRole('alert')).toHaveTextContent( + 'A request is already pending in your wallet. Open your wallet to complete or reject it, then try again.', + ), + ); + expect(screen.getByRole('button', { name: `Connect ${name}` })).toBeEnabled(); + }); + + it.each([ + ['eth_requestAccounts', 'resolve'], + ['eth_requestAccounts', 'reject'], + ['personal_sign', 'resolve'], + ['personal_sign', 'reject'], + ] as const)('ignores abandoned %s %s after reopening and completes the new attempt', async (method, outcome) => { + const old = deferred(); + const next = deferred(); + request.mockImplementation(async ({ method: requested }) => + requested === method ? old.promise : ['0x1234567890abcdef'], + ); + const { fapi, view, user } = await setup(); + await user.click(await screen.findByRole('button', { name: `Connect ${name}` })); + await waitFor(() => expect(request).toHaveBeenCalledWith(expect.objectContaining({ method }))); + view.rerender(
); + view.rerender(); + request.mockImplementation(async ({ method: requested }) => { + if (requested === method) { + return next.promise; + } + return requested === 'personal_sign' ? 'signature' : ['0x1234567890abcdef']; + }); + if (method === 'personal_sign') { + await openMenu(user, await screen.findByRole('button', { name: `Manage ${name}` })); + await user.click(screen.getByRole('menuitem', { name: 'Verify' })); + } else { + await user.click(await screen.findByRole('button', { name: `Connect ${name}` })); + } + await waitFor(() => expect(request.mock.calls.filter(([params]) => params.method === method)).toHaveLength(2)); + await act(() => { + if (outcome === 'resolve') { + old.resolve(method === 'personal_sign' ? 'old-signature' : ['0xold']); + } else { + old.reject({ code: 4001 }); + } + }); + expect(screen.queryByRole('alert')).not.toBeInTheDocument(); + expect(fapi.client.sessions[0]?.user.web3_wallets).toHaveLength(method === 'personal_sign' ? 1 : 0); + expect(fapi.client.sessions[0]?.user.web3_wallets[0]?.verification?.status).not.toBe('verified'); + expect( + screen.getByRole('button', { name: `${method === 'personal_sign' ? 'Manage' : 'Connect'} ${name}` }), + ).toHaveAttribute('aria-disabled', 'true'); + await act(() => next.resolve(method === 'personal_sign' ? 'signature' : ['0x1234567890abcdef'])); + await waitFor(() => expect(fapi.client.sessions[0]?.user.web3_wallets[0]?.verification?.status).toBe('verified')); + expect(fapi.client.sessions[0]?.user.web3_wallets).toHaveLength(1); + }); +}); diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-recovery.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-recovery.feature.test.tsx new file mode 100644 index 00000000000..6d0f8894d2e --- /dev/null +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-recovery.feature.test.tsx @@ -0,0 +1,126 @@ +import { act, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { expect, it, vi } from 'vitest'; + +import { deferred } from '../../../../__tests__/async'; +import { serveFapi } from '../../../../__tests__/feature/fake-fapi'; +import { renderWithClerk } from '../../../../__tests__/feature/render'; +import { UserProfileWeb3WalletsSection } from '../user-profile-web3-wallets-section'; +import { signedInWallets } from './user-profile-web3-wallets.fixtures'; + +it('handles cancellation in MetaMask without offering a Clerk cancel button and allows a fresh connection', async () => { + const accounts = deferred(); + const request = vi.fn(({ method }: { method: string }) => + method === 'personal_sign' ? Promise.resolve('signature') : accounts.promise, + ); + vi.stubGlobal('ethereum', { request }); + const fapi = serveFapi(signedInWallets(['web3_metamask_signature', 'web3_solana_signature'])); + await renderWithClerk(); + const user = userEvent.setup(); + + await user.click(await screen.findByRole('button', { name: 'Connect MetaMask' })); + await waitFor(() => expect(request).toHaveBeenCalledWith({ method: 'eth_requestAccounts' })); + expect(screen.getByRole('button', { name: 'Connect MetaMask' })).toHaveAttribute('aria-busy', 'true'); + expect(screen.getByRole('button', { name: 'Connect Solana' })).toBeDisabled(); + expect(screen.queryByRole('button', { name: /cancel/i })).not.toBeInTheDocument(); + await act(() => accounts.reject({ code: 4001, message: 'User rejected the request.' })); + + expect(screen.getByRole('button', { name: 'Connect MetaMask' })).toBeEnabled(); + expect(screen.getByRole('button', { name: 'Connect Solana' })).toBeEnabled(); + expect(screen.getByRole('alert')).toHaveTextContent('Wallet connection was cancelled. Please try again.'); + expect(fapi.client.sessions[0]?.user.web3_wallets).toHaveLength(0); + expect(request).not.toHaveBeenCalledWith(expect.objectContaining({ method: 'personal_sign' })); + + request.mockImplementation(({ method }) => + Promise.resolve(method === 'personal_sign' ? 'signature' : ['0xabcdef1234567890']), + ); + await user.click(screen.getByRole('button', { name: 'Connect MetaMask' })); + await waitFor(() => expect(fapi.client.sessions[0]?.user.web3_wallets[0]?.verification?.status).toBe('verified')); + expect(fapi.client.sessions[0]?.user.web3_wallets).toHaveLength(1); + expect(fapi.client.sessions[0]?.user.web3_wallets[0]?.web3_wallet).toBe('0xabcdef1234567890'); +}); + +it('explains a rejected MetaMask connection and releases the pending state', async () => { + const request = vi.fn().mockRejectedValue({ code: 4001, message: 'User rejected the request.' }); + vi.stubGlobal('ethereum', { request }); + serveFapi(signedInWallets(['web3_metamask_signature'])); + await renderWithClerk(); + await userEvent.setup().click(await screen.findByRole('button', { name: 'Connect MetaMask' })); + await waitFor(() => expect(screen.getByRole('button', { name: 'Connect MetaMask' })).toBeEnabled()); + expect(screen.getByRole('alert')).toHaveTextContent('Wallet connection was cancelled. Please try again.'); +}); + +it('explains that an existing MetaMask request needs attention in the wallet', async () => { + vi.stubGlobal('ethereum', { request: vi.fn().mockRejectedValue({ code: -32002 }) }); + serveFapi(signedInWallets(['web3_metamask_signature'])); + await renderWithClerk(); + await userEvent.setup().click(await screen.findByRole('button', { name: 'Connect MetaMask' })); + expect( + await screen.findByText( + 'A request is already pending in your wallet. Open your wallet to complete or reject it, then try again.', + ), + ).toBeVisible(); +}); + +it.each(['resolve', 'reject'] as const)( + 'ignores a late provider %s after closing and reopening the section', + async outcome => { + const old = deferred(); + const next = deferred(); + const request = vi.fn(({ method }: { method: string }) => + method === 'personal_sign' ? Promise.resolve('signature') : old.promise, + ); + vi.stubGlobal('ethereum', { request }); + const fapi = serveFapi(signedInWallets(['web3_metamask_signature'])); + const view = await renderWithClerk(); + const user = userEvent.setup(); + await user.click(await screen.findByRole('button', { name: 'Connect MetaMask' })); + await waitFor(() => expect(request).toHaveBeenCalledTimes(1)); + view.rerender(
); + view.rerender(); + request.mockImplementation(({ method }) => + method === 'personal_sign' ? Promise.resolve('signature') : next.promise, + ); + await user.click(await screen.findByRole('button', { name: 'Connect MetaMask' })); + await waitFor(() => expect(request).toHaveBeenCalledTimes(2)); + await act(() => { + if (outcome === 'resolve') { + old.resolve(['0xold']); + } else { + old.reject({ code: -32002 }); + } + }); + expect(screen.getByRole('button', { name: 'Connect MetaMask' })).toHaveAttribute('aria-busy', 'true'); + expect(screen.queryByRole('alert')).not.toBeInTheDocument(); + expect(fapi.client.sessions[0]?.user.web3_wallets).toHaveLength(0); + await act(() => next.resolve(['0xnew'])); + await waitFor(() => expect(fapi.client.sessions[0]?.user.web3_wallets[0]?.verification?.status).toBe('verified')); + expect(fapi.client.sessions[0]?.user.web3_wallets).toHaveLength(1); + expect(fapi.client.sessions[0]?.user.web3_wallets[0]?.web3_wallet).toBe('0xnew'); + }, +); + +it('keeps only the new wallet when MetaMask approves an abandoned request after the section closed', async () => { + const abandoned = deferred(); + const request = vi.fn(({ method }: { method: string }) => + method === 'eth_requestAccounts' ? abandoned.promise : Promise.resolve('signature'), + ); + vi.stubGlobal('ethereum', { request }); + const fapi = serveFapi(signedInWallets(['web3_metamask_signature'])); + const view = await renderWithClerk(); + const user = userEvent.setup(); + + await user.click(await screen.findByRole('button', { name: 'Connect MetaMask' })); + await waitFor(() => expect(request).toHaveBeenCalledWith({ method: 'eth_requestAccounts' })); + view.rerender(
); + await act(() => abandoned.resolve(['0xabandoned'])); + + request.mockImplementation(({ method }) => + Promise.resolve(method === 'eth_requestAccounts' ? ['0xnew'] : 'signature'), + ); + view.rerender(); + await user.click(await screen.findByRole('button', { name: 'Connect MetaMask' })); + + await waitFor(() => expect(fapi.client.sessions[0]?.user.web3_wallets[0]?.verification?.status).toBe('verified')); + expect(fapi.client.sessions[0]?.user.web3_wallets.map(wallet => wallet.web3_wallet)).toEqual(['0xnew']); +}); diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.feature.test.tsx index 7bf2d1ef0e7..666300f257d 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-section.feature.test.tsx @@ -1064,8 +1064,4 @@ describe('Web3 wallets', () => { // TODO: Share Solana discovery and filtering only after verifying parity with the legacy UI. // TODO: Share identification sorting only after verifying parity for legacy email, phone, and wallet sections. - // TODO: Add recovery for abandoned MetaMask connections. In Brave, closing the locked wallet prompt - // left wallet_requestPermissions pending. Refresh cleared Clerk's error, but a new connection returned -32002. - // Clerk cancellation must release the UI and ignore late responses without claiming to cancel the wallet request. - // The injected MetaMask API exposes no supported method to cancel that request, and clerk_go cannot clear it. }); diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-solana-recovery.feature.test.tsx b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-solana-recovery.feature.test.tsx new file mode 100644 index 00000000000..268315ac9c4 --- /dev/null +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/__tests__/user-profile-web3-wallets-solana-recovery.feature.test.tsx @@ -0,0 +1,124 @@ +import { act, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { expect, it, vi } from 'vitest'; + +import { deferred } from '../../../../__tests__/async'; +import { serveFapi } from '../../../../__tests__/feature/fake-fapi'; +import { openMenu } from '../../../../__tests__/feature/menu'; +import { renderWithClerk } from '../../../../__tests__/feature/render'; +import { UserProfileWeb3WalletsSection } from '../user-profile-web3-wallets-section'; +import { registerWallets, signedInWallets } from './user-profile-web3-wallets.fixtures'; + +const account = { + address: 'SolanaRecoveryAccount', + publicKey: new Uint8Array([1, 2, 3]), + chains: ['solana:mainnet' as const], + features: ['solana:signMessage' as const], +}; +const signatures = [{ signature: new Uint8Array([4, 5, 6]) }]; + +function registerSolana( + connect: () => Promise<{ accounts: (typeof account)[] }>, + signMessage: () => Promise, +) { + return registerWallets({ + version: '1.0.0', + name: 'Recovery Solana', + icon: 'data:image/svg+xml;base64,', + chains: ['solana:mainnet'], + accounts: [account], + features: { + 'standard:connect': { version: '1.0.0', connect }, + 'solana:signMessage': { version: '1.0.0', signMessage }, + }, + }); +} + +it('explains Solana connection cancellation and connects on retry', async () => { + const connect = vi + .fn() + .mockRejectedValueOnce({ code: 4001 }) + .mockResolvedValue({ accounts: [account] }); + const unregister = registerSolana(connect, () => Promise.resolve(signatures)); + try { + const fapi = serveFapi(signedInWallets(['web3_solana_signature'])); + await renderWithClerk(); + const user = userEvent.setup(); + await user.click(await screen.findByRole('button', { name: 'Connect Solana' })); + await user.click(await screen.findByRole('button', { name: 'Recovery Solana' })); + await waitFor(() => + expect(screen.getByRole('alert')).toHaveTextContent('Wallet connection was cancelled. Please try again.'), + ); + expect(fapi.client.sessions[0]?.user.web3_wallets).toHaveLength(0); + await user.click(screen.getByRole('button', { name: 'Recovery Solana' })); + await waitFor(() => expect(fapi.client.sessions[0]?.user.web3_wallets[0]?.verification?.status).toBe('verified')); + expect(fapi.client.sessions[0]?.user.web3_wallets).toHaveLength(1); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); + } finally { + unregister(); + } +}); + +it.each([ + ['connect', 'resolve'], + ['connect', 'reject'], + ['signMessage', 'resolve'], + ['signMessage', 'reject'], +] as const)('ignores abandoned Solana %s %s and verifies after reopening', async (stage, outcome) => { + const oldConnection = deferred<{ accounts: (typeof account)[] }>(); + const nextConnection = deferred<{ accounts: (typeof account)[] }>(); + const oldSignature = deferred(); + const nextSignature = deferred(); + const connect = vi.fn(() => (stage === 'connect' ? oldConnection.promise : Promise.resolve({ accounts: [account] }))); + const signMessage = vi.fn(() => (stage === 'signMessage' ? oldSignature.promise : Promise.resolve(signatures))); + const unregister = registerSolana(connect, signMessage); + try { + const fapi = serveFapi(signedInWallets(['web3_solana_signature'])); + const view = await renderWithClerk(); + const user = userEvent.setup(); + await user.click(await screen.findByRole('button', { name: 'Connect Solana' })); + await user.click(await screen.findByRole('button', { name: 'Recovery Solana' })); + await waitFor(() => expect(stage === 'connect' ? connect : signMessage).toHaveBeenCalledTimes(1)); + view.rerender(
); + view.rerender(); + connect.mockImplementation(() => + stage === 'connect' ? nextConnection.promise : Promise.resolve({ accounts: [account] }), + ); + signMessage.mockImplementation(() => + stage === 'signMessage' ? nextSignature.promise : Promise.resolve(signatures), + ); + if (stage === 'signMessage') { + await openMenu(user, await screen.findByRole('button', { name: 'Manage Solana' })); + await user.click(screen.getByRole('menuitem', { name: 'Verify' })); + } else { + await user.click(await screen.findByRole('button', { name: 'Connect Solana' })); + } + await user.click(await screen.findByRole('button', { name: 'Recovery Solana' })); + await waitFor(() => expect(stage === 'connect' ? connect : signMessage).toHaveBeenCalledTimes(2)); + await act(() => { + if (outcome === 'reject') { + (stage === 'connect' ? oldConnection : oldSignature).reject(new Error('User rejected the request.')); + } else if (stage === 'connect') { + oldConnection.resolve({ accounts: [account] }); + } else { + oldSignature.resolve(signatures); + } + }); + expect(screen.getByRole('alert')).toHaveTextContent(/^$/); + expect(screen.getByRole('button', { name: 'Recovery Solana' })).toHaveAttribute('aria-disabled', 'true'); + expect(fapi.client.sessions[0]?.user.web3_wallets).toHaveLength(stage === 'connect' ? 0 : 1); + expect(fapi.client.sessions[0]?.user.web3_wallets[0]?.verification?.status).not.toBe('verified'); + await act(() => { + if (stage === 'connect') { + nextConnection.resolve({ accounts: [account] }); + } else { + nextSignature.resolve(signatures); + } + }); + await waitFor(() => expect(fapi.client.sessions[0]?.user.web3_wallets[0]?.verification?.status).toBe('verified')); + expect(fapi.client.sessions[0]?.user.web3_wallets).toHaveLength(1); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); + } finally { + unregister(); + } +}); diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.controller.ts index 9d242b0b1ae..0c00ef38c03 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.controller.ts @@ -65,18 +65,23 @@ export function useUserProfileWeb3WalletsController({ }); const runConnection = (provider: UserProfileWeb3Provider, walletName?: string) => - action.run(provider.id, async () => { + action.run(provider.id, async signal => { setOwner({ kind: 'connect', providerId: provider.id, target: null }); if (walletName) { setPicker({ open: true, kind: 'connect', provider, walletName }); } try { - await connect(provider.id, walletName, target => { - setOwner({ kind: 'connect', providerId: provider.id, target }); - if (provider.walletPicker === 'solana' && target.kind === 'wallet') { - setPicker({ open: false }); - } - }); + await connect( + provider.id, + walletName, + target => { + setOwner({ kind: 'connect', providerId: provider.id, target }); + if (provider.walletPicker === 'solana' && target.kind === 'wallet') { + setPicker({ open: false }); + } + }, + signal, + ); setPicker({ open: false }); } finally { setPicker(current => @@ -86,13 +91,13 @@ export function useUserProfileWeb3WalletsController({ }); const runVerification = (walletId: string, walletName?: string) => - action.run(walletId, async () => { + action.run(walletId, async signal => { setOwner({ kind: 'verify', walletId }); if (walletName) { setPicker({ open: true, kind: 'verify', walletId, walletName }); } try { - await verify(walletId, walletName); + await verify(walletId, walletName, signal); } finally { setPicker({ open: false }); } diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.model.ts b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.model.ts index 9da99dda1c5..5d23ebdc16b 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.model.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.model.ts @@ -106,6 +106,18 @@ function walletUnavailable() { return new ClerkRuntimeError('This wallet provider is unavailable.', { code: 'web3_provider_unavailable' }); } +function rethrowProviderError(cause: unknown): never { + if (typeof cause === 'object' && cause !== null && 'code' in cause) { + if (cause.code === 4001) { + throw new ClerkRuntimeError('Wallet connection was cancelled.', { code: 'web3_connection_cancelled' }); + } + if (cause.code === -32002) { + throw new ClerkRuntimeError('A wallet request is already pending.', { code: 'web3_request_pending' }); + } + } + throw cause; +} + export function useUserProfileWeb3WalletsModel(): UserProfileWeb3WalletsModel { const clerk = useClerk(); const { isLoaded, user } = useUser(); @@ -120,8 +132,11 @@ export function useUserProfileWeb3WalletsModel(): UserProfileWeb3WalletsModel { const [setPrimaryWallet, setPrimaryReverification] = useReverificationFlow( (current: UserResource, walletId: string) => current.update({ primaryWeb3WalletId: walletId }), ); - const [createWallet, createReverification] = useReverificationFlow((current: UserResource, web3Wallet: string) => - current.createWeb3Wallet({ web3Wallet }), + const [createWallet, createReverification] = useReverificationFlow( + (current: UserResource, web3Wallet: string, signal?: AbortSignal) => { + signal?.throwIfAborted(); + return current.createWeb3Wallet({ web3Wallet }); + }, ); if (!isLoaded || !environment) { @@ -155,14 +170,18 @@ export function useUserProfileWeb3WalletsModel(): UserProfileWeb3WalletsModel { identifier, walletName, web3, + signal, }: { wallet: Web3WalletResource; provider: (typeof WEB3_PROVIDERS)[number]; identifier: string; walletName?: string; web3: ReturnType; + signal?: AbortSignal; }) => { + signal?.throwIfAborted(); const prepared = await wallet.prepareVerification({ strategy: provider.strategy }); + signal?.throwIfAborted(); requireCurrentUser(); const nonce = prepared.verification.message; if (!nonce) { @@ -170,7 +189,10 @@ export function useUserProfileWeb3WalletsModel(): UserProfileWeb3WalletsModel { code: 'web3_verification_message_unavailable', }); } - const signature = await web3.generateWeb3Signature({ identifier, nonce, provider: provider.provider, walletName }); + const signature = await web3 + .generateWeb3Signature({ identifier, nonce, provider: provider.provider, walletName }) + .catch(rethrowProviderError); + signal?.throwIfAborted(); requireCurrentUser(); if (!signature) { throw new ClerkRuntimeError('The wallet signature is unavailable.', { code: 'web3_signature_unavailable' }); @@ -180,7 +202,8 @@ export function useUserProfileWeb3WalletsModel(): UserProfileWeb3WalletsModel { return { ...projection, userId, - connect: async (strategy, walletName, onTarget) => { + connect: async (strategy, walletName, onTarget, signal) => { + signal?.throwIfAborted(); requireCurrentUser(); const provider = WEB3_PROVIDERS.find(candidate => candidate.strategy === strategy); const available = projection.availableProviders.some(candidate => candidate.id === strategy); @@ -189,7 +212,10 @@ export function useUserProfileWeb3WalletsModel(): UserProfileWeb3WalletsModel { throw new ClerkRuntimeError('This wallet provider is unavailable.', { code: 'web3_provider_unavailable' }); } const web3 = createWeb3(manager); - const identifier = await web3.getWeb3Identifier({ provider: provider.provider, walletName }); + const identifier = await web3 + .getWeb3Identifier({ provider: provider.provider, walletName }) + .catch(rethrowProviderError); + signal?.throwIfAborted(); if (!identifier) { throw new ClerkRuntimeError('A Web3 Wallet extension cannot be found.', { code: 'web3_missing_identifier' }); } @@ -213,15 +239,17 @@ export function useUserProfileWeb3WalletsModel(): UserProfileWeb3WalletsModel { throw walletUnavailable(); } onTarget?.(existing ? { kind: 'wallet', id: existing.id } : { kind: 'address', address: normalizedIdentifier }); - const wallet = existing ?? (await createWallet(current, identifier)); + const wallet = existing ?? (await createWallet(current, identifier, signal)); + signal?.throwIfAborted(); requireCurrentUser(); if (!wallet) { throw new ClerkRuntimeError('The wallet could not be created.', { code: 'web3_wallet_creation_failed' }); } onTarget?.({ kind: 'wallet', id: wallet.id }); - await attemptVerification({ wallet, provider, identifier, walletName, web3 }); + await attemptVerification({ wallet, provider, identifier, walletName, web3, signal }); }, - verify: async (walletId, walletName) => { + verify: async (walletId, walletName, signal) => { + signal?.throwIfAborted(); const current = requireCurrentUser(); const wallet = current.web3Wallets.find(candidate => candidate.id === walletId); const provider = WEB3_PROVIDERS.find(candidate => candidate.strategy === wallet?.verification.strategy); @@ -235,7 +263,10 @@ export function useUserProfileWeb3WalletsModel(): UserProfileWeb3WalletsModel { throw walletUnavailable(); } const web3 = createWeb3(manager); - const identifier = await web3.getWeb3Identifier({ provider: provider.provider, walletName }); + const identifier = await web3 + .getWeb3Identifier({ provider: provider.provider, walletName }) + .catch(rethrowProviderError); + signal?.throwIfAborted(); if (!identifier) { throw new ClerkRuntimeError('A Web3 Wallet extension cannot be found.', { code: 'web3_missing_identifier' }); } @@ -254,7 +285,7 @@ export function useUserProfileWeb3WalletsModel(): UserProfileWeb3WalletsModel { code: 'web3_wallet_account_mismatch', }); } - await attemptVerification({ wallet: latest, provider, identifier, walletName, web3 }); + await attemptVerification({ wallet: latest, provider, identifier, walletName, web3, signal }); }, setPrimary: async walletId => { const current = requireCurrentUser(); diff --git a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.types.ts b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.types.ts index a0ce8fec689..4c53961c2cd 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.types.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-web3-wallets-section/user-profile-web3-wallets-section.types.ts @@ -62,8 +62,9 @@ export type UserProfileWeb3WalletsModel = strategy: Web3Strategy, walletName?: string, onTarget?: (target: Web3ConnectionTarget) => void, + signal?: AbortSignal, ) => Promise; - verify: (walletId: string, walletName?: string) => Promise; + verify: (walletId: string, walletName?: string, signal?: AbortSignal) => Promise; setPrimary: (walletId: string) => Promise; remove?: (walletId: string) => Promise; removeReverification: ReverificationController; diff --git a/packages/mosaic/src/hooks/__tests__/use-pending-action.test.tsx b/packages/mosaic/src/hooks/__tests__/use-pending-action.test.tsx index b2e39244861..253ae6f353f 100644 --- a/packages/mosaic/src/hooks/__tests__/use-pending-action.test.tsx +++ b/packages/mosaic/src/hooks/__tests__/use-pending-action.test.tsx @@ -264,3 +264,26 @@ describe('usePendingAction', () => { expect(result.current.errorKey).toBeUndefined(); }); }); + +it.each(['resolve', 'reject'] as const)('ignores a late %s after unmount', async outcome => { + const pending = deferred(); + const { result, unmount } = renderHook(() => usePendingAction()); + let signal: AbortSignal | undefined; + let run: Promise | undefined; + act(() => { + run = result.current.run('a', current => { + signal = current; + return pending.promise; + }); + }); + unmount(); + expect(signal?.aborted).toBe(true); + await act(async () => { + if (outcome === 'resolve') { + pending.resolve(); + } else { + pending.reject(blocked()); + } + expect(await run).toBe(false); + }); +}); diff --git a/packages/mosaic/src/hooks/use-pending-action.ts b/packages/mosaic/src/hooks/use-pending-action.ts index bfeb88f3274..c7e78f554c6 100644 --- a/packages/mosaic/src/hooks/use-pending-action.ts +++ b/packages/mosaic/src/hooks/use-pending-action.ts @@ -1,5 +1,5 @@ import { isReverificationCancelledError } from '@clerk/shared/error'; -import { useRef, useState } from 'react'; +import { useEffect, useRef, useState } from 'react'; import type { ErrorDescription } from '../localization'; import { useErrorText } from '../localization'; @@ -11,7 +11,11 @@ export interface PendingActionOptions { } export interface PendingAction { - run: (key: TKey, action: () => Promise | void, options?: PendingActionOptions) => Promise; + run: ( + key: TKey, + action: (signal: AbortSignal) => Promise | void, + options?: PendingActionOptions, + ) => Promise; pendingKey: TKey | undefined; lastKey: TKey | undefined; isPending: boolean; @@ -28,7 +32,8 @@ interface PendingActionFailure { /** * Runs inline actions, such as row buttons, under one lock and tracks which key is pending, which key ran last, and which key last failed. - * `run` resolves `true` on success and `false` on failure, ignores calls while any run is in flight, and clears the last error when it starts. + * `run` resolves `true` on success and `false` on failure or cancellation, ignores calls while a run owns the lock, and clears the last error when it starts. + * Unmount aborts the action signal; callers guard their own side effects. */ export function usePendingAction({ errorFallback, @@ -37,27 +42,42 @@ export function usePendingAction({ const [pendingKey, setPendingKey] = useState(); const [lastKey, setLastKey] = useState(); const [failure, setFailure] = useState>(); - const running = useRef(false); + const running = useRef(); - const run = async (key: TKey, action: () => Promise | void, options: PendingActionOptions = {}) => { + useEffect( + () => () => { + running.current?.abort(); + running.current = undefined; + }, + [], + ); + + const run = async ( + key: TKey, + action: (signal: AbortSignal) => Promise | void, + options: PendingActionOptions = {}, + ) => { if (running.current) { return false; } - running.current = true; + const controller = new AbortController(); + running.current = controller; setPendingKey(key); setLastKey(key); setFailure(undefined); try { - await action(); - return true; + await action(controller.signal); + return !controller.signal.aborted; } catch (cause) { - if (!isReverificationCancelledError(cause)) { + if (!controller.signal.aborted && !isReverificationCancelledError(cause)) { setFailure({ key, error: toLocalizableError(cause), errorFallback: options.errorFallback }); } return false; } finally { - running.current = false; - setPendingKey(undefined); + if (running.current === controller) { + running.current = undefined; + setPendingKey(undefined); + } } }; diff --git a/packages/mosaic/src/localization/errors.messages.ts b/packages/mosaic/src/localization/errors.messages.ts index 8ea1d2fa8e3..076c8a27f3c 100644 --- a/packages/mosaic/src/localization/errors.messages.ts +++ b/packages/mosaic/src/localization/errors.messages.ts @@ -95,6 +95,9 @@ export const errorMessages: { readonly generic: string } & Readonly