From 0272437a4d096890875796bc9daeab6923b08003 Mon Sep 17 00:00:00 2001 From: Alex Carpenter Date: Fri, 2 Oct 2026 19:45:29 -0400 Subject: [PATCH 1/7] fix(mosaic): localize errors in profile actions and confirmations --- .changeset/mosaic-to-localizable-error.md | 2 + .../skills/mosaic/references/controllers.md | 19 +++ .claude/skills/mosaic/references/machines.md | 8 +- .claude/skills/mosaic/references/models.md | 5 + packages/mosaic/src/__tests__/clerk-errors.ts | 15 +++ .../confirmation.controller.test.ts | 42 +++++-- .../confirmation/confirmation.controller.ts | 24 ++-- .../blocks/confirmation/confirmation.test.tsx | 66 +++++++++- .../src/blocks/confirmation/confirmation.tsx | 7 +- .../destructive.controller.test.ts | 26 +++- .../destructive/destructive.controller.ts | 20 +-- .../blocks/destructive/destructive.test.tsx | 5 +- .../src/blocks/destructive/destructive.tsx | 5 +- .../src/features/api-keys/api-keys-table.tsx | 1 + .../features/api-keys/api-keys-table.view.tsx | 9 +- .../invitations-table-tab.view.test.tsx | 38 +++--- .../__tests__/members-table-tab.view.test.tsx | 24 +++- .../requests-table-tab.view.test.tsx | 9 +- .../invitations-table-tab.view.tsx | 9 +- .../members-table-tab.view.tsx | 9 +- ...anization-profile-edit-field.controller.ts | 1 + .../requests-table-tab.view.tsx | 32 ++--- .../reverification/reverification.utils.ts | 1 + ...rofile-connected-accounts.feature.test.tsx | 2 +- ...rofile-danger-section.integration.test.tsx | 7 +- .../user-profile-device-actions.test.tsx | 33 ++++- .../user-profile-email-actions.test.tsx | 3 +- ...ofile-enterprise-accounts.feature.test.tsx | 6 +- .../user-profile-mfa-section.view.test.tsx | 17 ++- ...ser-profile-passkeys-section.view.test.tsx | 28 ++++- .../user-profile-phone-actions.test.tsx | 3 +- .../user-profile-web3-actions.test.tsx | 3 +- ...er-profile-active-devices-section.view.tsx | 50 +++----- ...rofile-connected-accounts-feedback.test.ts | 75 ----------- ...ser-profile-connected-accounts-feedback.ts | 27 ---- ...e-connected-accounts-section.controller.ts | 28 +++-- ...ile-connected-accounts-section.messages.ts | 2 - ...rofile-connected-accounts-section.model.ts | 116 ++++++++---------- ...rofile-connected-accounts-section.types.ts | 11 -- .../user-profile-device-details.dialog.tsx | 35 ++---- ...ofile-enterprise-accounts-feedback.test.ts | 49 -------- ...er-profile-enterprise-accounts-feedback.ts | 16 --- ...-enterprise-accounts-section.controller.ts | 19 +-- ...le-enterprise-accounts-section.messages.ts | 2 - ...ofile-enterprise-accounts-section.model.ts | 24 ++-- ...er-profile-enterprise-accounts-section.tsx | 2 +- .../user-profile-mfa-section.view.tsx | 41 +++---- .../user-profile-password-errors.ts | 1 + .../user-profile-rename-passkey.controller.ts | 10 +- .../src/hooks/__tests__/use-action.test.tsx | 104 ++++++++++++++++ packages/mosaic/src/hooks/use-action.ts | 57 +++++++++ .../src/localization/errors.messages.ts | 4 + .../mosaic/src/localization/errors.test.tsx | 21 +++- packages/mosaic/src/localization/errors.ts | 12 +- packages/mosaic/src/utils/form-error.test.ts | 60 +++++++++ packages/mosaic/src/utils/form-error.ts | 13 ++ 56 files changed, 761 insertions(+), 497 deletions(-) create mode 100644 .changeset/mosaic-to-localizable-error.md create mode 100644 packages/mosaic/src/__tests__/clerk-errors.ts delete mode 100644 packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-feedback.test.ts delete mode 100644 packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-feedback.ts delete mode 100644 packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-feedback.test.ts delete mode 100644 packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-feedback.ts create mode 100644 packages/mosaic/src/hooks/__tests__/use-action.test.tsx create mode 100644 packages/mosaic/src/hooks/use-action.ts create mode 100644 packages/mosaic/src/utils/form-error.test.ts diff --git a/.changeset/mosaic-to-localizable-error.md b/.changeset/mosaic-to-localizable-error.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/mosaic-to-localizable-error.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/.claude/skills/mosaic/references/controllers.md b/.claude/skills/mosaic/references/controllers.md index c1481058f84..89d2a8efcff 100644 --- a/.claude/skills/mosaic/references/controllers.md +++ b/.claude/skills/mosaic/references/controllers.md @@ -120,6 +120,25 @@ have a machine. } ``` +## Errors + +One path, so every error a user sees goes through the `errors.*` catalog: + +- **Owners map, nothing else does.** `toLocalizableError(cause)` is called only + by the hooks that own a failure: `useForm`, `useConfirmationController`, + `useDestructiveController` and `useAction` (inline row actions). Use one of + them instead of catching in a feature. +- **Copy is resolved at render.** Owners store the `LocalizableError` and return + `errorMessage` from `useErrorText()`: the catalog entry for + `code__paramName`, then `code`, then Clerk's message, then the feature's + `errorFallback`, then the generic error. +- **`errorFallback` is the feature's copy for faults Clerk cannot describe** + (network, code bugs). An unknown error is logged and its `.message` is never + shown. +- In tests, reject with `clerkApiError(code, message)` from + `src/__tests__/clerk-errors.ts` to assert catalog copy. Reject with a plain + `Error` (silence `console.error`) to assert the fallback. + ## Testing The feature test covers the controller by default. Hold the diff --git a/.claude/skills/mosaic/references/machines.md b/.claude/skills/mosaic/references/machines.md index ea1db234cdb..160122062c2 100644 --- a/.claude/skills/mosaic/references/machines.md +++ b/.claude/skills/mosaic/references/machines.md @@ -42,14 +42,16 @@ deleting: { onDone: 'deleted', onError: { target: 'confirming', - actions: assign((_, event) => ({ - errorMessage: event.error instanceof Error ? event.error.message : 'Something went wrong.', - })), + actions: assign((_, event) => ({ error: toLocalizableError(event.error) })), }, }), } ``` +Context holds the `LocalizableError`, never copy. The controller turns it into +text at render with `errorText(error, errorFallback)`, so locale changes and +overrides still apply (see `controllers.md` → "Errors"). + To wire it to Clerk data see `models.md`; to render its state see `views.md`; to test it see `testing.md`; to migrate a legacy component into this pattern see `migration.md`. diff --git a/.claude/skills/mosaic/references/models.md b/.claude/skills/mosaic/references/models.md index 44858716d3a..74ffaa77968 100644 --- a/.claude/skills/mosaic/references/models.md +++ b/.claude/skills/mosaic/references/models.md @@ -83,6 +83,11 @@ drops the fallback instead of holding the space open. Keep the two apart. `organizationId`), never a resource. - An async callback returns its promise — the controller drives pending state off it. Navigation callbacks stay fire-and-forget. +- Never format an error. Let Clerk errors propagate untouched. For a failure + the model detects itself, throw + `new ClerkRuntimeError(message, { code })` and add the copy for `code` to + `src/localization/errors.messages.ts`. No `try`/`catch` that turns errors into + strings, no feature-level `toError` helpers. - No local UI state. What is open and what is in flight belong to the controller. - No React state machinery beyond the Clerk hooks themselves; the model is a derivation of what Clerk currently says. diff --git a/packages/mosaic/src/__tests__/clerk-errors.ts b/packages/mosaic/src/__tests__/clerk-errors.ts new file mode 100644 index 00000000000..f1b2082a704 --- /dev/null +++ b/packages/mosaic/src/__tests__/clerk-errors.ts @@ -0,0 +1,15 @@ +import { ClerkAPIResponseError } from '@clerk/shared/error'; + +export function clerkApiError(code: string, longMessage: string, { paramName }: { paramName?: string } = {}) { + return new ClerkAPIResponseError(longMessage, { + data: [ + { + code, + message: longMessage, + long_message: longMessage, + ...(paramName ? { meta: { param_name: paramName } } : {}), + }, + ], + status: 422, + }); +} diff --git a/packages/mosaic/src/blocks/confirmation/confirmation.controller.test.ts b/packages/mosaic/src/blocks/confirmation/confirmation.controller.test.ts index 9480a74c191..a178e506106 100644 --- a/packages/mosaic/src/blocks/confirmation/confirmation.controller.test.ts +++ b/packages/mosaic/src/blocks/confirmation/confirmation.controller.test.ts @@ -1,9 +1,16 @@ import { act, renderHook, waitFor } from '@testing-library/react'; -import { describe, expect, it, vi } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { createActor } from '../../machine/create-actor'; +import { SaveError } from '../../utils/form-error'; import { confirmationMachine, useConfirmationController } from './confirmation.controller'; +const blocked = new SaveError({ global: { code: 'action_blocked', message: 'Raw server sentence.' } }); + +afterEach(() => { + vi.restoreAllMocks(); +}); + function start() { const actor = createActor(confirmationMachine).start(); actor.send({ type: 'OPEN' }); @@ -43,24 +50,25 @@ describe('confirmationMachine', () => { it('lands back on confirming with the reason when the action fails', async () => { const actor = start(); - actor.send({ type: 'CONFIRM', run: () => Promise.reject(new Error('Google is your only way to sign in.')) }); + actor.send({ type: 'CONFIRM', run: () => Promise.reject(blocked) }); await vi.waitFor(() => expect(actor.getSnapshot().value).toBe('confirming')); - expect(actor.getSnapshot().context.error).toBe('Google is your only way to sign in.'); + expect(actor.getSnapshot().context.error).toEqual(blocked.formError.global); }); - it('falls back to generic copy when the rejection is not an Error', async () => { + it('holds nothing of an unexpected error, never its message', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}); const actor = start(); - // eslint-disable-next-line @typescript-eslint/prefer-promise-reject-errors -- a non-Error rejection is the case under test - actor.send({ type: 'CONFIRM', run: () => Promise.reject('nope') }); + actor.send({ type: 'CONFIRM', run: () => Promise.reject(new Error('Cannot read properties of undefined')) }); - await vi.waitFor(() => expect(actor.getSnapshot().context.error).toBe('Something went wrong. Please try again.')); + await vi.waitFor(() => expect(actor.getSnapshot().value).toBe('confirming')); + expect(actor.getSnapshot().context.error).toEqual({}); }); it('drops the error when cancelled, so the next open starts clean', async () => { const actor = start(); - actor.send({ type: 'CONFIRM', run: () => Promise.reject(new Error('nope')) }); - await vi.waitFor(() => expect(actor.getSnapshot().context.error).toBe('nope')); + actor.send({ type: 'CONFIRM', run: () => Promise.reject(blocked) }); + await vi.waitFor(() => expect(actor.getSnapshot().context.error).toBeDefined()); actor.send({ type: 'CANCEL' }); @@ -85,14 +93,24 @@ describe('useConfirmationController', () => { await waitFor(() => expect(result.current.isOpen).toBe(false)); }); - it('surfaces a failure as the error message and stays open', async () => { + it('surfaces a failure as the localized copy for its code and stays open', async () => { const { result } = renderHook(() => useConfirmationController()); act(() => result.current.onOpenChange(true)); - act(() => result.current.onConfirm(() => Promise.reject(new Error('nope')))); + act(() => result.current.onConfirm(() => Promise.reject(blocked))); - await waitFor(() => expect(result.current.errorMessage).toBe('nope')); + await waitFor(() => expect(result.current.errorMessage).toMatch(/contact support/)); expect(result.current.isOpen).toBe(true); expect(result.current.isConfirming).toBe(false); }); + + it('surfaces the fallback it was given for an unexpected error', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}); + const { result } = renderHook(() => useConfirmationController({ errorFallback: 'Unable to remove this member.' })); + act(() => result.current.onOpenChange(true)); + + act(() => result.current.onConfirm(() => Promise.reject(new Error('internal')))); + + await waitFor(() => expect(result.current.errorMessage).toBe('Unable to remove this member.')); + }); }); diff --git a/packages/mosaic/src/blocks/confirmation/confirmation.controller.ts b/packages/mosaic/src/blocks/confirmation/confirmation.controller.ts index be5c6200f4c..1f614f7f1f6 100644 --- a/packages/mosaic/src/blocks/confirmation/confirmation.controller.ts +++ b/packages/mosaic/src/blocks/confirmation/confirmation.controller.ts @@ -1,9 +1,12 @@ +import type { LocalizableError } from '../../localization'; +import { useErrorText } from '../../localization'; import { setup } from '../../machine/setup'; import { useMachine } from '../../machine/use-machine'; +import { toLocalizableError } from '../../utils/form-error'; export interface ConfirmationContext { run: () => Promise; - error: string | undefined; + error: LocalizableError | undefined; } export type ConfirmationEvent = { type: 'OPEN' } | { type: 'CONFIRM'; run: () => Promise } | { type: 'CANCEL' }; @@ -14,10 +17,6 @@ function notSeated(): Promise { return Promise.reject(new Error('confirmation run is not seated')); } -function toMessage(cause: unknown): string { - return cause instanceof Error ? cause.message : 'Something went wrong. Please try again.'; -} - export const confirmationMachine = createMachine({ id: 'confirmation', initial: 'idle', @@ -42,7 +41,7 @@ export const confirmationMachine = createMachine({ onDone: { target: 'idle', actions: assign(() => ({ error: undefined })) }, onError: { target: 'confirming', - actions: assign((_, event) => ({ error: toMessage(event.error) })), + actions: assign((_, event) => ({ error: toLocalizableError(event.error) })), }, }), }, @@ -57,14 +56,23 @@ export interface ConfirmationController { errorMessage: string | undefined; } -export function useConfirmationController(): ConfirmationController { +export interface ConfirmationControllerOptions { + /** Copy shown when the action fails without an error Clerk can describe, such as a network or code fault (default: the generic error) */ + errorFallback?: string; +} + +export function useConfirmationController({ + errorFallback, +}: ConfirmationControllerOptions = {}): ConfirmationController { const [snapshot, send] = useMachine(confirmationMachine); + const errorText = useErrorText(); + const { error } = snapshot.context; return { isOpen: snapshot.value === 'confirming' || snapshot.value === 'pending', onOpenChange: open => send({ type: open ? 'OPEN' : 'CANCEL' }), onConfirm: run => send({ type: 'CONFIRM', run }), isConfirming: snapshot.value === 'pending', - errorMessage: snapshot.context.error, + errorMessage: error ? errorText(error, errorFallback) : undefined, }; } diff --git a/packages/mosaic/src/blocks/confirmation/confirmation.test.tsx b/packages/mosaic/src/blocks/confirmation/confirmation.test.tsx index e7f42bb6ce8..e75471cb252 100644 --- a/packages/mosaic/src/blocks/confirmation/confirmation.test.tsx +++ b/packages/mosaic/src/blocks/confirmation/confirmation.test.tsx @@ -1,8 +1,10 @@ import { act, render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import { describe, expect, it, vi } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { clerkApiError } from '../../__tests__/clerk-errors'; import { Button } from '../../components/button'; +import type { MosaicLocalization } from '../../localization'; import { MosaicProvider } from '../../mosaic-provider'; import type { ConfirmationControlledProps, ConfirmationHandleProps } from './confirmation'; import { Confirmation } from './confirmation'; @@ -101,11 +103,15 @@ interface Member { const preston: Member = { id: 'mem_1', name: 'Preston Booth' }; -function renderWithHandle(onConfirm: ConfirmationHandleProps['onConfirm'] = () => Promise.resolve()) { +function renderWithHandle( + onConfirm: ConfirmationHandleProps['onConfirm'] = () => Promise.resolve(), + { errorFallback, localization }: { errorFallback?: string; localization?: MosaicLocalization } = {}, +) { const handle = Confirmation.createHandle(); render( - + ( @@ -121,6 +127,10 @@ function renderWithHandle(onConfirm: ConfirmationHandleProps['onConfirm' return handle; } +afterEach(() => { + vi.restoreAllMocks(); +}); + const removeButton = () => screen.getByRole('button', { name: 'Remove Preston Booth' }); describe('Confirmation with a handle', () => { @@ -175,7 +185,9 @@ describe('Confirmation with a handle', () => { it('keeps the dialog open and explains a failed attempt', async () => { const user = userEvent.setup(); - const handle = renderWithHandle(() => Promise.reject(new Error('Preston Booth is the last admin.'))); + const handle = renderWithHandle(() => + Promise.reject(clerkApiError('organization_minimum_permissions_needed', 'Preston Booth is the last admin.')), + ); act(() => { handle.open(preston); }); @@ -187,14 +199,56 @@ describe('Confirmation with a handle', () => { expect(removeButton()).not.toHaveAttribute('aria-busy', 'true'); }); + it('shows the localized copy for the code a refusal carries', async () => { + const user = userEvent.setup(); + const handle = renderWithHandle(() => Promise.reject(clerkApiError('action_blocked', 'Raw server sentence.')), { + localization: { overrides: { 'errors.action_blocked': 'Acción bloqueada.' } }, + }); + act(() => { + handle.open(preston); + }); + + await user.click(removeButton()); + + await waitFor(() => expect(screen.getByRole('alert')).toHaveTextContent('Acción bloqueada.')); + }); + + it('shows the fallback rather than what an unexpected error says', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}); + const user = userEvent.setup(); + const handle = renderWithHandle(() => Promise.reject(new TypeError('Cannot read properties of undefined')), { + errorFallback: 'Unable to remove this member.', + }); + act(() => { + handle.open(preston); + }); + + await user.click(removeButton()); + + await waitFor(() => expect(screen.getByRole('alert')).toHaveTextContent('Unable to remove this member.')); + }); + + it('shows the generic error when no fallback is given', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}); + const user = userEvent.setup(); + const handle = renderWithHandle(() => Promise.reject(new Error('internal'))); + act(() => { + handle.open(preston); + }); + + await user.click(removeButton()); + + await waitFor(() => expect(screen.getByRole('alert')).toHaveTextContent('Something went wrong. Please try again.')); + }); + it('starts the next open clean after a failure', async () => { const user = userEvent.setup(); - const handle = renderWithHandle(() => Promise.reject(new Error('nope'))); + const handle = renderWithHandle(() => Promise.reject(clerkApiError('action_blocked', 'nope'))); act(() => { handle.open(preston); }); await user.click(removeButton()); - await waitFor(() => expect(screen.getByRole('alert')).toBeInTheDocument()); + await waitFor(() => expect(screen.getByRole('alert')).not.toBeEmptyDOMElement()); await user.click(screen.getByRole('button', { name: 'Cancel' })); await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()); diff --git a/packages/mosaic/src/blocks/confirmation/confirmation.tsx b/packages/mosaic/src/blocks/confirmation/confirmation.tsx index bc7fece37e6..586fb6f9a0a 100644 --- a/packages/mosaic/src/blocks/confirmation/confirmation.tsx +++ b/packages/mosaic/src/blocks/confirmation/confirmation.tsx @@ -174,8 +174,10 @@ export interface ConfirmationHandleProps { actionLabel: FromPayload; /** Text of the cancel button (default: "Cancel") */ cancelLabel?: string; - /** Runs the action for the payload. Resolve to close the dialog; reject with an `Error` to keep it open showing why */ + /** Runs the action for the payload. Resolve to close the dialog; reject to keep it open showing why */ onConfirm: (payload: Payload) => Promise | void; + /** Copy shown when the action fails without an error Clerk can describe, such as a network or code fault (default: the generic error) */ + errorFallback?: string; } function HandleConfirmation({ @@ -187,8 +189,9 @@ function HandleConfirmation({ actionLabel, cancelLabel = 'Cancel', onConfirm, + errorFallback, }: ConfirmationHandleProps) { - const controller = useConfirmationController(); + const controller = useConfirmationController({ errorFallback }); return ( { + vi.restoreAllMocks(); +}); + describe('useDestructiveController', () => { it('starts closed and opens from the opener or from onOpenChange', () => { const { result } = renderHook(() => useDestructiveController({ onDelete: () => Promise.resolve() })); @@ -37,6 +42,21 @@ describe('useDestructiveController', () => { expect(result.current.isDeleting).toBe(true); }); + it('shows the fallback, never the message, of an unexpected error', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}); + const onDelete = () => Promise.reject(new Error('Cannot read properties of undefined')); + const { result } = renderHook(() => + useDestructiveController({ onDelete, errorFallback: 'Unable to delete this account.' }), + ); + act(() => result.current.onOpenChange(true)); + + await act(async () => { + await result.current.onDelete(); + }); + + expect(result.current.errorMessage).toBe('Unable to delete this account.'); + }); + it('stays open and pending until the action resolves, then closes', async () => { const pending = deferred(); const onDelete = vi.fn(() => pending.promise); @@ -61,7 +81,7 @@ describe('useDestructiveController', () => { it('stays open with a message when the action rejects, and a retry can succeed', async () => { const onDelete = vi .fn<() => Promise>() - .mockRejectedValueOnce(new Error('Your subscription is still active.')) + .mockRejectedValueOnce(new SaveError({ global: { code: 'action_blocked', message: 'Raw server sentence.' } })) .mockResolvedValueOnce(undefined); const { result } = renderHook(() => useDestructiveController({ onDelete })); act(() => result.current.onOpenChange(true)); @@ -71,7 +91,7 @@ describe('useDestructiveController', () => { }); expect(result.current.open).toBe(true); expect(result.current.isDeleting).toBe(false); - expect(result.current.errorMessage).toBe('Something went wrong. Please try again.'); + expect(result.current.errorMessage).toMatch(/contact support/); await act(async () => { await result.current.onDelete(); diff --git a/packages/mosaic/src/blocks/destructive/destructive.controller.ts b/packages/mosaic/src/blocks/destructive/destructive.controller.ts index 7673772bd04..a65e8fde901 100644 --- a/packages/mosaic/src/blocks/destructive/destructive.controller.ts +++ b/packages/mosaic/src/blocks/destructive/destructive.controller.ts @@ -1,9 +1,10 @@ -import { isClerkAPIResponseError, isReverificationCancelledError } from '@clerk/shared/error'; +import { isReverificationCancelledError } from '@clerk/shared/error'; import { useState } from 'react'; import type { ReverificationController } from '../../features/reverification'; import type { LocalizableError } from '../../localization'; -import { toLocalizableApiError, useErrorText } from '../../localization'; +import { useErrorText } from '../../localization'; +import { toLocalizableError } from '../../utils/form-error'; import type { DestructiveControlledProps } from './destructive'; export type DestructiveController = Pick< @@ -18,15 +19,18 @@ type DestructiveState = | { status: 'closed' } | { status: 'open-needs-confirmation' } | { status: 'open-pending' } - | { status: 'open-error'; errorMessage: string }; + | { status: 'open-error'; error: LocalizableError }; /** Optional controller for use with Destructive block */ export function useDestructiveController({ onDelete, reverification, + errorFallback, }: { onDelete: () => Promise; reverification?: ReverificationController; + /** Copy shown when the action fails without an error Clerk can describe, such as a network or code fault (default: the generic error) */ + errorFallback?: string; }): DestructiveController { const [destructiveState, setDestructiveState] = useState({ status: 'closed' }); const errorText = useErrorText(); @@ -44,7 +48,8 @@ export function useDestructiveController({ return { open: status !== 'closed', isDeleting, - errorMessage: status === 'open-error' ? destructiveState.errorMessage : undefined, + errorMessage: + destructiveState.status === 'open-error' ? errorText(destructiveState.error, errorFallback) : undefined, reverification, step, openDestructiveDialog, @@ -60,7 +65,7 @@ export function useDestructiveController({ setDestructiveState({ status: 'closed' }); return; } - setDestructiveState({ status: 'open-error', errorMessage: errorText(toLocalizableError(error)) }); + setDestructiveState({ status: 'open-error', error: toLocalizableError(error) }); } } }, @@ -79,8 +84,3 @@ export function useDestructiveController({ }, }; } - -function toLocalizableError(error: unknown): LocalizableError { - const apiError = isClerkAPIResponseError(error) ? error.errors[0] : undefined; - return apiError ? toLocalizableApiError(apiError) : {}; -} diff --git a/packages/mosaic/src/blocks/destructive/destructive.test.tsx b/packages/mosaic/src/blocks/destructive/destructive.test.tsx index e8725625f74..6bdacdaf973 100644 --- a/packages/mosaic/src/blocks/destructive/destructive.test.tsx +++ b/packages/mosaic/src/blocks/destructive/destructive.test.tsx @@ -7,6 +7,7 @@ import { Button } from '../../components/button'; import { Menu } from '../../components/menu'; import type { ReverificationController } from '../../features/reverification'; import { MosaicProvider } from '../../mosaic-provider'; +import { SaveError } from '../../utils/form-error'; import type { DestructiveControlledProps, DestructiveHandleProps } from './destructive'; import { Destructive } from './destructive'; @@ -345,7 +346,9 @@ describe('Destructive with a handle', () => { it('shows a deletion failure, clears it on reopening, and allows retrying', async () => { const onDelete = vi .fn['onDelete']>() - .mockRejectedValueOnce(new Error('The account still has active projects.')) + .mockRejectedValueOnce( + new SaveError({ global: { code: 'account_has_projects', message: 'The account still has active projects.' } }), + ) .mockResolvedValueOnce(undefined); const user = userEvent.setup(); const handle = renderWithHandle(onDelete); diff --git a/packages/mosaic/src/blocks/destructive/destructive.tsx b/packages/mosaic/src/blocks/destructive/destructive.tsx index 4fd87db9da4..5e5dd15e8ae 100644 --- a/packages/mosaic/src/blocks/destructive/destructive.tsx +++ b/packages/mosaic/src/blocks/destructive/destructive.tsx @@ -214,6 +214,8 @@ export interface DestructiveHandleProps { actionLabel: FromPayload; cancelLabel?: string; onDelete: (payload: Payload) => Promise | void; + /** Copy shown when the action fails without an error Clerk can describe, such as a network or code fault (default: the generic error) */ + errorFallback?: string; } function HandleDestructive({ @@ -226,8 +228,9 @@ function HandleDestructive({ actionLabel, cancelLabel, onDelete, + errorFallback, }: DestructiveHandleProps) { - const controller = useConfirmationController(); + const controller = useConfirmationController({ errorFallback }); return ( apiKey.name} actionLabel={m.revoke} cancelLabel={m.cancel} - onDelete={async apiKey => { - try { - await removalFocus.remove(apiKey.id); - } catch (error) { - throw error instanceof Error ? error : new Error(m.revokeError); - } - }} + onDelete={apiKey => removalFocus.remove(apiKey.id)} + errorFallback={m.revokeError} finalFocus={removalFocus.finalFocus} /> ) : null} diff --git a/packages/mosaic/src/features/organization-profile/__tests__/invitations-table-tab.view.test.tsx b/packages/mosaic/src/features/organization-profile/__tests__/invitations-table-tab.view.test.tsx index 380a03c75a6..75dca2a52fc 100644 --- a/packages/mosaic/src/features/organization-profile/__tests__/invitations-table-tab.view.test.tsx +++ b/packages/mosaic/src/features/organization-profile/__tests__/invitations-table-tab.view.test.tsx @@ -1,7 +1,7 @@ import { act, render, screen, waitFor, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { useState } from 'react'; -import { describe, expect, it, vi } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { deferred } from '../../../__tests__/async'; import { MosaicProvider } from '../../../mosaic-provider'; @@ -33,22 +33,30 @@ function renderView(overrides: Partial = {}) { } describe('InvitationsTableTabView', () => { - it.each(['', ' '])('uses the localized revoke error when the rejection message is "%s"', async message => { - const user = userEvent.setup(); - render( - - - , - ); + afterEach(() => { + vi.restoreAllMocks(); + }); - await user.click(screen.getByRole('button', { name: 'Manage ada@example.com' })); - await user.click(screen.getByRole('menuitem', { name: 'Revoke invitation' })); - await user.click(within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Revoke invitation' })); + it.each(['', 'Cannot read properties of undefined'])( + 'shows the localized revoke error, never the message "%s" of an unexpected error', + async message => { + vi.spyOn(console, 'error').mockImplementation(() => {}); + const user = userEvent.setup(); + render( + + + , + ); - await waitFor(() => expect(screen.getByRole('alert')).toHaveTextContent('Could not revoke invitation.')); - }); + await user.click(screen.getByRole('button', { name: 'Manage ada@example.com' })); + await user.click(screen.getByRole('menuitem', { name: 'Revoke invitation' })); + await user.click(within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Revoke invitation' })); + + await waitFor(() => expect(screen.getByRole('alert')).toHaveTextContent('Could not revoke invitation.')); + }, + ); it('distinguishes loading, an empty invitation list, and an empty search', () => { const { props, rerender } = renderView({ invitations: [], totalCount: 0, isLoading: true }); diff --git a/packages/mosaic/src/features/organization-profile/__tests__/members-table-tab.view.test.tsx b/packages/mosaic/src/features/organization-profile/__tests__/members-table-tab.view.test.tsx index c1e433bd036..1147e0566e5 100644 --- a/packages/mosaic/src/features/organization-profile/__tests__/members-table-tab.view.test.tsx +++ b/packages/mosaic/src/features/organization-profile/__tests__/members-table-tab.view.test.tsx @@ -1,7 +1,7 @@ import { act, render, screen, waitFor, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { useState } from 'react'; -import { describe, expect, it, vi } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { deferred } from '../../../__tests__/async'; import { MosaicProvider } from '../../../mosaic-provider'; @@ -56,6 +56,28 @@ function renderView(overrides: Partial = {}) { } describe('MembersTableTabView', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('shows the localized remove error, never the message of an unexpected error', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}); + const user = userEvent.setup(); + render( + + + , + ); + + await user.click(screen.getByRole('button', { name: 'Manage Grace Hopper' })); + await user.click(screen.getByRole('menuitem', { name: 'Remove from organization' })); + await user.click(within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Remove from organization' })); + + expect(await screen.findByRole('alert')).toHaveTextContent('Could not remove member.'); + }); + it('chooses loading, retained rows, and filtered empty results from the supplied state', () => { const { props, rerender } = renderView({ members: [], totalCount: 0, isLoading: true }); expect(screen.getByRole('status')).toHaveTextContent('Loading members'); diff --git a/packages/mosaic/src/features/organization-profile/__tests__/requests-table-tab.view.test.tsx b/packages/mosaic/src/features/organization-profile/__tests__/requests-table-tab.view.test.tsx index 6cce3f8fa86..64bcc01c62f 100644 --- a/packages/mosaic/src/features/organization-profile/__tests__/requests-table-tab.view.test.tsx +++ b/packages/mosaic/src/features/organization-profile/__tests__/requests-table-tab.view.test.tsx @@ -1,7 +1,7 @@ import { act, render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { useState } from 'react'; -import { describe, expect, it, vi } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { deferred } from '../../../__tests__/async'; import { MosaicProvider } from '../../../mosaic-provider'; @@ -33,9 +33,14 @@ function renderView(overrides: Partial = {}) { } describe('RequestsTableTabView', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + it.each(['Accept', 'Decline'] as const)( 'holds the row while %s is pending and allows retry after failure', async action => { + vi.spyOn(console, 'error').mockImplementation(() => {}); const user = userEvent.setup(); const attempt = deferred(); const onDecision = vi @@ -54,7 +59,7 @@ describe('RequestsTableTabView', () => { expect(onDecision).toHaveBeenCalledExactlyOnceWith('request-1'); expect(onOtherDecision).not.toHaveBeenCalled(); - await act(() => attempt.reject(new Error())); + await act(() => attempt.reject(new Error('Cannot read properties of undefined'))); await waitFor(() => expect(screen.getByRole('alert')).toHaveTextContent( action === 'Accept' diff --git a/packages/mosaic/src/features/organization-profile/invitations-table-tab.view.tsx b/packages/mosaic/src/features/organization-profile/invitations-table-tab.view.tsx index d867ce31c27..5842ea82516 100644 --- a/packages/mosaic/src/features/organization-profile/invitations-table-tab.view.tsx +++ b/packages/mosaic/src/features/organization-profile/invitations-table-tab.view.tsx @@ -244,13 +244,8 @@ export function InvitationsTableTabView({ description={m.revokeDescription} actionLabel={m.revoke} cancelLabel={m.cancel} - onConfirm={async invitation => { - try { - await removalFocus.remove(invitation.id); - } catch (error) { - throw error instanceof Error && error.message.trim() ? error : new Error(m.revokeError); - } - }} + onConfirm={invitation => removalFocus.remove(invitation.id)} + errorFallback={m.revokeError} finalFocus={removalFocus.finalFocus} /> ) : null} diff --git a/packages/mosaic/src/features/organization-profile/members-table-tab.view.tsx b/packages/mosaic/src/features/organization-profile/members-table-tab.view.tsx index a918b2431a9..7b37737444d 100644 --- a/packages/mosaic/src/features/organization-profile/members-table-tab.view.tsx +++ b/packages/mosaic/src/features/organization-profile/members-table-tab.view.tsx @@ -292,13 +292,8 @@ export function MembersTableTabView({ description={m.removeDescription} actionLabel={m.remove} cancelLabel={m.cancel} - onConfirm={async member => { - try { - await removalFocus.remove(member.id); - } catch (error) { - throw error instanceof Error ? error : new Error(m.removeError); - } - }} + onConfirm={member => removalFocus.remove(member.id)} + errorFallback={m.removeError} finalFocus={removalFocus.finalFocus} /> ) : null} diff --git a/packages/mosaic/src/features/organization-profile/organization-profile-profile-section/organization-profile-edit-field.controller.ts b/packages/mosaic/src/features/organization-profile/organization-profile-profile-section/organization-profile-edit-field.controller.ts index dca1ceb55b2..2b46bd3b049 100644 --- a/packages/mosaic/src/features/organization-profile/organization-profile-profile-section/organization-profile-edit-field.controller.ts +++ b/packages/mosaic/src/features/organization-profile/organization-profile-profile-section/organization-profile-edit-field.controller.ts @@ -29,6 +29,7 @@ function isSaveable(context: OrganizationProfileEditFieldContext): boolean { return context.value !== context.savedValue && context.value !== ''; } +// TODO: Move this dialog onto useForm so Clerk errors are localized by code and an unknown error never shows its message. function toFormError(cause: unknown): OrganizationProfileFormError { if (cause instanceof OrganizationProfileSaveError) { return { message: cause.message, field: cause.field }; diff --git a/packages/mosaic/src/features/organization-profile/requests-table-tab.view.tsx b/packages/mosaic/src/features/organization-profile/requests-table-tab.view.tsx index 7a885b8cb99..ee6b3feafdc 100644 --- a/packages/mosaic/src/features/organization-profile/requests-table-tab.view.tsx +++ b/packages/mosaic/src/features/organization-profile/requests-table-tab.view.tsx @@ -11,6 +11,7 @@ import { Section } from '../../components/section'; import { Spinner } from '../../components/spinner'; import { Table, type TableHeaderCellProps } from '../../components/table'; import { VisuallyHidden } from '../../components/visually-hidden'; +import { type Action, useAction } from '../../hooks/use-action'; import { useListRemovalFocus } from '../../hooks/use-list-removal-focus'; import { fill, useMessages } from '../../localization'; import { useDataTable } from '../../primitives/hooks'; @@ -273,28 +274,17 @@ function RequestActions({ const m = useMessages('requestsTableTab'); const [acceptRef] = useState(() => registerAcceptTrigger(request.id)); const [declineRef] = useState(() => registerDeclineTrigger(request.id)); - const [runningAction, setRunningAction] = useState(); - const [error, setError] = useState(); - const pendingAction = request.pendingAction ?? runningAction; - const runAction = async (action: 'accept' | 'decline', run: () => void | Promise) => { + const accept = useAction(() => onAccept?.(), { errorFallback: m.acceptError }); + const decline = useAction(() => onDecline?.(), { errorFallback: m.declineError }); + const pendingAction = + request.pendingAction ?? (accept.isPending ? 'accept' : decline.isPending ? 'decline' : undefined); + const error = accept.errorMessage ?? decline.errorMessage; + const decide = (action: Action<[]>, other: Action<[]>) => { if (pendingAction) { return; } - setRunningAction(action); - setError(undefined); - try { - await run(); - } catch (cause) { - setError( - cause instanceof Error && cause.message.trim() - ? cause.message - : action === 'accept' - ? m.acceptError - : m.declineError, - ); - } finally { - setRunningAction(undefined); - } + other.reset(); + void action.run(); }; return ( <> @@ -310,7 +300,7 @@ function RequestActions({ variant='outline' color='neutral' aria-label={fill(m.declineRequest, { name: request.name ?? request.email })} - onClick={() => void runAction('decline', onDecline)} + onClick={() => decide(decline, accept)} > {m.decline} @@ -324,7 +314,7 @@ function RequestActions({ isPending={pendingAction === 'accept'} pendingLabel={m.accepting} aria-label={fill(m.acceptRequest, { name: request.name ?? request.email })} - onClick={() => void runAction('accept', onAccept)} + onClick={() => decide(accept, decline)} > {m.accept} diff --git a/packages/mosaic/src/features/reverification/reverification.utils.ts b/packages/mosaic/src/features/reverification/reverification.utils.ts index ac089479bec..3aca0d4b86a 100644 --- a/packages/mosaic/src/features/reverification/reverification.utils.ts +++ b/packages/mosaic/src/features/reverification/reverification.utils.ts @@ -8,6 +8,7 @@ import type { ReverificationStrategy, } from './reverification.types'; +// TODO: Return toLocalizableError(error) and render with errorText so Clerk errors are localized by code instead of shown raw. export function errorDetail(error: unknown): string | undefined { if (isClerkAPIResponseError(error)) { const first = error.errors[0]; diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile-connected-accounts.feature.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile-connected-accounts.feature.test.tsx index 1da732402a0..424f6cf75b0 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile-connected-accounts.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile-connected-accounts.feature.test.tsx @@ -272,7 +272,7 @@ describe('connected accounts', () => { await renderWithClerk(); await userEvent.setup().click(screen.getByRole('button', { name: 'Connect GitHub' })); - expect(await screen.findByText('OAuth flow did not receive a verification URL.')).toBeInTheDocument(); + expect(await screen.findByText('The connection could not start. Please try again.')).toBeInTheDocument(); }); it('preserves modal state when connecting', async () => { diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile-danger-section.integration.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile-danger-section.integration.test.tsx index f9ecc752971..fe64d963b70 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile-danger-section.integration.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile-danger-section.integration.test.tsx @@ -4,6 +4,7 @@ import { render, screen, waitFor, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { clerkApiError } from '../../../__tests__/clerk-errors'; import { MosaicProvider } from '../../../mosaic-provider'; import { UserProfileDangerSection } from '../user-profile-danger-section/user-profile-danger-section'; @@ -119,7 +120,9 @@ describe('UserProfileDangerSection', () => { }); it('keeps the dialog up with a message when the delete fails', async () => { - deleteUser = vi.fn(() => Promise.reject(new Error('Your subscription is still active.'))); + deleteUser = vi.fn(() => + Promise.reject(clerkApiError('subscription_active', 'Your subscription is still active.')), + ); const user = userEvent.setup(); renderSection(); const dialog = await openDialog(user); @@ -127,7 +130,7 @@ describe('UserProfileDangerSection', () => { await user.type(within(dialog).getByRole('textbox'), 'Delete account'); await user.click(within(dialog).getByRole('button', { name: 'Delete account' })); - expect(await screen.findByText('Something went wrong. Please try again.')).toBeInTheDocument(); + expect(await screen.findByText('Your subscription is still active.')).toBeInTheDocument(); expect(screen.getByRole('dialog')).toBeInTheDocument(); expect(setActive).not.toHaveBeenCalled(); }); diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile-device-actions.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile-device-actions.test.tsx index 4b9fb1a540b..5f5ea410364 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile-device-actions.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile-device-actions.test.tsx @@ -2,8 +2,9 @@ import { createDeferredPromise } from '@clerk/shared/utils'; import { act, render, screen, waitFor, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { useState } from 'react'; -import { describe, expect, it, vi } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { clerkApiError } from '../../../__tests__/clerk-errors'; import { MosaicProvider } from '../../../mosaic-provider'; import type { UserProfileDevice } from '../user-profile-active-devices.types'; import { UserProfileActiveDevicesSectionView } from '../user-profile-active-devices-section.view'; @@ -44,6 +45,10 @@ async function openMenu(user: ReturnType, device: UserPr await user.click(screen.getByRole('button', { name: `Manage ${device.name}` })); } +afterEach(() => { + vi.restoreAllMocks(); +}); + describe('active device details', () => { it('opens the details of the device the menu belongs to', async () => { const user = userEvent.setup(); @@ -99,14 +104,22 @@ describe('active device sign out', () => { }); it('shows a failure in the confirmation and allows retrying', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}); const user = userEvent.setup(); - const onSignOutDevice = vi.fn().mockRejectedValueOnce(new Error('Unable to sign out')).mockResolvedValue(undefined); + const onSignOutDevice = vi + .fn() + .mockRejectedValueOnce(new Error('Cannot read properties of undefined')) + .mockResolvedValue(undefined); renderDevices(onSignOutDevice); await openMenu(user, mobile); await user.click(screen.getByRole('menuitem', { name: 'Sign out' })); await user.click(within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Sign out' })); - await waitFor(() => expect(screen.getByRole('alert')).toHaveTextContent('Unable to sign out')); + await waitFor(() => + expect(screen.getByRole('alert')).toHaveTextContent( + 'Something went wrong signing this device out. Please try again.', + ), + ); await user.click(within(screen.getByRole('alertdialog')).getByRole('button', { name: 'Sign out' })); await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()); @@ -157,7 +170,10 @@ describe('active device sign out', () => { it('keeps the details dialog open and explains a failed sign out', async () => { const user = userEvent.setup(); - const onSignOutDevice = vi.fn().mockRejectedValueOnce(new Error('Unable to sign out')).mockResolvedValue(undefined); + const onSignOutDevice = vi + .fn() + .mockRejectedValueOnce(clerkApiError('session_not_found', 'Unable to sign out')) + .mockResolvedValue(undefined); renderDevices(onSignOutDevice); await openMenu(user, mobile); await user.click(screen.getByRole('menuitem', { name: 'View details' })); @@ -213,16 +229,21 @@ describe('signing out of all other devices', () => { }); it('holds the confirmation open and explains a failure', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}); const user = userEvent.setup(); const onSignOutAllOtherDevices = vi .fn() - .mockRejectedValueOnce(new Error('Unable to sign out of all devices')) + .mockRejectedValueOnce(new Error('Cannot read properties of undefined')) .mockResolvedValue(undefined); renderAll(onSignOutAllOtherDevices); await user.click(screen.getByRole('button', { name: 'Sign out of all devices' })); await user.click(within(confirmation()).getByRole('button', { name: 'Sign out' })); - expect(await screen.findByText('Unable to sign out of all devices')).toBeInTheDocument(); + await waitFor(() => + expect(screen.getByRole('alert')).toHaveTextContent( + 'Something went wrong signing these devices out. Please try again.', + ), + ); expect(confirmation()).toBeInTheDocument(); await user.click(within(confirmation()).getByRole('button', { name: 'Sign out' })); diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile-email-actions.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile-email-actions.test.tsx index 72090f2a4db..26d58ac7e24 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile-email-actions.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile-email-actions.test.tsx @@ -4,6 +4,7 @@ import userEvent from '@testing-library/user-event'; import { useState } from 'react'; import { describe, expect, it, vi } from 'vitest'; +import { clerkApiError } from '../../../__tests__/clerk-errors'; import { MosaicProvider } from '../../../mosaic-provider'; import type { UserProfileAccountSectionViewProps } from '../user-profile-account-section/user-profile-account-section.view'; import { UserProfileAccountSectionView } from '../user-profile-account-section/user-profile-account-section.view'; @@ -96,7 +97,7 @@ describe('email actions', () => { expect(within(dialog).getByRole('button', { name: 'Remove' })).toHaveAttribute('aria-busy', 'true'); await act(async () => { - removal.reject(new Error('Unable to remove email.')); + removal.reject(clerkApiError('email_address_in_use', 'Unable to remove email.')); await removal.promise.catch(() => undefined); }); expect(within(dialog).getByRole('alert')).toHaveTextContent('Unable to remove email.'); diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile-enterprise-accounts.feature.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile-enterprise-accounts.feature.test.tsx index 5785dc4f4ee..4cb3e4da970 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile-enterprise-accounts.feature.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile-enterprise-accounts.feature.test.tsx @@ -26,7 +26,7 @@ async function renderSection(seed: FakeFapiSeed = signedIn()) { } describe('enterprise accounts', () => { - it('preserves the original cause when localizing a linking failure', async () => { + it('propagates a linking failure untouched', async () => { const feedback = vi.fn<(error: Error) => void>(); function Connect() { const model = useUserProfileEnterpriseAccountsModel(); @@ -49,7 +49,7 @@ describe('enterprise accounts', () => { vi.spyOn(user, 'createExternalAccount').mockRejectedValue(cause); await userEvent.setup().click(await screen.findByRole('button', { name: 'Connect' })); await waitFor(() => expect(feedback).toHaveBeenCalledOnce()); - expect(feedback.mock.calls[0]?.[0].cause).toBe(cause); + expect(feedback).toHaveBeenCalledWith(cause); }); it('shows the fallback while Clerk loads', async () => { @@ -338,7 +338,7 @@ describe('enterprise accounts', () => { localization={{ locale: 'fr-FR', overrides: { - 'userProfileEnterpriseAccountsSection.errors.missingVerificationUrl': 'La connexion ne peut pas démarrer.', + 'errors.oauth_missing_verification_url': 'La connexion ne peut pas démarrer.', }, }} > diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile-mfa-section.view.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile-mfa-section.view.test.tsx index f6f04e0f0c9..2036628b25b 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile-mfa-section.view.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile-mfa-section.view.test.tsx @@ -1,9 +1,10 @@ import { act, render, screen, waitFor, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { useState } from 'react'; -import { describe, expect, it, vi } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { deferred } from '../../../__tests__/async'; +import { clerkApiError } from '../../../__tests__/clerk-errors'; import { MosaicProvider } from '../../../mosaic-provider'; import type { UserProfileMfaMethod, UserProfileMfaSectionViewProps } from '../user-profile-mfa-section.view'; import { UserProfileMfaSectionView } from '../user-profile-mfa-section.view'; @@ -28,6 +29,10 @@ function renderView(overrides: Partial = {}) { }; } +afterEach(() => { + vi.restoreAllMocks(); +}); + describe('MFA section', () => { it.each(['picker', 'custom', 'none'] as const)( 'restores focus after removing methods with %s add control', @@ -170,9 +175,17 @@ describe('MFA section', () => { }); it.each([ - { cause: new Error('Unable to update the default method.'), message: 'Unable to update the default method.' }, + { + cause: clerkApiError('phone_number_not_verified', 'Unable to update the default method.'), + message: 'Unable to update the default method.', + }, + { + cause: new Error('Cannot read properties of undefined'), + message: 'Unable to set this method as default. Please try again.', + }, { cause: 'network failure', message: 'Unable to set this method as default. Please try again.' }, ])('shows a default-change error and clears it on retry: $message', async ({ cause, message }) => { + vi.spyOn(console, 'error').mockImplementation(() => {}); const user = userEvent.setup(); const retry = deferred(); const onSetDefault = vi.fn().mockRejectedValueOnce(cause).mockReturnValueOnce(retry.promise); diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile-passkeys-section.view.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile-passkeys-section.view.test.tsx index 560932c7fcd..d154b369b01 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile-passkeys-section.view.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile-passkeys-section.view.test.tsx @@ -1,12 +1,17 @@ import { act, render, screen, waitFor, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { useState } from 'react'; -import { describe, expect, it, vi } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { clerkApiError } from '../../../__tests__/clerk-errors'; import { MosaicProvider } from '../../../mosaic-provider'; import type { UserProfilePasskey, UserProfilePasskeysSectionViewProps } from '../user-profile-passkeys-section.view'; import { UserProfilePasskeysSectionView } from '../user-profile-passkeys-section.view'; +afterEach(() => { + vi.restoreAllMocks(); +}); + const passkeys: UserProfilePasskey[] = [ { id: 'laptop', name: 'MacBook' }, { id: 'phone', name: 'iPhone' }, @@ -91,7 +96,10 @@ describe('passkeys section', () => { it('renames through a prefilled form, preserves the draft after failure, and retries', async () => { const user = userEvent.setup(); - const onRename = vi.fn().mockRejectedValueOnce(new Error('Try again')).mockResolvedValueOnce(undefined); + const onRename = vi + .fn() + .mockRejectedValueOnce(clerkApiError('passkey_rename_failed', 'Try again')) + .mockResolvedValueOnce(undefined); render( { await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); }); + it('shows the rename fallback, never the message, when renaming fails unexpectedly', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}); + const user = userEvent.setup(); + renderView({ onRename: vi.fn().mockRejectedValue(new Error('Cannot read properties of undefined')) }); + await user.click(screen.getByRole('button', { name: 'Manage MacBook' })); + await user.click(screen.getByRole('menuitem', { name: 'Rename' })); + await user.type(screen.getByRole('textbox', { name: 'Passkey name' }), ' Pro'); + await user.click(screen.getByRole('button', { name: 'Save' })); + expect(await screen.findByRole('alert')).toHaveTextContent('Something went wrong. Please try again.'); + }); + it('keeps an empty section visible when Add is unavailable', () => { renderView({ passkeys: [], onAdd: undefined }); @@ -185,7 +204,10 @@ describe('passkeys section', () => { it('retries removal for the same passkey after a failure', async () => { const user = userEvent.setup(); - const onRemove = vi.fn().mockRejectedValueOnce(new Error('Removal failed')).mockResolvedValueOnce(undefined); + const onRemove = vi + .fn() + .mockRejectedValueOnce(clerkApiError('passkey_not_found', 'Removal failed')) + .mockResolvedValueOnce(undefined); render( { const user = userEvent.setup(); const onRemovePhone = vi .fn() - .mockRejectedValueOnce(new Error('Cannot remove this phone.')) + .mockRejectedValueOnce(clerkApiError('phone_number_in_use', 'Cannot remove this phone.')) .mockResolvedValue(undefined); renderPhone({ onRemovePhone }); await user.click(screen.getByRole('button', { name: 'Manage +1 (801) 555-0100' })); diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile-web3-actions.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile-web3-actions.test.tsx index e1b68f4007b..cbc866af4ee 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile-web3-actions.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile-web3-actions.test.tsx @@ -4,6 +4,7 @@ import userEvent from '@testing-library/user-event'; import { useState } from 'react'; import { describe, expect, it, vi } from 'vitest'; +import { clerkApiError } from '../../../__tests__/clerk-errors'; import { MosaicProvider } from '../../../mosaic-provider'; import { UserProfileWeb3WalletsSectionView } from '../user-profile-web3-wallets-section.view'; @@ -86,7 +87,7 @@ describe('Web3 wallet removal', () => { await user.click(remove); expect(onRemove).toHaveBeenCalledOnce(); await act(async () => { - removal.reject(new Error('Unable to remove wallet')); + removal.reject(clerkApiError('web3_wallet_in_use', 'Unable to remove wallet')); await removal.promise.catch(() => undefined); }); expect(within(dialog).getByRole('alert')).toHaveTextContent('Unable to remove wallet'); diff --git a/packages/mosaic/src/features/user-profile/user-profile-active-devices-section.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-active-devices-section.view.tsx index ced3c9815ec..259e385cdf5 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-active-devices-section.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-active-devices-section.view.tsx @@ -1,8 +1,9 @@ import * as stylex from '@stylexjs/stylex'; import type { Ref } from 'react'; -import { useMemo, useRef, useState } from 'react'; +import { useMemo, useRef } from 'react'; import { Confirmation } from '../../blocks/confirmation'; +import { useConfirmationController } from '../../blocks/confirmation/confirmation.controller'; import type { ActionMenuAction } from '../../components/action-menu'; import { ActionMenu } from '../../components/action-menu'; import { Badge } from '../../components/badge'; @@ -47,32 +48,10 @@ export function UserProfileActiveDevicesSectionView({ }); const signOutDeviceAt = onSignOutDevice ? (device: UserProfileDevice) => removalFocus.remove(device.id) : undefined; - const [isSignOutAllOpen, setIsSignOutAllOpen] = useState(false); - const [isSigningOutAll, setIsSigningOutAll] = useState(false); - const [signOutAllError, setSignOutAllError] = useState(); - const signingOutAll = useRef(false); + const signOutAll = useConfirmationController({ errorFallback: m.signOutAllError }); const signedOutAll = useRef(false); const signOutAllTrigger = useRef(null); - const signOutAllOtherDevices = async () => { - if (!onSignOutAllOtherDevices || signingOutAll.current) { - return; - } - signingOutAll.current = true; - setIsSigningOutAll(true); - setSignOutAllError(undefined); - try { - await onSignOutAllOtherDevices(); - signedOutAll.current = true; - setIsSignOutAllOpen(false); - } catch (error) { - setSignOutAllError(error instanceof Error ? error.message : m.signOutAllError); - } finally { - signingOutAll.current = false; - setIsSigningOutAll(false); - } - }; - // Confirming removes the other devices and the trigger with them — hence the dialog mounted // outside the card, and the current device as the place focus lands. Cancelling keeps the // trigger, so focus goes back to it. @@ -95,7 +74,7 @@ export function UserProfileActiveDevicesSectionView({ color='neutral' size='sm' variant='outline' - onClick={() => setIsSignOutAllOpen(true)} + onClick={() => signOutAll.onOpenChange(true)} > {m.signOutAll} @@ -135,22 +114,22 @@ export function UserProfileActiveDevicesSectionView({ {onSignOutAllOtherDevices ? ( { - setIsSignOutAllOpen(open); - if (!open) { - setSignOutAllError(undefined); - } - }} + open={signOutAll.isOpen} + onOpenChange={signOutAll.onOpenChange} color='primary' finalFocus={focusAfterSignOutAll} title={m.signOutAllDialog.title} description={plural(m.signOutAllDialog.description, otherDevices.length, locale)} actionLabel={m.signOutAllDialog.confirm} cancelLabel={m.signOutAllDialog.cancel} - onConfirm={() => void signOutAllOtherDevices()} - isConfirming={isSigningOutAll} - errorMessage={signOutAllError} + onConfirm={() => + signOutAll.onConfirm(async () => { + await onSignOutAllOtherDevices(); + signedOutAll.current = true; + }) + } + isConfirming={signOutAll.isConfirming} + errorMessage={signOutAll.errorMessage} /> ) : null} signOutDeviceAt?.(device)} + errorFallback={m.detailsDialog.signOutError} /> ) : null} diff --git a/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-feedback.test.ts b/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-feedback.test.ts deleted file mode 100644 index 3e709579a2f..00000000000 --- a/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-feedback.test.ts +++ /dev/null @@ -1,75 +0,0 @@ -import { ClerkAPIResponseError } from '@clerk/shared/error'; -import { describe, expect, it } from 'vitest'; - -import { resolveLocalization } from '../../../localization'; -import { connectedAccountFeedback } from './user-profile-connected-accounts-feedback'; -import { ConnectedAccountActionError } from './user-profile-connected-accounts-section.types'; - -const errorText = ({ message }: { message?: string }) => message ?? 'Fallback'; -const messages = resolveLocalization({ locale: 'en' }).messages.userProfileConnectedAccounts; - -function apiError(data: ConstructorParameters[1]['data']) { - return new ClerkAPIResponseError('Invalid', { status: 422, data }); -} - -describe('connected account error messages', () => { - it('retains the internal action code and cause in localized feedback', () => { - const cause = new ConnectedAccountActionError('unavailable'); - const feedback = connectedAccountFeedback(cause, messages, errorText); - expect(feedback).toBeInstanceOf(ConnectedAccountActionError); - expect(feedback.cause).toBe(cause); - expect(feedback).toMatchObject({ code: 'unavailable', cause, message: messages.errors.unavailable }); - }); - - it('localizes errors raised by the model', () => { - expect(connectedAccountFeedback(new ConnectedAccountActionError('unavailable'), messages, errorText).message).toBe( - messages.errors.unavailable, - ); - expect( - connectedAccountFeedback(new ConnectedAccountActionError('missing_verification_url'), messages, errorText) - .message, - ).toBe(messages.errors.missingVerificationUrl); - }); - - it('shows the first API error, preferring its long message', () => { - expect( - connectedAccountFeedback( - apiError([ - { code: 'oauth_error', message: 'Short', long_message: 'GitHub is unavailable right now.' }, - { code: 'other', message: 'Second' }, - ]), - messages, - errorText, - ).message, - ).toBe('GitHub is unavailable right now.'); - expect( - connectedAccountFeedback(apiError([{ code: 'oauth_error', message: 'Short' }]), messages, errorText).message, - ).toBe('Short'); - }); - - it('passes the API code, parameter, and fallback copy to the canonical formatter', () => { - expect( - connectedAccountFeedback( - apiError([ - { code: 'form_param_invalid', message: 'Short', long_message: 'Long', meta: { param_name: 'strategy' } }, - ]), - messages, - ({ code, paramName, message }) => `${code}:${paramName}:${message}`, - ).message, - ).toBe('form_param_invalid:strategy:Long'); - }); - - it('falls back to the generic message', () => { - expect(connectedAccountFeedback(apiError([]), messages, errorText).message).toBe(messages.errors.generic); - expect(connectedAccountFeedback(new Error('Popup closed'), messages, errorText).message).toBe( - messages.errors.generic, - ); - }); - - it('preserves an empty API message and its cause', () => { - const cause = apiError([{ code: 'unknown', message: '', long_message: '' }]); - const feedback = connectedAccountFeedback(cause, messages, errorText); - expect(feedback.message).toBe(''); - expect(feedback.cause).toBe(cause); - }); -}); diff --git a/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-feedback.ts b/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-feedback.ts deleted file mode 100644 index e4cef35f9af..00000000000 --- a/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-feedback.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { isClerkAPIResponseError } from '@clerk/shared/error'; - -import type { LocalizableError, MosaicMessages } from '../../../localization'; -import { toLocalizableApiError } from '../../../localization'; -import { ConnectedAccountActionError } from './user-profile-connected-accounts-section.types'; - -type Messages = MosaicMessages['userProfileConnectedAccounts']; - -export function connectedAccountFeedback( - error: unknown, - messages: Messages, - errorText: (error: LocalizableError) => string, -): Error { - if (error instanceof ConnectedAccountActionError) { - const message = - error.code === 'missing_verification_url' ? messages.errors.missingVerificationUrl : messages.errors.unavailable; - return new ConnectedAccountActionError(error.code, message, { cause: error }); - } - let message: string = messages.errors.generic; - if (isClerkAPIResponseError(error)) { - const first = error.errors[0]; - if (first) { - message = errorText(toLocalizableApiError(first)); - } - } - return new Error(message, { cause: error }); -} diff --git a/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.controller.ts index 588b29e4b4a..055b03ab9bd 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.controller.ts @@ -1,6 +1,8 @@ import { useRef, useState } from 'react'; -import { useMessages } from '../../../localization'; +import type { LocalizableError } from '../../../localization'; +import { useErrorText, useMessages } from '../../../localization'; +import { toLocalizableError } from '../../../utils/form-error'; import type { ConnectedAccountActionResult, UserProfileConnectedAccount, @@ -31,8 +33,9 @@ export function useUserProfileConnectedAccountsController({ onReconnect, }: UserProfileConnectedAccountsControllerOptions): UserProfileConnectedAccountsController { const messages = useMessages('userProfileConnectedAccounts'); + const errorText = useErrorText(); const [pendingId, setPendingId] = useState(); - const [errors, setErrors] = useState>({}); + const [errors, setErrors] = useState>({}); const connecting = useRef(false); const run = async (id: string, action: (id: string) => Promise) => { @@ -48,20 +51,27 @@ export function useUserProfileConnectedAccountsController({ await new Promise(resolve => setTimeout(resolve, OAUTH_REDIRECT_HOLD_MS)); } } catch (error) { - setErrors(current => ({ ...current, [id]: error instanceof Error ? error.message : messages.errors.generic })); + setErrors(current => ({ ...current, [id]: toLocalizableError(error) })); } finally { connecting.current = false; setPendingId(undefined); } }; + const errorFor = (id: string) => { + const error = errors[id]; + return error ? errorText(error, messages.errors.generic) : undefined; + }; + return { - accounts: accounts.map(account => - errors[account.id] ? { ...account, reconnectError: errors[account.id] } : account, - ), - availableProviders: availableProviders.map(provider => - errors[provider.id] ? { ...provider, connectError: errors[provider.id] } : provider, - ), + accounts: accounts.map(account => { + const reconnectError = errorFor(account.id); + return reconnectError ? { ...account, reconnectError } : account; + }), + availableProviders: availableProviders.map(provider => { + const connectError = errorFor(provider.id); + return connectError ? { ...provider, connectError } : provider; + }), pendingId, onConnect: id => void run(id, onConnect), onReconnect: id => void run(id, onReconnect), diff --git a/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.messages.ts b/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.messages.ts index c181f31860b..33e30c792ec 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.messages.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.messages.ts @@ -16,7 +16,5 @@ export const userProfileConnectedAccountsMessages = { }, errors: { generic: 'Something went wrong. Please try again.', - unavailable: 'This connected account is no longer available.', - missingVerificationUrl: 'OAuth flow did not receive a verification URL.', }, } as const; diff --git a/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.model.ts b/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.model.ts index 60970f8f377..dc88339dc31 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.model.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.model.ts @@ -1,4 +1,5 @@ import { iconImageUrl } from '@clerk/shared/constants'; +import { ClerkRuntimeError } from '@clerk/shared/error'; import { appendModalState } from '@clerk/shared/internal/clerk-js/queryStateParams'; import { OAUTH_PROVIDERS } from '@clerk/shared/oauth'; import { useClerk, useUser } from '@clerk/shared/react'; @@ -18,15 +19,12 @@ import type { import { useMosaicEnvironment } from '../../../hooks/use-mosaic-environment'; import { useMosaicRouter } from '../../../hooks/use-mosaic-router'; -import { useErrorText, useMessages } from '../../../localization'; -import { connectedAccountFeedback } from './user-profile-connected-accounts-feedback'; import type { ConnectedAccountActionResult, ConnectedAccountProviderDisplay, UserProfileConnectedAccount, UserProfileConnectionProvider, } from './user-profile-connected-accounts-section.types'; -import { ConnectedAccountActionError } from './user-profile-connected-accounts-section.types'; type AccountData = Pick & { verification: @@ -245,8 +243,6 @@ export function useUserProfileConnectedAccountsModel({ const { isLoaded, user } = useUser(); const environment = useMosaicEnvironment(); const router = useMosaicRouter(); - const messages = useMessages('userProfileConnectedAccounts'); - const errorText = useErrorText(); if (!isLoaded || !environment) { return { status: 'loading' }; @@ -274,10 +270,13 @@ export function useUserProfileConnectedAccountsModel({ const userId = user.id; const transport = clerk.__internal_oauthTransport; + const unavailable = () => + new ClerkRuntimeError('This connected account is no longer available.', { code: 'connected_account_unavailable' }); + const requireCurrentUser = () => { const current = clerk.user; if (!current || current.id !== userId) { - throw new ConnectedAccountActionError('unavailable'); + throw unavailable(); } return current; }; @@ -289,7 +288,9 @@ export function useUserProfileConnectedAccountsModel({ const completeVerification = async (response: ExternalAccountResource): Promise => { const url = response.verification?.externalVerificationRedirectURL; if (!url) { - throw new ConnectedAccountActionError('missing_verification_url'); + throw new ClerkRuntimeError('OAuth flow did not receive a verification URL.', { + code: 'oauth_missing_verification_url', + }); } requireCurrentUser(); @@ -305,65 +306,54 @@ export function useUserProfileConnectedAccountsModel({ return 'redirecting'; }; - async function runAction(action: () => Promise): Promise { - try { - return await action(); - } catch (error) { - throw connectedAccountFeedback(error, messages, errorText); - } - } - return { ...projection, userId, - connect: strategyId => - runAction(async () => { - const provider = providers.find(candidate => candidate.enabled && candidate.strategy === strategyId); - if (!provider) { - throw new ConnectedAccountActionError('unavailable'); - } - const redirectUrl = await getRedirectUrl(); - const current = requireCurrentUser(); - const response = await current.createExternalAccount({ - strategy: provider.strategy, - redirectUrl: withModalState(redirectUrl, provider.provider), - additionalScopes: additionalOAuthScopes ? additionalOAuthScopes[provider.provider] : [], - }); - return completeVerification(response); - }), - reconnect: accountId => - runAction(async () => { - const redirectUrl = await getRedirectUrl(); - const current = requireCurrentUser(); - const account = current.externalAccounts.find(candidate => candidate.id === accountId); - if (!account) { - throw new ConnectedAccountActionError('unavailable'); - } - const { plan: recovery } = recoveryFor(account, additionalOAuthScopes, providers); - if (!recovery) { - throw new ConnectedAccountActionError('unavailable'); - } + connect: async strategyId => { + const provider = providers.find(candidate => candidate.enabled && candidate.strategy === strategyId); + if (!provider) { + throw unavailable(); + } + const redirectUrl = await getRedirectUrl(); + const current = requireCurrentUser(); + const response = await current.createExternalAccount({ + strategy: provider.strategy, + redirectUrl: withModalState(redirectUrl, provider.provider), + additionalScopes: additionalOAuthScopes ? additionalOAuthScopes[provider.provider] : [], + }); + return completeVerification(response); + }, + reconnect: async accountId => { + const redirectUrl = await getRedirectUrl(); + const current = requireCurrentUser(); + const account = current.externalAccounts.find(candidate => candidate.id === accountId); + if (!account) { + throw unavailable(); + } + const { plan: recovery } = recoveryFor(account, additionalOAuthScopes, providers); + if (!recovery) { + throw unavailable(); + } - const response = - recovery.kind === 'reauthorize' - ? await account.reauthorize({ - additionalScopes: recovery.additionalScopes, - redirectUrl: withModalState(redirectUrl), - }) - : await current.createExternalAccount({ - strategy: recovery.strategy, - redirectUrl: withModalState(redirectUrl), - additionalScopes: recovery.additionalScopes, - }); - return completeVerification(response); - }), - remove: accountId => - runAction(async () => { - const account = requireCurrentUser().externalAccounts.find(candidate => candidate.id === accountId); - if (!account) { - throw new ConnectedAccountActionError('unavailable'); - } - await account.destroy(); - }), + const response = + recovery.kind === 'reauthorize' + ? await account.reauthorize({ + additionalScopes: recovery.additionalScopes, + redirectUrl: withModalState(redirectUrl), + }) + : await current.createExternalAccount({ + strategy: recovery.strategy, + redirectUrl: withModalState(redirectUrl), + additionalScopes: recovery.additionalScopes, + }); + return completeVerification(response); + }, + remove: async accountId => { + const account = requireCurrentUser().externalAccounts.find(candidate => candidate.id === accountId); + if (!account) { + throw unavailable(); + } + await account.destroy(); + }, }; } diff --git a/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.types.ts b/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.types.ts index 276150261f0..adcd5ffc3d5 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.types.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.types.ts @@ -28,14 +28,3 @@ export interface UserProfileConnectedAccountsSectionViewProps { onReconnect?: (id: string) => void; onRemove?: (id: string) => void | Promise; } - -export class ConnectedAccountActionError extends Error { - constructor( - readonly code: 'unavailable' | 'missing_verification_url', - message: string = code, - options?: ErrorOptions, - ) { - super(message, options); - this.name = 'ConnectedAccountActionError'; - } -} diff --git a/packages/mosaic/src/features/user-profile/user-profile-device-details.dialog.tsx b/packages/mosaic/src/features/user-profile/user-profile-device-details.dialog.tsx index 8d050f0939b..5ef1b02eb3d 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-device-details.dialog.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-device-details.dialog.tsx @@ -1,10 +1,9 @@ -import { useRef, useState } from 'react'; - import { Button, SubmitButton } from '../../components/button'; import { Card } from '../../components/card'; import { DataList } from '../../components/data-list'; import type { DialogFocusTarget, DialogHandle } from '../../components/dialog'; import { Dialog } from '../../components/dialog'; +import { useAction } from '../../hooks/use-action'; import { fill, useMessages } from '../../localization'; import type { UserProfileDevice } from './user-profile-active-devices.types'; @@ -45,27 +44,13 @@ function DeviceDetailsCard({ onSignOut: UserProfileDeviceDetailsDialogProps['onSignOut']; }) { const m = useMessages('userProfileActiveDevices'); - const [isSigningOut, setIsSigningOut] = useState(false); - const [errorMessage, setErrorMessage] = useState(); - const signingOut = useRef(false); - - const signOut = async () => { - if (!onSignOut || signingOut.current) { - return; - } - signingOut.current = true; - setIsSigningOut(true); - setErrorMessage(undefined); - try { - await onSignOut(device); + const signOut = useAction( + async () => { + await onSignOut?.(device); handle.close(); - } catch (error) { - setErrorMessage(error instanceof Error ? error.message : m.detailsDialog.signOutError); - } finally { - signingOut.current = false; - setIsSigningOut(false); - } - }; + }, + { errorFallback: m.detailsDialog.signOutError }, + ); const fields: { label: string; value: string | undefined }[] = [ { label: m.detailsDialog.model, value: device.model }, @@ -91,7 +76,7 @@ function DeviceDetailsCard({ role='alert' color='negative' > - {errorMessage} + {signOut.errorMessage} {details.length > 0 ? ( @@ -110,8 +95,8 @@ function DeviceDetailsCard({ void signOut()} + isPending={signOut.isPending} + onClick={() => void signOut.run()} > {m.detailsDialog.signOut} diff --git a/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-feedback.test.ts b/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-feedback.test.ts deleted file mode 100644 index adf4d171c62..00000000000 --- a/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-feedback.test.ts +++ /dev/null @@ -1,49 +0,0 @@ -import { ClerkAPIResponseError } from '@clerk/shared/error'; -import { describe, expect, it } from 'vitest'; - -import { resolveLocalization } from '../../../localization'; -import { enterpriseAccountErrorMessage } from './user-profile-enterprise-accounts-feedback'; - -const messages = resolveLocalization({ locale: 'en' }).messages.userProfileEnterpriseAccountsSection; -const errorText = ({ message }: { message?: string }) => message || messages.errors.generic; - -function apiError(data: ConstructorParameters[1]['data']) { - return new ClerkAPIResponseError('Invalid', { status: 422, data }); -} - -describe('enterprise account error messages', () => { - it('shows the first API error, preferring its long message', () => { - expect( - enterpriseAccountErrorMessage( - apiError([ - { code: 'oauth_error', message: 'Short', long_message: 'The identity provider is unavailable.' }, - { code: 'other', message: 'Second' }, - ]), - errorText, - messages.errors.generic, - ), - ).toBe('The identity provider is unavailable.'); - expect( - enterpriseAccountErrorMessage( - apiError([{ code: 'oauth_error', message: 'Short' }]), - errorText, - messages.errors.generic, - ), - ).toBe('Short'); - }); - - it('falls back to the generic message', () => { - expect(enterpriseAccountErrorMessage(apiError([]), errorText, messages.errors.generic)).toBe( - messages.errors.generic, - ); - expect(enterpriseAccountErrorMessage(new TypeError('Failed to fetch'), errorText, messages.errors.generic)).toBe( - messages.errors.generic, - ); - }); - - it('uses the section fallback for empty API messages', () => { - expect( - enterpriseAccountErrorMessage(apiError([{ code: 'unknown', message: '', long_message: '' }]), errorText, 'Retry'), - ).toBe('Retry'); - }); -}); diff --git a/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-feedback.ts b/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-feedback.ts deleted file mode 100644 index 64a39361910..00000000000 --- a/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-feedback.ts +++ /dev/null @@ -1,16 +0,0 @@ -import { isClerkAPIResponseError } from '@clerk/shared/error'; - -import type { LocalizableError } from '../../../localization'; -import { toLocalizableApiError } from '../../../localization'; - -export function enterpriseAccountErrorMessage( - error: unknown, - errorText: (error: LocalizableError) => string, - fallback: string, -): string { - if (isClerkAPIResponseError(error)) { - const first = error.errors[0]; - return first ? errorText(toLocalizableApiError(first, fallback)) : fallback; - } - return fallback; -} diff --git a/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.controller.ts index b53c19e4a3f..ce016ddf28c 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.controller.ts @@ -1,5 +1,8 @@ import { useRef, useState } from 'react'; +import type { LocalizableError } from '../../../localization'; +import { useErrorText } from '../../../localization'; +import { toLocalizableError } from '../../../utils/form-error'; import type { EnterpriseAccountActionResult, UserProfileEnterpriseAccount, @@ -10,7 +13,7 @@ export interface UserProfileEnterpriseAccountsControllerOptions { accounts: UserProfileEnterpriseAccount[]; connections: UserProfileEnterpriseConnection[]; onConnect: (connectionId: string) => Promise; - errorMessage: string; + errorFallback: string; } export interface UserProfileEnterpriseAccountsController { @@ -24,10 +27,11 @@ export function useUserProfileEnterpriseAccountsController({ accounts, connections, onConnect, - errorMessage, + errorFallback, }: UserProfileEnterpriseAccountsControllerOptions): UserProfileEnterpriseAccountsController { const [pendingId, setPendingId] = useState(); - const [connectErrors, setConnectErrors] = useState>({}); + const errorText = useErrorText(); + const [connectErrors, setConnectErrors] = useState>({}); const connecting = useRef(false); const run = async (id: string) => { @@ -43,7 +47,7 @@ export function useUserProfileEnterpriseAccountsController({ await new Promise(resolve => setTimeout(resolve, 2000)); } } catch (error) { - setConnectErrors(current => ({ ...current, [id]: error instanceof Error ? error.message : errorMessage })); + setConnectErrors(current => ({ ...current, [id]: toLocalizableError(error) })); } finally { connecting.current = false; setPendingId(undefined); @@ -52,9 +56,10 @@ export function useUserProfileEnterpriseAccountsController({ return { accounts, - connections: connections.map(connection => - connectErrors[connection.id] ? { ...connection, connectError: connectErrors[connection.id] } : connection, - ), + connections: connections.map(connection => { + const error = connectErrors[connection.id]; + return error ? { ...connection, connectError: errorText(error, errorFallback) } : connection; + }), pendingId, onConnect: id => void run(id), }; diff --git a/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.messages.ts b/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.messages.ts index 23125b9a5c0..0fbf1d5ee31 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.messages.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.messages.ts @@ -5,7 +5,5 @@ export const userProfileEnterpriseAccountsMessages = { requiresAction: 'Requires action', errors: { generic: 'Something went wrong. Please try again.', - unavailable: 'This enterprise connection is no longer available.', - missingVerificationUrl: 'The connection could not start. Please try again.', }, } as const; diff --git a/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.model.ts b/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.model.ts index dfb1ef1dcad..00dd76a2054 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.model.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.model.ts @@ -1,11 +1,10 @@ +import { ClerkRuntimeError } from '@clerk/shared/error'; import { appendModalState } from '@clerk/shared/internal/clerk-js/queryStateParams'; import { __internal_useUserEnterpriseConnections, useClerk, useUser } from '@clerk/shared/react'; import type { EnterpriseAccountResource, EnterpriseConnectionResource } from '@clerk/shared/types'; import { useMosaicEnvironment } from '../../../hooks/use-mosaic-environment'; import { useMosaicRouter } from '../../../hooks/use-mosaic-router'; -import { useErrorText, useMessages } from '../../../localization'; -import { enterpriseAccountErrorMessage } from './user-profile-enterprise-accounts-feedback'; import type { EnterpriseAccountActionResult, UserProfileEnterpriseAccount, @@ -77,8 +76,6 @@ export function useUserProfileEnterpriseAccountsModel({ }: { mode?: 'modal' | 'mounted' } = {}): UserProfileEnterpriseAccountsModel { const clerk = useClerk(); const router = useMosaicRouter(); - const m = useMessages('userProfileEnterpriseAccountsSection'); - const errorText = useErrorText(); const { isLoaded, user } = useUser(); const environment = useMosaicEnvironment(); const { data: connections = [] } = __internal_useUserEnterpriseConnections({ @@ -105,10 +102,15 @@ export function useUserProfileEnterpriseAccountsModel({ const userId = user.id; + const unavailable = () => + new ClerkRuntimeError('This enterprise connection is no longer available.', { + code: 'enterprise_connection_unavailable', + }); + const currentUser = () => { const current = clerk.user; if (!current || current.id !== userId) { - throw new Error(m.errors.unavailable); + throw unavailable(); } return current; }; @@ -119,19 +121,17 @@ export function useUserProfileEnterpriseAccountsModel({ connect: async connectionId => { const current = currentUser(); if (!projection.connections.some(connection => connection.id === connectionId)) { - throw new Error(m.errors.unavailable); + throw unavailable(); } const url = window.location.href; const redirectUrl = mode === 'modal' ? appendModalState({ url, componentName: 'UserProfile' }) : url; - const account = await current - .createExternalAccount({ enterpriseConnectionId: connectionId, redirectUrl }) - .catch(error => { - throw new Error(enterpriseAccountErrorMessage(error, errorText, m.errors.generic), { cause: error }); - }); + const account = await current.createExternalAccount({ enterpriseConnectionId: connectionId, redirectUrl }); currentUser(); const redirect = account.verification?.externalVerificationRedirectURL; if (!redirect) { - throw new Error(m.errors.missingVerificationUrl); + throw new ClerkRuntimeError('OAuth flow did not receive a verification URL.', { + code: 'oauth_missing_verification_url', + }); } router.windowNavigate(redirect); return 'redirecting'; diff --git a/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.tsx b/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.tsx index 15411130eaf..153c981c49f 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.tsx @@ -33,7 +33,7 @@ function EnterpriseAccounts({ model }: { model: Extract; diff --git a/packages/mosaic/src/features/user-profile/user-profile-mfa-section.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-mfa-section.view.tsx index c0f7c275357..8af4fee0530 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-mfa-section.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-mfa-section.view.tsx @@ -2,6 +2,7 @@ import { type ReactNode, type Ref, useMemo, useRef, useState } from 'react'; import { Confirmation } from '../../blocks/confirmation'; import { Section } from '../../components/section'; +import { useAction } from '../../hooks/use-action'; import { useListRemovalFocus } from '../../hooks/use-list-removal-focus'; import { fill, type MosaicMessages, useMessages } from '../../localization'; import { UserProfileAddMfaDialog } from './user-profile-add-mfa.dialog'; @@ -51,32 +52,10 @@ export function UserProfileMfaSectionView({ }); const removeMethod = useMemo(() => Confirmation.createHandle(), []); const [pickerOpen, setPickerOpen] = useState(false); - const [isSettingDefault, setIsSettingDefault] = useState(false); - const [defaultError, setDefaultError] = useState(); - const settingDefault = useRef(false); - - const setDefault = async (id: string) => { + const setDefault = useAction((id: string) => onSetDefault?.(id), { errorFallback: m.setDefaultError }); + const canSetDefault = (id: string) => { const method = methods.find(method => method.id === id); - if ( - !onSetDefault || - method?.type !== 'sms' || - !method.canSetDefault || - method.isDefault || - settingDefault.current - ) { - return; - } - settingDefault.current = true; - setIsSettingDefault(true); - setDefaultError(undefined); - try { - await onSetDefault(id); - } catch (error) { - setDefaultError(error instanceof Error ? error.message : m.setDefaultError); - } finally { - settingDefault.current = false; - setIsSettingDefault(false); - } + return method?.type === 'sms' && method.canSetDefault && !method.isDefault; }; return ( @@ -112,12 +91,20 @@ export function UserProfileMfaSectionView({ method={method} triggerRef={removalFocus.registerTrigger(method.id)} onRemove={onRemove ? () => removeMethod.open(method) : undefined} - onSetDefault={onSetDefault && !isSettingDefault ? id => void setDefault(id) : undefined} + onSetDefault={ + onSetDefault && !setDefault.isPending + ? id => { + if (canSetDefault(id)) { + void setDefault.run(id); + } + } + : undefined + } onRegenerateBackupCodes={onRegenerateBackupCodes} /> ))} - {defaultError} + {setDefault.errorMessage} {onRemove ? ( void | Promise) | undefined; - error: string | undefined; + error: LocalizableError | undefined; } type UserProfileRenamePasskeyEvent = @@ -53,7 +56,7 @@ const userProfileRenamePasskeyMachine = createMachine({ onDone: { target: 'idle' }, onError: { target: 'editing', - actions: assign((_, event) => ({ error: event.error instanceof Error ? event.error.message : m.saveError })), + actions: assign((_, event) => ({ error: toLocalizableError(event.error) })), }, }), }, @@ -82,6 +85,7 @@ export function useUserProfileRenamePasskeyController({ name, onRename, }: UserProfileRenamePasskeyControllerOptions): UserProfileRenamePasskeyController { + const errorText = useErrorText(); const [snapshot, send] = useMachine(userProfileRenamePasskeyMachine, { context: { passkeyId: id, savedName: name, onRename }, }); @@ -94,6 +98,6 @@ export function useUserProfileRenamePasskeyController({ onSubmit: () => send({ type: 'SAVE' }), canSave: isSaveable(snapshot.context), isSaving: snapshot.value === 'saving', - error: snapshot.context.error, + error: snapshot.context.error ? errorText(snapshot.context.error, m.saveError) : undefined, }; } diff --git a/packages/mosaic/src/hooks/__tests__/use-action.test.tsx b/packages/mosaic/src/hooks/__tests__/use-action.test.tsx new file mode 100644 index 00000000000..462b06556af --- /dev/null +++ b/packages/mosaic/src/hooks/__tests__/use-action.test.tsx @@ -0,0 +1,104 @@ +import { act, renderHook } from '@testing-library/react'; +import type { ReactNode } from 'react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { deferred } from '../../__tests__/async'; +import { clerkApiError } from '../../__tests__/clerk-errors'; +import { MosaicProvider } from '../../mosaic-provider'; +import { useAction } from '../use-action'; + +afterEach(() => { + vi.restoreAllMocks(); +}); + +function blocked() { + return clerkApiError('action_blocked', 'Raw server sentence.'); +} + +describe('useAction', () => { + it('reports success and leaves no error', async () => { + const { result } = renderHook(() => useAction(() => Promise.resolve())); + + let succeeded: boolean | undefined; + await act(async () => { + succeeded = await result.current.run(); + }); + + expect(succeeded).toBe(true); + expect(result.current.errorMessage).toBeUndefined(); + }); + + it('is pending while the action runs and ignores a second run until it settles', async () => { + const pending = deferred(); + const action = vi.fn(() => pending.promise); + const { result } = renderHook(() => useAction(action)); + + act(() => { + void result.current.run(); + void result.current.run(); + }); + + expect(result.current.isPending).toBe(true); + expect(action).toHaveBeenCalledOnce(); + await act(async () => { + pending.resolve(); + await pending.promise; + }); + expect(result.current.isPending).toBe(false); + }); + + it('passes its arguments to the action', async () => { + const action = vi.fn((_id: string) => Promise.resolve()); + const { result } = renderHook(() => useAction(action)); + + await act(() => result.current.run('phone_1')); + + expect(action).toHaveBeenCalledWith('phone_1'); + }); + + it('reports failure with the localized copy for the code Clerk refused with', async () => { + const wrapper = ({ children }: { children: ReactNode }) => ( + + {children} + + ); + const { result } = renderHook(() => useAction(() => Promise.reject(blocked())), { wrapper }); + + let succeeded: boolean | undefined; + await act(async () => { + succeeded = await result.current.run(); + }); + + expect(succeeded).toBe(false); + expect(result.current.errorMessage).toBe('Acción bloqueada.'); + expect(result.current.isPending).toBe(false); + }); + + it('shows the fallback, never the message, of an unexpected error', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}); + const { result } = renderHook(() => + useAction(() => Promise.reject(new Error('Cannot read properties of undefined')), { + errorFallback: 'Unable to sign out.', + }), + ); + + await act(() => result.current.run()); + + expect(result.current.errorMessage).toBe('Unable to sign out.'); + }); + + it('clears the last error when run again and when reset', async () => { + const action = vi.fn<() => Promise>().mockRejectedValueOnce(blocked()).mockResolvedValueOnce(undefined); + const { result } = renderHook(() => useAction(action)); + await act(() => result.current.run()); + expect(result.current.errorMessage).toBeDefined(); + + await act(() => result.current.run()); + expect(result.current.errorMessage).toBeUndefined(); + + action.mockRejectedValueOnce(blocked()); + await act(() => result.current.run()); + act(() => result.current.reset()); + expect(result.current.errorMessage).toBeUndefined(); + }); +}); diff --git a/packages/mosaic/src/hooks/use-action.ts b/packages/mosaic/src/hooks/use-action.ts new file mode 100644 index 00000000000..717fee3722d --- /dev/null +++ b/packages/mosaic/src/hooks/use-action.ts @@ -0,0 +1,57 @@ +import { useRef, useState } from 'react'; + +import type { LocalizableError } from '../localization'; +import { useErrorText } from '../localization'; +import { toLocalizableError } from '../utils/form-error'; + +export interface ActionOptions { + /** Copy shown when the action fails without an error Clerk can describe, such as a network or code fault (default: the generic error) */ + errorFallback?: string; +} + +export interface Action { + run: (...args: TArgs) => Promise; + isPending: boolean; + errorMessage: string | undefined; + reset: () => void; +} + +/** + * Runs an inline action, such as a row button, and tracks whether it is pending and why it failed. + * `run` resolves `true` on success and `false` on failure, and ignores calls while one is in flight. + */ +export function useAction( + action: (...args: TArgs) => Promise | void, + { errorFallback }: ActionOptions = {}, +): Action { + const errorText = useErrorText(); + const [isPending, setIsPending] = useState(false); + const [error, setError] = useState(); + const running = useRef(false); + + const run = async (...args: TArgs) => { + if (running.current) { + return false; + } + running.current = true; + setIsPending(true); + setError(undefined); + try { + await action(...args); + return true; + } catch (cause) { + setError(toLocalizableError(cause)); + return false; + } finally { + running.current = false; + setIsPending(false); + } + }; + + return { + run, + isPending, + errorMessage: error ? errorText(error, errorFallback) : undefined, + reset: () => setError(undefined), + }; +} diff --git a/packages/mosaic/src/localization/errors.messages.ts b/packages/mosaic/src/localization/errors.messages.ts index 3f9acc7e9cf..4827bd81b69 100644 --- a/packages/mosaic/src/localization/errors.messages.ts +++ b/packages/mosaic/src/localization/errors.messages.ts @@ -1,9 +1,12 @@ +// TODO: Seed this catalog from the unstable__errors entries in @clerk/localizations so existing copy and translations carry over. export const errorMessages: { readonly generic: string } & Readonly> = { generic: 'Something went wrong. Please try again.', action_blocked: "This action couldn't be completed. Please try again later or contact support if this persists.", avatar_file_count_exceeded: 'Only one file can be uploaded at a time.', avatar_file_size_exceeded: 'File size exceeds the maximum limit of 10MB. Please choose a smaller file.', avatar_file_type_invalid: 'File type not supported. Please upload a JPG, PNG, GIF, or WEBP image.', + connected_account_unavailable: 'This connected account is no longer available.', + enterprise_connection_unavailable: 'This enterprise connection is no longer available.', form_new_password_matches_current: 'New password cannot be the same as the current password.', form_password_incorrect: 'Your current password is incorrect.', form_password_length_too_short: 'Your password is too short. It must be at least 8 characters long.', @@ -17,4 +20,5 @@ export const errorMessages: { readonly generic: string } & Readonly { expect(errorText({ 'errors.generic': 'Algo salió mal.' })({})).toBe('Algo salió mal.'); expect(errorText()({ code: 'toString' })).toBe('Something went wrong. Please try again.'); }); + + it('uses the fallback it is given ahead of the generic message', () => { + expect(errorText()({}, 'Unable to sign out.')).toBe('Unable to sign out.'); + }); + + it('prefers the copy for the code and the message Clerk sent over the fallback', () => { + const text = errorText({ 'errors.form_identifier_exists': 'Ya existe.' }); + expect(text({ code: 'form_identifier_exists', message: 'Taken' }, 'Unable to save.')).toBe('Ya existe.'); + expect(text({ code: 'unknown', message: 'Taken' }, 'Unable to save.')).toBe('Taken'); + }); + + it('never shows an empty message', () => { + expect(errorText()({ code: 'unknown', message: '' }, 'Unable to save.')).toBe('Unable to save.'); + expect(errorText()({ code: 'unknown', message: '' })).toBe('Something went wrong. Please try again.'); + }); }); describe('Clerk API error localization', () => { @@ -90,10 +105,8 @@ describe('Clerk API error localization', () => { expect(text(toLocalizableApiError(firstError([{ code: 'unknown', message: 'Short' }])))).toBe('Short'); }); - it('preserves an empty message unless the feature provides a fallback', () => { + it('uses the feature fallback for an empty message', () => { const error = firstError([{ code: 'unknown', message: '', long_message: '' }]); - const text = errorText(); - expect(text(toLocalizableApiError(error))).toBe(''); - expect(text(toLocalizableApiError(error, 'Retry'))).toBe('Retry'); + expect(errorText()(toLocalizableApiError(error, 'Retry'))).toBe('Retry'); }); }); diff --git a/packages/mosaic/src/localization/errors.ts b/packages/mosaic/src/localization/errors.ts index d45afbfd554..5a92380c054 100644 --- a/packages/mosaic/src/localization/errors.ts +++ b/packages/mosaic/src/localization/errors.ts @@ -21,12 +21,18 @@ export function toLocalizableApiError(error: ClerkAPIError, fallback?: string): }; } -export function useErrorText(): (error: LocalizableError) => string { +/** + * Turns an error into copy: the catalog entry for its code on that field, then for its code, then the + * message Clerk sent, then `fallback`, then the generic error. + */ +export function useErrorText(): (error: LocalizableError, fallback?: string) => string { const messages = useMessages('errors'); const lookup = (key: string | undefined) => (key && Object.hasOwn(messages, key) ? messages[key] : undefined); - return ({ code, paramName, message, params }) => { + return ({ code, paramName, message, params }, fallback) => { const template = - lookup(code && paramName ? `${code}__${paramName}` : undefined) ?? lookup(code) ?? message ?? messages.generic; + lookup(code && paramName ? `${code}__${paramName}` : undefined) ?? + lookup(code) ?? + (message || fallback || messages.generic); return params ? fill(template, params) : template; }; } diff --git a/packages/mosaic/src/utils/form-error.test.ts b/packages/mosaic/src/utils/form-error.test.ts new file mode 100644 index 00000000000..8accc0c9c1a --- /dev/null +++ b/packages/mosaic/src/utils/form-error.test.ts @@ -0,0 +1,60 @@ +import { ClerkAPIResponseError, ClerkRuntimeError } from '@clerk/shared/error'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { SaveError, toLocalizableError } from './form-error'; + +function silenceConsole() { + return vi.spyOn(console, 'error').mockImplementation(() => {}); +} + +describe('toLocalizableError', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('reads the global error a save left, code and all', () => { + const cause = new SaveError({ global: { code: 'action_blocked', message: 'Raw server sentence.' } }); + expect(toLocalizableError(cause)).toEqual({ + code: 'action_blocked', + message: 'Raw server sentence.', + }); + }); + + it('falls back to the first field error when a save named only fields', () => { + const cause = new SaveError({ fields: { name: { code: 'form_param_nil', paramName: 'name' } } }); + expect(toLocalizableError(cause)).toEqual({ code: 'form_param_nil', paramName: 'name' }); + }); + + it('has nothing to say when a save carried nothing', () => { + expect(toLocalizableError(new SaveError({}))).toEqual({}); + }); + + it('maps a Clerk API error that never went through save', () => { + const cause = new ClerkAPIResponseError('Forbidden', { + status: 403, + data: [{ code: 'not_allowed_access', message: 'Short', long_message: 'Long', meta: { param_name: 'role' } }], + }); + expect(toLocalizableError(cause)).toEqual({ + code: 'not_allowed_access', + paramName: 'role', + message: 'Long', + }); + }); + + it('maps a Clerk runtime error by its code', () => { + const cause = new ClerkRuntimeError('Network down.', { code: 'network_error' }); + expect(toLocalizableError(cause)).toMatchObject({ code: 'network_error' }); + }); + + it('never shows the message of a plain error', () => { + const log = silenceConsole(); + const cause = new Error('Cannot read properties of undefined'); + expect(toLocalizableError(cause)).toEqual({}); + expect(log).toHaveBeenCalledWith(cause); + }); + + it('has nothing to say for something that is not an error at all', () => { + silenceConsole(); + expect(toLocalizableError('nope')).toEqual({}); + }); +}); diff --git a/packages/mosaic/src/utils/form-error.ts b/packages/mosaic/src/utils/form-error.ts index 45dd494ae4d..40742824787 100644 --- a/packages/mosaic/src/utils/form-error.ts +++ b/packages/mosaic/src/utils/form-error.ts @@ -79,6 +79,19 @@ export async function save( } } +/** + * Reads what to tell the user about a failed action. Clerk errors keep their code so the catalog can + * localize them; anything else is logged and carries nothing, leaving the caller's fallback to show. + */ +export function toLocalizableError(cause: unknown): LocalizableError { + const formError = cause instanceof SaveError ? cause.formError : toClerkFormError(cause, [], undefined); + if (formError) { + return formError.global ?? Object.values(formError.fields ?? {}).find(error => error !== undefined) ?? {}; + } + console.error(cause); + return {}; +} + /** Reads what a rejected save left for the view. An unrecognized rejection is the generic error. */ export function toFormError(cause: unknown): FormError { if (cause instanceof SaveError) { From 47e5d55613b6ca8b27ce4f246ef95da725fae5a0 Mon Sep 17 00:00:00 2001 From: Alex Carpenter Date: Mon, 5 Oct 2026 16:35:36 -0400 Subject: [PATCH 2/7] fix(mosaic): register passkey messages, log user button failures, rename useAction to usePendingAction --- .../skills/mosaic/references/controllers.md | 28 +++++++++++++++---- .claude/skills/mosaic/references/machines.md | 4 +++ .claude/skills/mosaic/references/views.md | 13 ++++++--- .../requests-table-tab.view.tsx | 8 +++--- .../__tests__/user-button.feature.test.tsx | 14 ++++++++-- .../user-button/user-button.controller.tsx | 5 +++- ...ser-profile-passkeys-section.view.test.tsx | 12 +++++--- .../user-profile-device-details.dialog.tsx | 4 +-- .../user-profile-mfa-section.view.tsx | 4 +-- .../user-profile-passkey-row.view.tsx | 4 +-- .../user-profile-passkeys-section.view.tsx | 4 +-- .../user-profile-rename-passkey.controller.ts | 4 +-- .../user-profile-rename-passkey.dialog.tsx | 3 +- ...n.test.tsx => use-pending-action.test.tsx} | 16 +++++------ .../{use-action.ts => use-pending-action.ts} | 10 +++---- packages/mosaic/src/localization/registry.ts | 2 ++ 16 files changed, 90 insertions(+), 45 deletions(-) rename packages/mosaic/src/hooks/__tests__/{use-action.test.tsx => use-pending-action.test.tsx} (83%) rename packages/mosaic/src/hooks/{use-action.ts => use-pending-action.ts} (86%) diff --git a/.claude/skills/mosaic/references/controllers.md b/.claude/skills/mosaic/references/controllers.md index 89d2a8efcff..64f7ed1e26b 100644 --- a/.claude/skills/mosaic/references/controllers.md +++ b/.claude/skills/mosaic/references/controllers.md @@ -28,6 +28,20 @@ complexity calls for: | Is a boolean or a controlled value, no async, nothing else depends on it | `useState` | | Has an async lifecycle, or two values that must change together | A machine, same file | | Has a coordinated async core plus some UI-only flags beside it | Both — machine for the core | +| Is one async action from a button, needing only pending and an error | `usePendingAction` | + +`usePendingAction(fn, { errorFallback })` covers row actions: set default, sign +out, accept or decline. It ignores clicks while one is in flight, owns the error +(see "Errors") and returns `{ run, isPending, errorMessage, reset }`. `run` +resolves `true` on success, so the caller can close a dialog or move focus. It +is not for: + +- a form → `useForm` +- a confirm step → `Confirmation` / `Destructive` +- an action whose result drives other state → a machine +- per-row pending across a list, or holding pending through a redirect → the + connected and enterprise accounts controllers, until `usePendingAction` takes + a key `useUserProfileDeleteSectionController` earns a machine on three counts: the delete is async, a failure must land back on the previous step with a reason, @@ -126,12 +140,16 @@ One path, so every error a user sees goes through the `errors.*` catalog: - **Owners map, nothing else does.** `toLocalizableError(cause)` is called only by the hooks that own a failure: `useForm`, `useConfirmationController`, - `useDestructiveController` and `useAction` (inline row actions). Use one of + `useDestructiveController` and `usePendingAction` (inline row actions). Use one of them instead of catching in a feature. -- **Copy is resolved at render.** Owners store the `LocalizableError` and return - `errorMessage` from `useErrorText()`: the catalog entry for - `code__paramName`, then `code`, then Clerk's message, then the feature's - `errorFallback`, then the generic error. +- **Owners resolve copy at render; views get a string.** Owners store the + `LocalizableError` and return `errorMessage` from `useErrorText()`: the + catalog entry for `code__paramName`, then `code`, then Clerk's message, then + the feature's `errorFallback`, then the generic error. Views never call + `useErrorText` on an error they did not create themselves. +- Not there yet, each with a follow-up: `useForm` resolves when the save fails + and falls back to `form.error` instead of taking an `errorFallback`, and the + profile picture row resolves its controller's error in the view. - **`errorFallback` is the feature's copy for faults Clerk cannot describe** (network, code bugs). An unknown error is logged and its `.message` is never shown. diff --git a/.claude/skills/mosaic/references/machines.md b/.claude/skills/mosaic/references/machines.md index 160122062c2..fd62c96fec5 100644 --- a/.claude/skills/mosaic/references/machines.md +++ b/.claude/skills/mosaic/references/machines.md @@ -48,6 +48,10 @@ deleting: { } ``` +A machine whose surface shows no error yet still logs it, so a failure is never +silent: `onError: { target: 'idle', actions: [assign(() => settled), (_, event) => console.error(event.error)] }` +(`user-button.controller.tsx`). + Context holds the `LocalizableError`, never copy. The controller turns it into text at render with `errorText(error, errorFallback)`, so locale changes and overrides still apply (see `controllers.md` → "Errors"). diff --git a/.claude/skills/mosaic/references/views.md b/.claude/skills/mosaic/references/views.md index 84253fd8355..8591cb6c450 100644 --- a/.claude/skills/mosaic/references/views.md +++ b/.claude/skills/mosaic/references/views.md @@ -45,14 +45,19 @@ Two shapes, chosen by whether the slice fetches its own data: ## Where the strings live Every string a view renders comes from the feature's `*.messages.ts`, shaped the -way `@clerk/i18n` takes a base definition, so localizing is registering a -namespace rather than hunting literals down first. A plural message is its forms; -a parameterized one is its template. Import it as `m` and read through it: +way `@clerk/i18n` takes a base definition. A plural message is its forms; a +parameterized one is its template. + +Register the file as a namespace in `src/localization/registry.ts`, then read it +with `useMessages` in the view or controller that renders it: ```tsx -import { fill, plural, userButtonBase as m } from './user-button.messages'; +const m = useMessages('userButton'); ``` +Never import a `*.messages.ts` directly. A direct import skips the customer's +locale and overrides, so the copy can't be localized. + ## Blocks A **block** is a view fragment that owns one piece of state nothing outside it can diff --git a/packages/mosaic/src/features/organization-profile/requests-table-tab.view.tsx b/packages/mosaic/src/features/organization-profile/requests-table-tab.view.tsx index ee6b3feafdc..3e8fdd167a1 100644 --- a/packages/mosaic/src/features/organization-profile/requests-table-tab.view.tsx +++ b/packages/mosaic/src/features/organization-profile/requests-table-tab.view.tsx @@ -11,8 +11,8 @@ import { Section } from '../../components/section'; import { Spinner } from '../../components/spinner'; import { Table, type TableHeaderCellProps } from '../../components/table'; import { VisuallyHidden } from '../../components/visually-hidden'; -import { type Action, useAction } from '../../hooks/use-action'; import { useListRemovalFocus } from '../../hooks/use-list-removal-focus'; +import { type PendingAction, usePendingAction } from '../../hooks/use-pending-action'; import { fill, useMessages } from '../../localization'; import { useDataTable } from '../../primitives/hooks'; import { mergeStyleProps, themeProps } from '../../props'; @@ -274,12 +274,12 @@ function RequestActions({ const m = useMessages('requestsTableTab'); const [acceptRef] = useState(() => registerAcceptTrigger(request.id)); const [declineRef] = useState(() => registerDeclineTrigger(request.id)); - const accept = useAction(() => onAccept?.(), { errorFallback: m.acceptError }); - const decline = useAction(() => onDecline?.(), { errorFallback: m.declineError }); + const accept = usePendingAction(() => onAccept?.(), { errorFallback: m.acceptError }); + const decline = usePendingAction(() => onDecline?.(), { errorFallback: m.declineError }); const pendingAction = request.pendingAction ?? (accept.isPending ? 'accept' : decline.isPending ? 'decline' : undefined); const error = accept.errorMessage ?? decline.errorMessage; - const decide = (action: Action<[]>, other: Action<[]>) => { + const decide = (action: PendingAction<[]>, other: PendingAction<[]>) => { if (pendingAction) { return; } diff --git a/packages/mosaic/src/features/user-button/__tests__/user-button.feature.test.tsx b/packages/mosaic/src/features/user-button/__tests__/user-button.feature.test.tsx index 70c4d37400b..f87f61be1d8 100644 --- a/packages/mosaic/src/features/user-button/__tests__/user-button.feature.test.tsx +++ b/packages/mosaic/src/features/user-button/__tests__/user-button.feature.test.tsx @@ -2,7 +2,7 @@ import { UNSAFE_PortalProvider as PortalProvider } from '@clerk/shared/react'; import type { CustomPage, PhoneNumberJSON, UserJSON, Web3WalletJSON } from '@clerk/shared/types'; import { act, screen, waitFor, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import { describe, expect, it, vi } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { type FakeFapiSeed, holdRequests, serveFapi } from '../../../__tests__/feature/fake-fapi'; import { @@ -20,6 +20,10 @@ import { renderWithClerk } from '../../../__tests__/feature/render'; import type { UserButtonProps } from '../user-button'; import { UserButton } from '../user-button'; +afterEach(() => { + vi.restoreAllMocks(); +}); + const acme = fapiOrganization({ id: 'org_1', name: 'Acme', members_count: 3 }); const other = fapiOrganization({ id: 'org_9', name: 'Other' }); const beta = fapiOrganization({ id: 'org_2', name: 'Beta' }); @@ -588,7 +592,8 @@ describe('UserButton', () => { await waitFor(() => expect(popup()).toBeNull()); }); - it('stays open and clears busy state when the switch fails', async () => { + it('stays open, clears busy state and logs the failure when the switch fails', async () => { + const log = vi.spyOn(console, 'error').mockImplementation(() => {}); await renderUserButton(); const user = await open(); await screen.findByRole('button', { name: 'Other' }); @@ -597,11 +602,14 @@ describe('UserButton', () => { await user.click(screen.getByRole('button', { name: 'Other' })); expect(waiting()).toBe(true); - touch.fail(); + touch.fail('session_touch_failed'); await waitFor(() => expect(waiting()).toBe(false), { timeout: 2000 }); expect(popup()).toBeInTheDocument(); expect(signOut()).toBeEnabled(); + expect(log).toHaveBeenCalledWith( + expect.objectContaining({ errors: [expect.objectContaining({ code: 'session_touch_failed' })] }), + ); }); it('still shows the action when reopened before it settles, and starts no second one', async () => { diff --git a/packages/mosaic/src/features/user-button/user-button.controller.tsx b/packages/mosaic/src/features/user-button/user-button.controller.tsx index 8a8a31e535a..1c3861966d2 100644 --- a/packages/mosaic/src/features/user-button/user-button.controller.tsx +++ b/packages/mosaic/src/features/user-button/user-button.controller.tsx @@ -84,7 +84,10 @@ const userButtonMachine = createMachine({ { target: 'idle', actions: assign(() => settled) }, ], // Leave `open` as the user left it. The error surface is a later change. - onError: { target: 'idle', actions: assign(() => settled) }, + onError: { + target: 'idle', + actions: [assign(() => settled), (_, event) => console.error(event.error)], + }, }), }, }, diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile-passkeys-section.view.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile-passkeys-section.view.test.tsx index d154b369b01..00aeffb265c 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile-passkeys-section.view.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile-passkeys-section.view.test.tsx @@ -4,6 +4,7 @@ import { useState } from 'react'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { clerkApiError } from '../../../__tests__/clerk-errors'; +import type { MosaicLocalization } from '../../../localization'; import { MosaicProvider } from '../../../mosaic-provider'; import type { UserProfilePasskey, UserProfilePasskeysSectionViewProps } from '../user-profile-passkeys-section.view'; import { UserProfilePasskeysSectionView } from '../user-profile-passkeys-section.view'; @@ -17,7 +18,7 @@ const passkeys: UserProfilePasskey[] = [ { id: 'phone', name: 'iPhone' }, ]; -function renderView(overrides: Partial = {}) { +function renderView(overrides: Partial = {}, localization?: MosaicLocalization) { const props: UserProfilePasskeysSectionViewProps = { passkeys, onAdd: vi.fn(), @@ -26,7 +27,7 @@ function renderView(overrides: Partial = {} ...overrides, }; return render( - + , ); @@ -130,12 +131,15 @@ describe('passkeys section', () => { it('shows the rename fallback, never the message, when renaming fails unexpectedly', async () => { vi.spyOn(console, 'error').mockImplementation(() => {}); const user = userEvent.setup(); - renderView({ onRename: vi.fn().mockRejectedValue(new Error('Cannot read properties of undefined')) }); + renderView( + { onRename: vi.fn().mockRejectedValue(new Error('Cannot read properties of undefined')) }, + { overrides: { 'userProfilePasskeys.saveError': 'Could not rename this passkey.' } }, + ); await user.click(screen.getByRole('button', { name: 'Manage MacBook' })); await user.click(screen.getByRole('menuitem', { name: 'Rename' })); await user.type(screen.getByRole('textbox', { name: 'Passkey name' }), ' Pro'); await user.click(screen.getByRole('button', { name: 'Save' })); - expect(await screen.findByRole('alert')).toHaveTextContent('Something went wrong. Please try again.'); + expect(await screen.findByRole('alert')).toHaveTextContent('Could not rename this passkey.'); }); it('keeps an empty section visible when Add is unavailable', () => { diff --git a/packages/mosaic/src/features/user-profile/user-profile-device-details.dialog.tsx b/packages/mosaic/src/features/user-profile/user-profile-device-details.dialog.tsx index 5ef1b02eb3d..df90ead4fa5 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-device-details.dialog.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-device-details.dialog.tsx @@ -3,7 +3,7 @@ import { Card } from '../../components/card'; import { DataList } from '../../components/data-list'; import type { DialogFocusTarget, DialogHandle } from '../../components/dialog'; import { Dialog } from '../../components/dialog'; -import { useAction } from '../../hooks/use-action'; +import { usePendingAction } from '../../hooks/use-pending-action'; import { fill, useMessages } from '../../localization'; import type { UserProfileDevice } from './user-profile-active-devices.types'; @@ -44,7 +44,7 @@ function DeviceDetailsCard({ onSignOut: UserProfileDeviceDetailsDialogProps['onSignOut']; }) { const m = useMessages('userProfileActiveDevices'); - const signOut = useAction( + const signOut = usePendingAction( async () => { await onSignOut?.(device); handle.close(); diff --git a/packages/mosaic/src/features/user-profile/user-profile-mfa-section.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-mfa-section.view.tsx index 8af4fee0530..aa8c23b2a66 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-mfa-section.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-mfa-section.view.tsx @@ -2,8 +2,8 @@ import { type ReactNode, type Ref, useMemo, useRef, useState } from 'react'; import { Confirmation } from '../../blocks/confirmation'; import { Section } from '../../components/section'; -import { useAction } from '../../hooks/use-action'; import { useListRemovalFocus } from '../../hooks/use-list-removal-focus'; +import { usePendingAction } from '../../hooks/use-pending-action'; import { fill, type MosaicMessages, useMessages } from '../../localization'; import { UserProfileAddMfaDialog } from './user-profile-add-mfa.dialog'; import { UserProfileAddMfaView } from './user-profile-add-mfa.view'; @@ -52,7 +52,7 @@ export function UserProfileMfaSectionView({ }); const removeMethod = useMemo(() => Confirmation.createHandle(), []); const [pickerOpen, setPickerOpen] = useState(false); - const setDefault = useAction((id: string) => onSetDefault?.(id), { errorFallback: m.setDefaultError }); + const setDefault = usePendingAction((id: string) => onSetDefault?.(id), { errorFallback: m.setDefaultError }); const canSetDefault = (id: string) => { const method = methods.find(method => method.id === id); return method?.type === 'sms' && method.canSetDefault && !method.isDefault; diff --git a/packages/mosaic/src/features/user-profile/user-profile-passkey-row.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-passkey-row.view.tsx index b8343d0c3c9..0c75e25b59c 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-passkey-row.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-passkey-row.view.tsx @@ -6,8 +6,7 @@ import { ActionMenu } from '../../components/action-menu'; import { Dialog } from '../../components/dialog'; import { Icon, IconFrame } from '../../components/icon'; import { Section } from '../../components/section'; -import { fill } from '../../localization/messages'; -import { userProfilePasskeysMessages as m } from './user-profile-passkeys-section.messages'; +import { fill, useMessages } from '../../localization'; import { styles } from './user-profile-passkeys-section.styles'; import type { UserProfilePasskey } from './user-profile-passkeys-section.view'; import { useUserProfileRenamePasskeyController } from './user-profile-rename-passkey.controller'; @@ -24,6 +23,7 @@ export function UserProfilePasskeyRowView({ onRename?: (id: string, name: string) => void | Promise; onRemove?: () => void; }) { + const m = useMessages('userProfilePasskeys'); const renameDialog = useMemo(() => Dialog.createHandle(), []); const controller = useUserProfileRenamePasskeyController({ id: passkey.id, name: passkey.name, onRename }); const description = diff --git a/packages/mosaic/src/features/user-profile/user-profile-passkeys-section.view.tsx b/packages/mosaic/src/features/user-profile/user-profile-passkeys-section.view.tsx index 38de095b398..1c38afb1857 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-passkeys-section.view.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-passkeys-section.view.tsx @@ -6,9 +6,8 @@ import { Field } from '../../components/field'; import { Icon } from '../../components/icon'; import { Section } from '../../components/section'; import { useListRemovalFocus } from '../../hooks/use-list-removal-focus'; -import { fill } from '../../localization/messages'; +import { fill, useMessages } from '../../localization'; import { UserProfilePasskeyRowView } from './user-profile-passkey-row.view'; -import { userProfilePasskeysMessages as m } from './user-profile-passkeys-section.messages'; import { styles } from './user-profile-passkeys-section.styles'; export interface UserProfilePasskey { @@ -33,6 +32,7 @@ export function UserProfilePasskeysSectionView({ onRename, onRemove, }: UserProfilePasskeysSectionViewProps) { + const m = useMessages('userProfilePasskeys'); const addButton = useRef(null); const section = useRef(null); const removalFocus = useListRemovalFocus({ diff --git a/packages/mosaic/src/features/user-profile/user-profile-rename-passkey.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-rename-passkey.controller.ts index 58570759333..eb566b36c46 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-rename-passkey.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-rename-passkey.controller.ts @@ -1,9 +1,8 @@ import type { LocalizableError } from '../../localization'; -import { useErrorText } from '../../localization'; +import { useErrorText, useMessages } from '../../localization'; import { setup } from '../../machine/setup'; import { useMachine } from '../../machine/use-machine'; import { toLocalizableError } from '../../utils/form-error'; -import { userProfilePasskeysMessages as m } from './user-profile-passkeys-section.messages'; interface UserProfileRenamePasskeyContext { passkeyId: string; @@ -85,6 +84,7 @@ export function useUserProfileRenamePasskeyController({ name, onRename, }: UserProfileRenamePasskeyControllerOptions): UserProfileRenamePasskeyController { + const m = useMessages('userProfilePasskeys'); const errorText = useErrorText(); const [snapshot, send] = useMachine(userProfileRenamePasskeyMachine, { context: { passkeyId: id, savedName: name, onRename }, diff --git a/packages/mosaic/src/features/user-profile/user-profile-rename-passkey.dialog.tsx b/packages/mosaic/src/features/user-profile/user-profile-rename-passkey.dialog.tsx index 25cef6658d9..bb3451ddc7d 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-rename-passkey.dialog.tsx +++ b/packages/mosaic/src/features/user-profile/user-profile-rename-passkey.dialog.tsx @@ -6,7 +6,7 @@ import type { DialogHandle } from '../../components/dialog'; import { Dialog } from '../../components/dialog'; import { Field } from '../../components/field'; import { Input } from '../../components/input'; -import { userProfilePasskeysMessages as m } from './user-profile-passkeys-section.messages'; +import { useMessages } from '../../localization'; export interface UserProfileRenamePasskeyDialogProps { handle: DialogHandle; @@ -31,6 +31,7 @@ export function UserProfileRenamePasskeyDialog({ error, onSubmit, }: UserProfileRenamePasskeyDialogProps) { + const m = useMessages('userProfilePasskeys'); const formId = useId(); const inputRef = useRef(null); return ( diff --git a/packages/mosaic/src/hooks/__tests__/use-action.test.tsx b/packages/mosaic/src/hooks/__tests__/use-pending-action.test.tsx similarity index 83% rename from packages/mosaic/src/hooks/__tests__/use-action.test.tsx rename to packages/mosaic/src/hooks/__tests__/use-pending-action.test.tsx index 462b06556af..ce7c3c45ccf 100644 --- a/packages/mosaic/src/hooks/__tests__/use-action.test.tsx +++ b/packages/mosaic/src/hooks/__tests__/use-pending-action.test.tsx @@ -5,7 +5,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import { deferred } from '../../__tests__/async'; import { clerkApiError } from '../../__tests__/clerk-errors'; import { MosaicProvider } from '../../mosaic-provider'; -import { useAction } from '../use-action'; +import { usePendingAction } from '../use-pending-action'; afterEach(() => { vi.restoreAllMocks(); @@ -15,9 +15,9 @@ function blocked() { return clerkApiError('action_blocked', 'Raw server sentence.'); } -describe('useAction', () => { +describe('usePendingAction', () => { it('reports success and leaves no error', async () => { - const { result } = renderHook(() => useAction(() => Promise.resolve())); + const { result } = renderHook(() => usePendingAction(() => Promise.resolve())); let succeeded: boolean | undefined; await act(async () => { @@ -31,7 +31,7 @@ describe('useAction', () => { it('is pending while the action runs and ignores a second run until it settles', async () => { const pending = deferred(); const action = vi.fn(() => pending.promise); - const { result } = renderHook(() => useAction(action)); + const { result } = renderHook(() => usePendingAction(action)); act(() => { void result.current.run(); @@ -49,7 +49,7 @@ describe('useAction', () => { it('passes its arguments to the action', async () => { const action = vi.fn((_id: string) => Promise.resolve()); - const { result } = renderHook(() => useAction(action)); + const { result } = renderHook(() => usePendingAction(action)); await act(() => result.current.run('phone_1')); @@ -62,7 +62,7 @@ describe('useAction', () => { {children} ); - const { result } = renderHook(() => useAction(() => Promise.reject(blocked())), { wrapper }); + const { result } = renderHook(() => usePendingAction(() => Promise.reject(blocked())), { wrapper }); let succeeded: boolean | undefined; await act(async () => { @@ -77,7 +77,7 @@ describe('useAction', () => { it('shows the fallback, never the message, of an unexpected error', async () => { vi.spyOn(console, 'error').mockImplementation(() => {}); const { result } = renderHook(() => - useAction(() => Promise.reject(new Error('Cannot read properties of undefined')), { + usePendingAction(() => Promise.reject(new Error('Cannot read properties of undefined')), { errorFallback: 'Unable to sign out.', }), ); @@ -89,7 +89,7 @@ describe('useAction', () => { it('clears the last error when run again and when reset', async () => { const action = vi.fn<() => Promise>().mockRejectedValueOnce(blocked()).mockResolvedValueOnce(undefined); - const { result } = renderHook(() => useAction(action)); + const { result } = renderHook(() => usePendingAction(action)); await act(() => result.current.run()); expect(result.current.errorMessage).toBeDefined(); diff --git a/packages/mosaic/src/hooks/use-action.ts b/packages/mosaic/src/hooks/use-pending-action.ts similarity index 86% rename from packages/mosaic/src/hooks/use-action.ts rename to packages/mosaic/src/hooks/use-pending-action.ts index 717fee3722d..f981227c8f7 100644 --- a/packages/mosaic/src/hooks/use-action.ts +++ b/packages/mosaic/src/hooks/use-pending-action.ts @@ -4,12 +4,12 @@ import type { LocalizableError } from '../localization'; import { useErrorText } from '../localization'; import { toLocalizableError } from '../utils/form-error'; -export interface ActionOptions { +export interface PendingActionOptions { /** Copy shown when the action fails without an error Clerk can describe, such as a network or code fault (default: the generic error) */ errorFallback?: string; } -export interface Action { +export interface PendingAction { run: (...args: TArgs) => Promise; isPending: boolean; errorMessage: string | undefined; @@ -20,10 +20,10 @@ export interface Action { * Runs an inline action, such as a row button, and tracks whether it is pending and why it failed. * `run` resolves `true` on success and `false` on failure, and ignores calls while one is in flight. */ -export function useAction( +export function usePendingAction( action: (...args: TArgs) => Promise | void, - { errorFallback }: ActionOptions = {}, -): Action { + { errorFallback }: PendingActionOptions = {}, +): PendingAction { const errorText = useErrorText(); const [isPending, setIsPending] = useState(false); const [error, setError] = useState(); diff --git a/packages/mosaic/src/localization/registry.ts b/packages/mosaic/src/localization/registry.ts index 9064579842f..2a40f6b2d6c 100644 --- a/packages/mosaic/src/localization/registry.ts +++ b/packages/mosaic/src/localization/registry.ts @@ -24,6 +24,7 @@ import { userProfileConnectedAccountsMessages } from '../features/user-profile/u import { userProfileDangerSectionMessages } from '../features/user-profile/user-profile-danger-section/user-profile-danger-section.messages'; import { userProfileEnterpriseAccountsMessages } from '../features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.messages'; import { userProfileMfaMessages } from '../features/user-profile/user-profile-mfa-section.messages'; +import { userProfilePasskeysMessages } from '../features/user-profile/user-profile-passkeys-section.messages'; import { userProfilePasswordSectionMessages } from '../features/user-profile/user-profile-password-section/user-profile-password-section.messages'; import { userProfileWeb3WalletsMessages } from '../features/user-profile/user-profile-web3-wallets.messages'; import { errorMessages } from './errors.messages'; @@ -56,6 +57,7 @@ export const mosaicMessages = { userProfileDangerSection: userProfileDangerSectionMessages, userProfileEnterpriseAccountsSection: userProfileEnterpriseAccountsMessages, userProfileMfa: userProfileMfaMessages, + userProfilePasskeys: userProfilePasskeysMessages, userProfilePasswordSection: userProfilePasswordSectionMessages, userProfileWeb3Wallets: userProfileWeb3WalletsMessages, }; From 273c0dc4c1bb9b6f05a23c72d3778c5796134bbd Mon Sep 17 00:00:00 2001 From: Alex Carpenter Date: Tue, 6 Oct 2026 07:43:47 -0400 Subject: [PATCH 3/7] docs(mosaic): describe usePendingAction as any single button action --- .claude/skills/mosaic/references/controllers.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.claude/skills/mosaic/references/controllers.md b/.claude/skills/mosaic/references/controllers.md index 64f7ed1e26b..f3ec5f8f68a 100644 --- a/.claude/skills/mosaic/references/controllers.md +++ b/.claude/skills/mosaic/references/controllers.md @@ -30,8 +30,9 @@ complexity calls for: | Has a coordinated async core plus some UI-only flags beside it | Both — machine for the core | | Is one async action from a button, needing only pending and an error | `usePendingAction` | -`usePendingAction(fn, { errorFallback })` covers row actions: set default, sign -out, accept or decline. It ignores clicks while one is in flight, owns the error +`usePendingAction(fn, { errorFallback })` covers any single async action a +button starts: set a default, sign out a device, accept or decline a request. It +ignores clicks while one is in flight, owns the error (see "Errors") and returns `{ run, isPending, errorMessage, reset }`. `run` resolves `true` on success, so the caller can close a dialog or move focus. It is not for: From 35cb2c5a1dedc3e94fd9e350022bca1b0ccde2b1 Mon Sep 17 00:00:00 2001 From: Alex Carpenter Date: Tue, 6 Oct 2026 08:12:59 -0400 Subject: [PATCH 4/7] fix(mosaic): keep the cause of errors Clerk cannot describe toLocalizableError now lives in utils/errors.ts and returns an ErrorDescription: a LocalizableError with a code, or { cause } logged as '[Mosaic] Could not localize error'. Adds catalog copy for network_error. The password form no longer shows the raw message of a non-Clerk error. --- .../skills/mosaic/references/controllers.md | 23 ++++-- .claude/skills/mosaic/references/machines.md | 2 +- .../confirmation.controller.test.ts | 9 +- .../confirmation/confirmation.controller.ts | 6 +- .../destructive.controller.test.ts | 2 +- .../destructive/destructive.controller.ts | 6 +- .../blocks/destructive/destructive.test.tsx | 2 +- .../src/components/form/form.machine.ts | 2 +- .../src/components/form/use-form.test.ts | 2 +- ...ser-profile-account-section.model.test.tsx | 4 +- .../user-profile-edit-name.controller.test.ts | 2 +- .../user-profile-edit-name.dialog.test.tsx | 2 +- ...r-profile-edit-username.controller.test.ts | 2 +- ...user-profile-edit-username.dialog.test.tsx | 2 +- .../user-profile-picture-row.view.test.tsx | 2 +- .../user-profile-picture.controller.test.ts | 2 +- .../user-profile-account-section.model.ts | 2 +- .../user-profile-picture.controller.ts | 2 +- ...e-connected-accounts-section.controller.ts | 6 +- ...-enterprise-accounts-section.controller.ts | 6 +- .../user-profile-password-errors.ts | 11 +-- .../user-profile-password-feedback.test.ts | 8 +- .../user-profile-rename-passkey.controller.ts | 6 +- .../mosaic/src/hooks/use-pending-action.ts | 6 +- .../src/localization/errors.messages.ts | 1 + .../mosaic/src/localization/errors.test.tsx | 4 +- packages/mosaic/src/localization/errors.ts | 24 +++++- packages/mosaic/src/localization/index.ts | 4 +- packages/mosaic/src/utils/errors.test.tsx | 82 +++++++++++++++++++ .../src/utils/{form-error.ts => errors.ts} | 70 ++++++++++------ packages/mosaic/src/utils/form-error.test.ts | 60 -------------- 31 files changed, 214 insertions(+), 148 deletions(-) create mode 100644 packages/mosaic/src/utils/errors.test.tsx rename packages/mosaic/src/utils/{form-error.ts => errors.ts} (61%) delete mode 100644 packages/mosaic/src/utils/form-error.test.ts diff --git a/.claude/skills/mosaic/references/controllers.md b/.claude/skills/mosaic/references/controllers.md index f3ec5f8f68a..526e1b90180 100644 --- a/.claude/skills/mosaic/references/controllers.md +++ b/.claude/skills/mosaic/references/controllers.md @@ -139,12 +139,18 @@ have a machine. One path, so every error a user sees goes through the `errors.*` catalog: -- **Owners map, nothing else does.** `toLocalizableError(cause)` is called only - by the hooks that own a failure: `useForm`, `useConfirmationController`, - `useDestructiveController` and `usePendingAction` (inline row actions). Use one of - them instead of catching in a feature. -- **Owners resolve copy at render; views get a string.** Owners store the - `LocalizableError` and return `errorMessage` from `useErrorText()`: the +- **Owners map, nothing else does.** `toLocalizableError(cause)` from + `utils/errors.ts` is called only by the hooks that own a failure: `useForm`, + `useConfirmationController`, `useDestructiveController` and + `usePendingAction` (inline row actions). Use one of them instead of catching + in a feature. +- **Owners hold an `ErrorDescription`.** It is either a `LocalizableError` + (Clerk gave a `code`) or an `UnlocalizableError` (`{ cause }`, the original + failure). `toLocalizableError` logs the second as + `[Mosaic] Could not localize error`. Branch with `isLocalizableError` only + when a feature handles the two differently. +- **Owners resolve copy at render; views get a string.** Owners return + `errorMessage` from `useErrorText()`: the catalog entry for `code__paramName`, then `code`, then Clerk's message, then the feature's `errorFallback`, then the generic error. Views never call `useErrorText` on an error they did not create themselves. @@ -152,8 +158,9 @@ One path, so every error a user sees goes through the `errors.*` catalog: and falls back to `form.error` instead of taking an `errorFallback`, and the profile picture row resolves its controller's error in the view. - **`errorFallback` is the feature's copy for faults Clerk cannot describe** - (network, code bugs). An unknown error is logged and its `.message` is never - shown. + (code bugs, non-Clerk errors). An `UnlocalizableError` renders the fallback, + never its cause's `.message`. A network failure has its own catalog entry, + `network_error`. - In tests, reject with `clerkApiError(code, message)` from `src/__tests__/clerk-errors.ts` to assert catalog copy. Reject with a plain `Error` (silence `console.error`) to assert the fallback. diff --git a/.claude/skills/mosaic/references/machines.md b/.claude/skills/mosaic/references/machines.md index fd62c96fec5..0d72e5ed840 100644 --- a/.claude/skills/mosaic/references/machines.md +++ b/.claude/skills/mosaic/references/machines.md @@ -52,7 +52,7 @@ A machine whose surface shows no error yet still logs it, so a failure is never silent: `onError: { target: 'idle', actions: [assign(() => settled), (_, event) => console.error(event.error)] }` (`user-button.controller.tsx`). -Context holds the `LocalizableError`, never copy. The controller turns it into +Context holds the `ErrorDescription`, never copy. The controller turns it into text at render with `errorText(error, errorFallback)`, so locale changes and overrides still apply (see `controllers.md` → "Errors"). diff --git a/packages/mosaic/src/blocks/confirmation/confirmation.controller.test.ts b/packages/mosaic/src/blocks/confirmation/confirmation.controller.test.ts index a178e506106..849034f9a0d 100644 --- a/packages/mosaic/src/blocks/confirmation/confirmation.controller.test.ts +++ b/packages/mosaic/src/blocks/confirmation/confirmation.controller.test.ts @@ -2,7 +2,7 @@ import { act, renderHook, waitFor } from '@testing-library/react'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { createActor } from '../../machine/create-actor'; -import { SaveError } from '../../utils/form-error'; +import { SaveError } from '../../utils/errors'; import { confirmationMachine, useConfirmationController } from './confirmation.controller'; const blocked = new SaveError({ global: { code: 'action_blocked', message: 'Raw server sentence.' } }); @@ -56,13 +56,14 @@ describe('confirmationMachine', () => { expect(actor.getSnapshot().context.error).toEqual(blocked.formError.global); }); - it('holds nothing of an unexpected error, never its message', async () => { + it('keeps the cause of an unexpected error, never its message', async () => { vi.spyOn(console, 'error').mockImplementation(() => {}); + const cause = new Error('Cannot read properties of undefined'); const actor = start(); - actor.send({ type: 'CONFIRM', run: () => Promise.reject(new Error('Cannot read properties of undefined')) }); + actor.send({ type: 'CONFIRM', run: () => Promise.reject(cause) }); await vi.waitFor(() => expect(actor.getSnapshot().value).toBe('confirming')); - expect(actor.getSnapshot().context.error).toEqual({}); + expect(actor.getSnapshot().context.error).toEqual({ cause }); }); it('drops the error when cancelled, so the next open starts clean', async () => { diff --git a/packages/mosaic/src/blocks/confirmation/confirmation.controller.ts b/packages/mosaic/src/blocks/confirmation/confirmation.controller.ts index 1f614f7f1f6..053bb1a3f7d 100644 --- a/packages/mosaic/src/blocks/confirmation/confirmation.controller.ts +++ b/packages/mosaic/src/blocks/confirmation/confirmation.controller.ts @@ -1,12 +1,12 @@ -import type { LocalizableError } from '../../localization'; +import type { ErrorDescription } from '../../localization'; import { useErrorText } from '../../localization'; import { setup } from '../../machine/setup'; import { useMachine } from '../../machine/use-machine'; -import { toLocalizableError } from '../../utils/form-error'; +import { toLocalizableError } from '../../utils/errors'; export interface ConfirmationContext { run: () => Promise; - error: LocalizableError | undefined; + error: ErrorDescription | undefined; } export type ConfirmationEvent = { type: 'OPEN' } | { type: 'CONFIRM'; run: () => Promise } | { type: 'CANCEL' }; diff --git a/packages/mosaic/src/blocks/destructive/destructive.controller.test.ts b/packages/mosaic/src/blocks/destructive/destructive.controller.test.ts index 6bbf2ce3191..2360e9383cd 100644 --- a/packages/mosaic/src/blocks/destructive/destructive.controller.test.ts +++ b/packages/mosaic/src/blocks/destructive/destructive.controller.test.ts @@ -4,7 +4,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import { deferred } from '../../__tests__/async'; import type { ReverificationController } from '../../features/reverification'; -import { SaveError } from '../../utils/form-error'; +import { SaveError } from '../../utils/errors'; import { useDestructiveController } from './destructive.controller'; const idleReverification = { status: 'idle', visible: false, reset: vi.fn() } as ReverificationController; diff --git a/packages/mosaic/src/blocks/destructive/destructive.controller.ts b/packages/mosaic/src/blocks/destructive/destructive.controller.ts index a65e8fde901..526639325fe 100644 --- a/packages/mosaic/src/blocks/destructive/destructive.controller.ts +++ b/packages/mosaic/src/blocks/destructive/destructive.controller.ts @@ -2,9 +2,9 @@ import { isReverificationCancelledError } from '@clerk/shared/error'; import { useState } from 'react'; import type { ReverificationController } from '../../features/reverification'; -import type { LocalizableError } from '../../localization'; +import type { ErrorDescription } from '../../localization'; import { useErrorText } from '../../localization'; -import { toLocalizableError } from '../../utils/form-error'; +import { toLocalizableError } from '../../utils/errors'; import type { DestructiveControlledProps } from './destructive'; export type DestructiveController = Pick< @@ -19,7 +19,7 @@ type DestructiveState = | { status: 'closed' } | { status: 'open-needs-confirmation' } | { status: 'open-pending' } - | { status: 'open-error'; error: LocalizableError }; + | { status: 'open-error'; error: ErrorDescription }; /** Optional controller for use with Destructive block */ export function useDestructiveController({ diff --git a/packages/mosaic/src/blocks/destructive/destructive.test.tsx b/packages/mosaic/src/blocks/destructive/destructive.test.tsx index 6bdacdaf973..d7f4b544d94 100644 --- a/packages/mosaic/src/blocks/destructive/destructive.test.tsx +++ b/packages/mosaic/src/blocks/destructive/destructive.test.tsx @@ -7,7 +7,7 @@ import { Button } from '../../components/button'; import { Menu } from '../../components/menu'; import type { ReverificationController } from '../../features/reverification'; import { MosaicProvider } from '../../mosaic-provider'; -import { SaveError } from '../../utils/form-error'; +import { SaveError } from '../../utils/errors'; import type { DestructiveControlledProps, DestructiveHandleProps } from './destructive'; import { Destructive } from './destructive'; diff --git a/packages/mosaic/src/components/form/form.machine.ts b/packages/mosaic/src/components/form/form.machine.ts index c90577f3d88..b01d2893c4c 100644 --- a/packages/mosaic/src/components/form/form.machine.ts +++ b/packages/mosaic/src/components/form/form.machine.ts @@ -1,7 +1,7 @@ import type { LocalizableError } from '../../localization'; import { setup } from '../../machine/setup'; import type { TransitionResult } from '../../machine/types'; -import { SaveError } from '../../utils/form-error'; +import { SaveError } from '../../utils/errors'; import { keysOf, mapKeys } from '../../utils/object'; import type { FieldFeedback, FormError, FormFieldErrors } from './form-submit-error'; import { FormSubmitError } from './form-submit-error'; diff --git a/packages/mosaic/src/components/form/use-form.test.ts b/packages/mosaic/src/components/form/use-form.test.ts index f7b9ce8be8c..98995a5af67 100644 --- a/packages/mosaic/src/components/form/use-form.test.ts +++ b/packages/mosaic/src/components/form/use-form.test.ts @@ -2,7 +2,7 @@ import { act, renderHook } from '@testing-library/react'; import { describe, expect, expectTypeOf, it, vi } from 'vitest'; import { deferred, tick } from '../../__tests__/async'; -import { SaveError } from '../../utils/form-error'; +import { SaveError } from '../../utils/errors'; import type { FieldFeedback } from './form-submit-error'; import { FormSubmitError } from './form-submit-error'; import { useForm } from './use-form'; diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile-account-section.model.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile-account-section.model.test.tsx index 5485c1364b7..9adba0054e8 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile-account-section.model.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile-account-section.model.test.tsx @@ -3,8 +3,8 @@ import type * as SharedReact from '@clerk/shared/react'; import { renderHook } from '@testing-library/react'; import { beforeEach, describe, expect, it, vi } from 'vitest'; -import type { FormError } from '../../../utils/form-error'; -import { SaveError } from '../../../utils/form-error'; +import type { FormError } from '../../../utils/errors'; +import { SaveError } from '../../../utils/errors'; import { useUserProfileAccountSectionModel } from '../user-profile-account-section/user-profile-account-section.model'; interface FakeAttribute { diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile-edit-name.controller.test.ts b/packages/mosaic/src/features/user-profile/__tests__/user-profile-edit-name.controller.test.ts index d746f5d7530..704c644a52a 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile-edit-name.controller.test.ts +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile-edit-name.controller.test.ts @@ -1,7 +1,7 @@ import { act, renderHook, waitFor } from '@testing-library/react'; import { describe, expect, it, vi } from 'vitest'; -import { SaveError } from '../../../utils/form-error'; +import { SaveError } from '../../../utils/errors'; import { useUserProfileEditNameController } from '../user-profile-account-section/user-profile-edit-name.controller'; import type { UserProfileEditNameValue } from '../user-profile-account-section/user-profile-edit-name.dialog'; diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile-edit-name.dialog.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile-edit-name.dialog.test.tsx index c65ac8c800a..90a4f0bb31c 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile-edit-name.dialog.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile-edit-name.dialog.test.tsx @@ -4,7 +4,7 @@ import { describe, expect, it, vi } from 'vitest'; import { Button } from '../../../components/button'; import { MosaicProvider } from '../../../mosaic-provider'; -import { SaveError } from '../../../utils/form-error'; +import { SaveError } from '../../../utils/errors'; import { useUserProfileEditNameController } from '../user-profile-account-section/user-profile-edit-name.controller'; import type { UserProfileEditNameDialogProps, diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile-edit-username.controller.test.ts b/packages/mosaic/src/features/user-profile/__tests__/user-profile-edit-username.controller.test.ts index 4f5b7f27fd0..2c87dcd84ce 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile-edit-username.controller.test.ts +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile-edit-username.controller.test.ts @@ -1,7 +1,7 @@ import { act, renderHook, waitFor } from '@testing-library/react'; import { describe, expect, it, vi } from 'vitest'; -import { SaveError } from '../../../utils/form-error'; +import { SaveError } from '../../../utils/errors'; import { useUserProfileEditUsernameController } from '../user-profile-account-section/user-profile-edit-username.controller'; function deferred() { diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile-edit-username.dialog.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile-edit-username.dialog.test.tsx index 8cee1bbda25..cd3c1c2ff10 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile-edit-username.dialog.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile-edit-username.dialog.test.tsx @@ -5,7 +5,7 @@ import { describe, expect, it, vi } from 'vitest'; import { Button } from '../../../components/button'; import type { MosaicLocalization } from '../../../localization'; import { MosaicProvider } from '../../../mosaic-provider'; -import { SaveError } from '../../../utils/form-error'; +import { SaveError } from '../../../utils/errors'; import { useUserProfileEditUsernameController } from '../user-profile-account-section/user-profile-edit-username.controller'; import type { UserProfileEditUsernameDialogProps } from '../user-profile-account-section/user-profile-edit-username.dialog'; import { UserProfileEditUsernameDialog } from '../user-profile-account-section/user-profile-edit-username.dialog'; diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile-picture-row.view.test.tsx b/packages/mosaic/src/features/user-profile/__tests__/user-profile-picture-row.view.test.tsx index ed4a42f090d..7c77b074811 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile-picture-row.view.test.tsx +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile-picture-row.view.test.tsx @@ -3,7 +3,7 @@ import userEvent from '@testing-library/user-event'; import { describe, expect, it, vi } from 'vitest'; import { MosaicProvider } from '../../../mosaic-provider'; -import { SaveError } from '../../../utils/form-error'; +import { SaveError } from '../../../utils/errors'; import type { UserProfilePictureRowViewProps } from '../user-profile-account-section/user-profile-picture-row.view'; import { UserProfilePictureRowView } from '../user-profile-account-section/user-profile-picture-row.view'; diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile-picture.controller.test.ts b/packages/mosaic/src/features/user-profile/__tests__/user-profile-picture.controller.test.ts index c5abf01570d..6546543daf4 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile-picture.controller.test.ts +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile-picture.controller.test.ts @@ -2,7 +2,7 @@ import { createDeferredPromise } from '@clerk/shared/utils'; import { act, renderHook } from '@testing-library/react'; import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { SaveError } from '../../../utils/form-error'; +import { SaveError } from '../../../utils/errors'; import type { UserProfilePictureControllerOptions } from '../user-profile-account-section/user-profile-picture.controller'; import { useUserProfilePictureController } from '../user-profile-account-section/user-profile-picture.controller'; diff --git a/packages/mosaic/src/features/user-profile/user-profile-account-section/user-profile-account-section.model.ts b/packages/mosaic/src/features/user-profile/user-profile-account-section/user-profile-account-section.model.ts index 9dd21959440..daf5fc22e3f 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-account-section/user-profile-account-section.model.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-account-section/user-profile-account-section.model.ts @@ -4,7 +4,7 @@ import type { AttributeData, EnterpriseAccountResource, UserResource } from '@cl import { useMosaicEnvironment } from '../../../hooks/use-mosaic-environment'; import type { MessageValues } from '../../../localization'; -import { save, SaveError, UNEXPECTED_ERROR } from '../../../utils/form-error'; +import { save, SaveError, UNEXPECTED_ERROR } from '../../../utils/errors'; import type { UserProfileManagedBy } from '../user-profile-managed-by'; import type { UserProfileEmail, diff --git a/packages/mosaic/src/features/user-profile/user-profile-account-section/user-profile-picture.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-account-section/user-profile-picture.controller.ts index 765c08e981d..4d4f2b2226d 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-account-section/user-profile-picture.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-account-section/user-profile-picture.controller.ts @@ -2,7 +2,7 @@ import { useRef, useState } from 'react'; import type { LocalizableError } from '../../../localization'; import { FileUpload } from '../../../primitives/file-upload'; -import { toFormError } from '../../../utils/form-error'; +import { toFormError } from '../../../utils/errors'; export interface UserProfilePictureControllerOptions { onChange?: (file: File) => Promise; diff --git a/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.controller.ts index 055b03ab9bd..eca8f6e705a 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-connected-accounts-section/user-profile-connected-accounts-section.controller.ts @@ -1,8 +1,8 @@ import { useRef, useState } from 'react'; -import type { LocalizableError } from '../../../localization'; +import type { ErrorDescription } from '../../../localization'; import { useErrorText, useMessages } from '../../../localization'; -import { toLocalizableError } from '../../../utils/form-error'; +import { toLocalizableError } from '../../../utils/errors'; import type { ConnectedAccountActionResult, UserProfileConnectedAccount, @@ -35,7 +35,7 @@ export function useUserProfileConnectedAccountsController({ const messages = useMessages('userProfileConnectedAccounts'); const errorText = useErrorText(); const [pendingId, setPendingId] = useState(); - const [errors, setErrors] = useState>({}); + const [errors, setErrors] = useState>({}); const connecting = useRef(false); const run = async (id: string, action: (id: string) => Promise) => { diff --git a/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.controller.ts index ce016ddf28c..06eabb11de5 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-enterprise-accounts-section/user-profile-enterprise-accounts-section.controller.ts @@ -1,8 +1,8 @@ import { useRef, useState } from 'react'; -import type { LocalizableError } from '../../../localization'; +import type { ErrorDescription } from '../../../localization'; import { useErrorText } from '../../../localization'; -import { toLocalizableError } from '../../../utils/form-error'; +import { toLocalizableError } from '../../../utils/errors'; import type { EnterpriseAccountActionResult, UserProfileEnterpriseAccount, @@ -31,7 +31,7 @@ export function useUserProfileEnterpriseAccountsController({ }: UserProfileEnterpriseAccountsControllerOptions): UserProfileEnterpriseAccountsController { const [pendingId, setPendingId] = useState(); const errorText = useErrorText(); - const [connectErrors, setConnectErrors] = useState>({}); + const [connectErrors, setConnectErrors] = useState>({}); const connecting = useRef(false); const run = async (id: string) => { diff --git a/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-errors.ts b/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-errors.ts index 2b5ffd8207c..ac33d87fb78 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-errors.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-errors.ts @@ -3,8 +3,9 @@ import type { ClerkAPIError, PasswordSettingsData } from '@clerk/shared/types'; import { snakeToCamel } from '@clerk/shared/underscore'; import { FormSubmitError } from '../../../components/form'; -import type { LocalizableError, MosaicMessages } from '../../../localization'; +import type { ErrorDescription, MosaicMessages } from '../../../localization'; import { toLocalizableApiError } from '../../../localization'; +import { toLocalizableError } from '../../../utils/errors'; import { passwordComplexityMessage, passwordStrengthMessage } from './user-profile-password-feedback'; import type { UserProfileEditPasswordValues } from './user-profile-password-section.types'; @@ -32,7 +33,7 @@ function passwordError( settings: Settings, messages: Messages, locale: string, - errorText: (error: LocalizableError) => string, + errorText: (error: ErrorDescription) => string, ) { const first = errors[0]; if (!first) { @@ -52,17 +53,17 @@ function passwordError( ); } -// TODO: Move onto save() and toLocalizableError so an unknown error never shows its message, then drop toLocalizableApiError. +// TODO: Move onto save() and drop toLocalizableApiError. export function passwordFormError( error: unknown, requiresCurrentPassword: boolean, settings: Settings, messages: Messages, locale: string, - errorText: (error: LocalizableError) => string, + errorText: (error: ErrorDescription) => string, ): FormSubmitError { if (typeof error !== 'object' || error === null || !isClerkAPIResponseError(error)) { - return new FormSubmitError({ message: errorText({ message: error instanceof Error ? error.message : undefined }) }); + return new FormSubmitError({ message: errorText(toLocalizableError(error)) }); } const fields: { currentPassword?: string; newPassword?: string } = {}; const passwordErrors: ClerkAPIError[] = []; diff --git a/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-feedback.test.ts b/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-feedback.test.ts index a111719c53e..8247ea627a4 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-feedback.test.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-password-section/user-profile-password-feedback.test.ts @@ -2,7 +2,7 @@ import { ClerkAPIResponseError } from '@clerk/shared/error'; import type { PasswordStrength } from '@clerk/shared/types'; import { renderHook } from '@testing-library/react'; import { createElement, type ReactNode } from 'react'; -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import { FormSubmitError } from '../../../components/form'; import { MosaicLocalizationProvider, resolveLocalization, useErrorText } from '../../../localization'; @@ -197,13 +197,15 @@ describe('password error feedback', () => { ).toBe('Your password is not strong enough.'); }); - it('returns form errors for unexpected failures', () => { + it('returns the generic form error for unexpected failures, never their message', () => { + const log = vi.spyOn(console, 'error').mockImplementation(() => {}); expect( passwordFormError(new Error('Connection interrupted'), true, settings, messages, localization.locale, resolver()), - ).toMatchObject({ banner: 'Connection interrupted' }); + ).toMatchObject({ banner: 'Something went wrong. Please try again.' }); expect(passwordFormError(undefined, true, settings, messages, localization.locale, resolver())).toBeInstanceOf( FormSubmitError, ); + log.mockRestore(); }); }); diff --git a/packages/mosaic/src/features/user-profile/user-profile-rename-passkey.controller.ts b/packages/mosaic/src/features/user-profile/user-profile-rename-passkey.controller.ts index eb566b36c46..773e2ff4c17 100644 --- a/packages/mosaic/src/features/user-profile/user-profile-rename-passkey.controller.ts +++ b/packages/mosaic/src/features/user-profile/user-profile-rename-passkey.controller.ts @@ -1,15 +1,15 @@ -import type { LocalizableError } from '../../localization'; +import type { ErrorDescription } from '../../localization'; import { useErrorText, useMessages } from '../../localization'; import { setup } from '../../machine/setup'; import { useMachine } from '../../machine/use-machine'; -import { toLocalizableError } from '../../utils/form-error'; +import { toLocalizableError } from '../../utils/errors'; interface UserProfileRenamePasskeyContext { passkeyId: string; savedName: string; name: string; onRename: ((id: string, name: string) => void | Promise) | undefined; - error: LocalizableError | undefined; + error: ErrorDescription | undefined; } type UserProfileRenamePasskeyEvent = diff --git a/packages/mosaic/src/hooks/use-pending-action.ts b/packages/mosaic/src/hooks/use-pending-action.ts index f981227c8f7..17ee4c44f0b 100644 --- a/packages/mosaic/src/hooks/use-pending-action.ts +++ b/packages/mosaic/src/hooks/use-pending-action.ts @@ -1,8 +1,8 @@ import { useRef, useState } from 'react'; -import type { LocalizableError } from '../localization'; +import type { ErrorDescription } from '../localization'; import { useErrorText } from '../localization'; -import { toLocalizableError } from '../utils/form-error'; +import { toLocalizableError } from '../utils/errors'; export interface PendingActionOptions { /** Copy shown when the action fails without an error Clerk can describe, such as a network or code fault (default: the generic error) */ @@ -26,7 +26,7 @@ export function usePendingAction( ): PendingAction { const errorText = useErrorText(); const [isPending, setIsPending] = useState(false); - const [error, setError] = useState(); + const [error, setError] = useState(); const running = useRef(false); const run = async (...args: TArgs) => { diff --git a/packages/mosaic/src/localization/errors.messages.ts b/packages/mosaic/src/localization/errors.messages.ts index 4827bd81b69..a6f21c93e2f 100644 --- a/packages/mosaic/src/localization/errors.messages.ts +++ b/packages/mosaic/src/localization/errors.messages.ts @@ -20,5 +20,6 @@ export const errorMessages: { readonly generic: string } & Readonly { }); it('falls back to the generic message when there is nothing else', () => { - expect(errorText({ 'errors.generic': 'Algo salió mal.' })({})).toBe('Algo salió mal.'); + expect(errorText({ 'errors.generic': 'Algo salió mal.' })({ cause: new Error('boom') })).toBe('Algo salió mal.'); expect(errorText()({ code: 'toString' })).toBe('Something went wrong. Please try again.'); }); it('uses the fallback it is given ahead of the generic message', () => { - expect(errorText()({}, 'Unable to sign out.')).toBe('Unable to sign out.'); + expect(errorText()({ cause: new Error('boom') }, 'Unable to sign out.')).toBe('Unable to sign out.'); }); it('prefers the copy for the code and the message Clerk sent over the fallback', () => { diff --git a/packages/mosaic/src/localization/errors.ts b/packages/mosaic/src/localization/errors.ts index 5a92380c054..170e6509bdd 100644 --- a/packages/mosaic/src/localization/errors.ts +++ b/packages/mosaic/src/localization/errors.ts @@ -5,13 +5,25 @@ import type { MessageValues } from './messages'; import { fill } from './messages'; export interface LocalizableError { - code?: string; + code: string; paramName?: string; message?: string; /** Values for the `{placeholder}`s in whichever message the code resolves to. */ params?: MessageValues; } +/** A failure Clerk cannot describe, such as a code fault. Rendering it shows the fallback; `cause` is the original. */ +export interface UnlocalizableError { + cause: unknown; +} + +/** What an owner holds about a failure, whether or not Clerk could describe it. */ +export type ErrorDescription = LocalizableError | UnlocalizableError; + +export function isLocalizableError(error: ErrorDescription): error is LocalizableError { + return !('cause' in error); +} + export function toLocalizableApiError(error: ClerkAPIError, fallback?: string): LocalizableError { const message = error.longMessage || error.message; return { @@ -25,12 +37,16 @@ export function toLocalizableApiError(error: ClerkAPIError, fallback?: string): * Turns an error into copy: the catalog entry for its code on that field, then for its code, then the * message Clerk sent, then `fallback`, then the generic error. */ -export function useErrorText(): (error: LocalizableError, fallback?: string) => string { +export function useErrorText(): (error: ErrorDescription, fallback?: string) => string { const messages = useMessages('errors'); const lookup = (key: string | undefined) => (key && Object.hasOwn(messages, key) ? messages[key] : undefined); - return ({ code, paramName, message, params }, fallback) => { + return (error, fallback) => { + if (!isLocalizableError(error)) { + return fallback || messages.generic; + } + const { code, paramName, message, params } = error; const template = - lookup(code && paramName ? `${code}__${paramName}` : undefined) ?? + lookup(paramName ? `${code}__${paramName}` : undefined) ?? lookup(code) ?? (message || fallback || messages.generic); return params ? fill(template, params) : template; diff --git a/packages/mosaic/src/localization/index.ts b/packages/mosaic/src/localization/index.ts index 6fb16605066..349e55081d4 100644 --- a/packages/mosaic/src/localization/index.ts +++ b/packages/mosaic/src/localization/index.ts @@ -1,7 +1,7 @@ export type { MosaicCatalog, MosaicLocalization } from './catalog'; export { MosaicLocalizationProvider, resolveLocalization, useLocale, useMessages } from './context'; -export type { LocalizableError } from './errors'; -export { toLocalizableApiError, useErrorText } from './errors'; +export type { ErrorDescription, LocalizableError, UnlocalizableError } from './errors'; +export { isLocalizableError, toLocalizableApiError, useErrorText } from './errors'; export type { MosaicMessages } from './registry'; export { mosaicMessages } from './registry'; export type { MessageComponents, MessageValues, PluralForms, RichOptions } from './messages'; diff --git a/packages/mosaic/src/utils/errors.test.tsx b/packages/mosaic/src/utils/errors.test.tsx new file mode 100644 index 00000000000..ce5e1583e35 --- /dev/null +++ b/packages/mosaic/src/utils/errors.test.tsx @@ -0,0 +1,82 @@ +import { ClerkAPIResponseError, ClerkRuntimeError } from '@clerk/shared/error'; +import { renderHook } from '@testing-library/react'; +import type { ReactNode } from 'react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { isLocalizableError, MosaicLocalizationProvider, resolveLocalization, useErrorText } from '../localization'; +import { SaveError, toLocalizableError } from './errors'; + +function errorText() { + const wrapper = ({ children }: { children: ReactNode }) => ( + {children} + ); + return renderHook(() => useErrorText(), { wrapper }).result.current; +} + +function silenceConsole() { + return vi.spyOn(console, 'error').mockImplementation(() => {}); +} + +describe('toLocalizableError', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('reads the global error a save left, code and all', () => { + const cause = new SaveError({ global: { code: 'action_blocked', message: 'Raw server sentence.' } }); + expect(toLocalizableError(cause)).toEqual({ code: 'action_blocked', message: 'Raw server sentence.' }); + }); + + it('falls back to the first field error when a save named only fields', () => { + const cause = new SaveError({ fields: { name: { code: 'form_param_nil', paramName: 'name' } } }); + expect(toLocalizableError(cause)).toEqual({ code: 'form_param_nil', paramName: 'name' }); + }); + + it('keeps the cause of a save that carried nothing', () => { + silenceConsole(); + const cause = new SaveError({}); + expect(toLocalizableError(cause)).toEqual({ cause }); + }); + + it('maps a Clerk API error by its code, field and message', () => { + const cause = new ClerkAPIResponseError('Forbidden', { + status: 403, + data: [{ code: 'not_allowed_access', message: 'Short', long_message: 'Long', meta: { param_name: 'role' } }], + }); + expect(toLocalizableError(cause)).toEqual({ code: 'not_allowed_access', paramName: 'role', message: 'Long' }); + }); + + it('maps a Clerk runtime error by its code', () => { + const cause = new ClerkRuntimeError('Something broke.', { code: 'cannot_render_user_missing' }); + expect(toLocalizableError(cause)).toEqual({ code: 'cannot_render_user_missing' }); + }); + + it('localizes a network failure instead of falling back', () => { + const error = toLocalizableError(new ClerkRuntimeError('Failed to fetch', { code: 'network_error' })); + expect(errorText()(error, 'Unable to save.')).toBe( + 'Unable to reach the server. Check your connection and try again.', + ); + }); + + it('keeps the cause of an error Clerk cannot describe and never shows its message', () => { + const log = silenceConsole(); + const cause = new Error('Cannot read properties of undefined'); + const error = toLocalizableError(cause); + expect(error).toEqual({ cause }); + expect(isLocalizableError(error)).toBe(false); + expect(errorText()(error, 'Unable to save.')).toBe('Unable to save.'); + expect(log).toHaveBeenCalledWith('[Mosaic] Could not localize error', cause); + }); + + it('keeps the cause of something that is not an error at all', () => { + silenceConsole(); + expect(toLocalizableError('nope')).toEqual({ cause: 'nope' }); + expect(toLocalizableError(undefined)).toEqual({ cause: undefined }); + }); + + it('keeps the cause of a Clerk API error that carries no errors', () => { + silenceConsole(); + const cause = new ClerkAPIResponseError('Server error', { status: 500, data: [] }); + expect(toLocalizableError(cause)).toEqual({ cause }); + }); +}); diff --git a/packages/mosaic/src/utils/form-error.ts b/packages/mosaic/src/utils/errors.ts similarity index 61% rename from packages/mosaic/src/utils/form-error.ts rename to packages/mosaic/src/utils/errors.ts index 40742824787..d56100b0b2b 100644 --- a/packages/mosaic/src/utils/form-error.ts +++ b/packages/mosaic/src/utils/errors.ts @@ -1,7 +1,7 @@ import { isClerkAPIResponseError, isClerkRuntimeError } from '@clerk/shared/error'; import { snakeToCamel } from '@clerk/shared/underscore'; -import type { LocalizableError, MessageValues } from '../localization'; +import type { ErrorDescription, LocalizableError, MessageValues } from '../localization'; export interface FormError { global?: LocalizableError; @@ -21,40 +21,53 @@ export class SaveError extends Error { } } -function toClerkFormError( - cause: unknown, - fields: readonly TField[], - params: MessageValues | undefined, -): FormError | undefined { +/** Reads every error a Clerk failure carries, or `undefined` when the cause is not from Clerk. */ +function toClerkErrors(cause: unknown, params?: MessageValues): LocalizableError[] | undefined { + if (!(cause instanceof Error)) { + return undefined; + } if (isClerkRuntimeError(cause)) { - return { - global: { + return [ + { code: cause.code, ...(cause.longMessage ? { message: cause.longMessage } : {}), ...(params ? { params } : {}), }, - }; + ]; } if (!isClerkAPIResponseError(cause)) { return undefined; } - const error: FormError = {}; - for (const apiError of cause.errors) { - const paramName = apiError.meta?.paramName; - const localizable: LocalizableError = { - code: apiError.code, + return cause.errors.map(error => { + const paramName = error.meta?.paramName; + return { + code: error.code, ...(paramName ? { paramName } : {}), - message: apiError.longMessage || apiError.message, + message: error.longMessage || error.message, ...(params ? { params } : {}), }; - const field = fields.find(f => paramName && snakeToCamel(paramName) === f); + }); +} + +function toClerkFormError( + cause: unknown, + fields: readonly TField[], + params: MessageValues | undefined, +): FormError | undefined { + const errors = toClerkErrors(cause, params); + if (!errors) { + return undefined; + } + const formError: FormError = {}; + for (const error of errors) { + const field = fields.find(f => error.paramName && snakeToCamel(error.paramName) === f); if (field) { - error.fields = { ...error.fields, [field]: error.fields?.[field] ?? localizable }; + formError.fields = { ...formError.fields, [field]: formError.fields?.[field] ?? error }; } else { - error.global ??= localizable; + formError.global ??= error; } } - return error; + return formError; } /** @@ -80,16 +93,19 @@ export async function save( } /** - * Reads what to tell the user about a failed action. Clerk errors keep their code so the catalog can - * localize them; anything else is logged and carries nothing, leaving the caller's fallback to show. + * Reads what to tell the user about a failed action. A Clerk error keeps its code so the catalog can + * localize it; anything else keeps its cause, is logged, and renders as the caller's fallback. */ -export function toLocalizableError(cause: unknown): LocalizableError { - const formError = cause instanceof SaveError ? cause.formError : toClerkFormError(cause, [], undefined); - if (formError) { - return formError.global ?? Object.values(formError.fields ?? {}).find(error => error !== undefined) ?? {}; +export function toLocalizableError(cause: unknown): ErrorDescription { + const error = + cause instanceof SaveError + ? (cause.formError.global ?? Object.values(cause.formError.fields ?? {}).find(field => field !== undefined)) + : toClerkErrors(cause)?.[0]; + if (error) { + return error; } - console.error(cause); - return {}; + console.error('[Mosaic] Could not localize error', cause); + return { cause }; } /** Reads what a rejected save left for the view. An unrecognized rejection is the generic error. */ diff --git a/packages/mosaic/src/utils/form-error.test.ts b/packages/mosaic/src/utils/form-error.test.ts deleted file mode 100644 index 8accc0c9c1a..00000000000 --- a/packages/mosaic/src/utils/form-error.test.ts +++ /dev/null @@ -1,60 +0,0 @@ -import { ClerkAPIResponseError, ClerkRuntimeError } from '@clerk/shared/error'; -import { afterEach, describe, expect, it, vi } from 'vitest'; - -import { SaveError, toLocalizableError } from './form-error'; - -function silenceConsole() { - return vi.spyOn(console, 'error').mockImplementation(() => {}); -} - -describe('toLocalizableError', () => { - afterEach(() => { - vi.restoreAllMocks(); - }); - - it('reads the global error a save left, code and all', () => { - const cause = new SaveError({ global: { code: 'action_blocked', message: 'Raw server sentence.' } }); - expect(toLocalizableError(cause)).toEqual({ - code: 'action_blocked', - message: 'Raw server sentence.', - }); - }); - - it('falls back to the first field error when a save named only fields', () => { - const cause = new SaveError({ fields: { name: { code: 'form_param_nil', paramName: 'name' } } }); - expect(toLocalizableError(cause)).toEqual({ code: 'form_param_nil', paramName: 'name' }); - }); - - it('has nothing to say when a save carried nothing', () => { - expect(toLocalizableError(new SaveError({}))).toEqual({}); - }); - - it('maps a Clerk API error that never went through save', () => { - const cause = new ClerkAPIResponseError('Forbidden', { - status: 403, - data: [{ code: 'not_allowed_access', message: 'Short', long_message: 'Long', meta: { param_name: 'role' } }], - }); - expect(toLocalizableError(cause)).toEqual({ - code: 'not_allowed_access', - paramName: 'role', - message: 'Long', - }); - }); - - it('maps a Clerk runtime error by its code', () => { - const cause = new ClerkRuntimeError('Network down.', { code: 'network_error' }); - expect(toLocalizableError(cause)).toMatchObject({ code: 'network_error' }); - }); - - it('never shows the message of a plain error', () => { - const log = silenceConsole(); - const cause = new Error('Cannot read properties of undefined'); - expect(toLocalizableError(cause)).toEqual({}); - expect(log).toHaveBeenCalledWith(cause); - }); - - it('has nothing to say for something that is not an error at all', () => { - silenceConsole(); - expect(toLocalizableError('nope')).toEqual({}); - }); -}); From 25bb35541eefbba8370bc7a6e7e25b898daecdcb Mon Sep 17 00:00:00 2001 From: Alex Carpenter Date: Tue, 6 Oct 2026 08:24:54 -0400 Subject: [PATCH 5/7] fix(mosaic): label unlocalized error logs with [Clerk] --- .claude/skills/mosaic/references/controllers.md | 2 +- .claude/skills/mosaic/references/machines.md | 2 +- packages/mosaic/src/components/form/form.machine.ts | 2 +- packages/mosaic/src/components/form/use-form.test.ts | 2 +- .../user-button/__tests__/user-button.feature.test.tsx | 1 + .../src/features/user-button/user-button.controller.tsx | 5 ++++- .../__tests__/user-profile-picture.controller.test.ts | 2 +- packages/mosaic/src/utils/errors.test.tsx | 2 +- packages/mosaic/src/utils/errors.ts | 4 ++-- 9 files changed, 13 insertions(+), 9 deletions(-) diff --git a/.claude/skills/mosaic/references/controllers.md b/.claude/skills/mosaic/references/controllers.md index 526e1b90180..bc51468a921 100644 --- a/.claude/skills/mosaic/references/controllers.md +++ b/.claude/skills/mosaic/references/controllers.md @@ -147,7 +147,7 @@ One path, so every error a user sees goes through the `errors.*` catalog: - **Owners hold an `ErrorDescription`.** It is either a `LocalizableError` (Clerk gave a `code`) or an `UnlocalizableError` (`{ cause }`, the original failure). `toLocalizableError` logs the second as - `[Mosaic] Could not localize error`. Branch with `isLocalizableError` only + `[Clerk] Could not localize error`. Branch with `isLocalizableError` only when a feature handles the two differently. - **Owners resolve copy at render; views get a string.** Owners return `errorMessage` from `useErrorText()`: the diff --git a/.claude/skills/mosaic/references/machines.md b/.claude/skills/mosaic/references/machines.md index 0d72e5ed840..00dafc70a09 100644 --- a/.claude/skills/mosaic/references/machines.md +++ b/.claude/skills/mosaic/references/machines.md @@ -49,7 +49,7 @@ deleting: { ``` A machine whose surface shows no error yet still logs it, so a failure is never -silent: `onError: { target: 'idle', actions: [assign(() => settled), (_, event) => console.error(event.error)] }` +silent: `onError: { target: 'idle', actions: [assign(() => settled), (_, event) => console.error('[Clerk] User button action failed', event.error)] }` (`user-button.controller.tsx`). Context holds the `ErrorDescription`, never copy. The controller turns it into diff --git a/packages/mosaic/src/components/form/form.machine.ts b/packages/mosaic/src/components/form/form.machine.ts index b01d2893c4c..92ce1d89b9f 100644 --- a/packages/mosaic/src/components/form/form.machine.ts +++ b/packages/mosaic/src/components/form/form.machine.ts @@ -149,7 +149,7 @@ function savedFieldErrors( function toFormError(cause: unknown, context: FormContext): FormError { if (!(cause instanceof FormSubmitError) && !(cause instanceof SaveError)) { - console.error(cause); + console.error('[Clerk] Could not localize error', cause); return { message: context.fallbackMessage }; } const error: FormError = diff --git a/packages/mosaic/src/components/form/use-form.test.ts b/packages/mosaic/src/components/form/use-form.test.ts index 98995a5af67..e96afb45a1d 100644 --- a/packages/mosaic/src/components/form/use-form.test.ts +++ b/packages/mosaic/src/components/form/use-form.test.ts @@ -146,7 +146,7 @@ describe('useForm', () => { await tick(); }); expect(result.current.error).toBe('Something went wrong. Please try again.'); - expect(log).toHaveBeenCalledWith(cause); + expect(log).toHaveBeenCalledWith('[Clerk] Could not localize error', cause); } log.mockRestore(); }); diff --git a/packages/mosaic/src/features/user-button/__tests__/user-button.feature.test.tsx b/packages/mosaic/src/features/user-button/__tests__/user-button.feature.test.tsx index f87f61be1d8..8d8909398a0 100644 --- a/packages/mosaic/src/features/user-button/__tests__/user-button.feature.test.tsx +++ b/packages/mosaic/src/features/user-button/__tests__/user-button.feature.test.tsx @@ -608,6 +608,7 @@ describe('UserButton', () => { expect(popup()).toBeInTheDocument(); expect(signOut()).toBeEnabled(); expect(log).toHaveBeenCalledWith( + '[Clerk] User button action failed', expect.objectContaining({ errors: [expect.objectContaining({ code: 'session_touch_failed' })] }), ); }); diff --git a/packages/mosaic/src/features/user-button/user-button.controller.tsx b/packages/mosaic/src/features/user-button/user-button.controller.tsx index 1c3861966d2..1ed13abd0c7 100644 --- a/packages/mosaic/src/features/user-button/user-button.controller.tsx +++ b/packages/mosaic/src/features/user-button/user-button.controller.tsx @@ -86,7 +86,10 @@ const userButtonMachine = createMachine({ // Leave `open` as the user left it. The error surface is a later change. onError: { target: 'idle', - actions: [assign(() => settled), (_, event) => console.error(event.error)], + actions: [ + assign(() => settled), + (_, event) => console.error('[Clerk] User button action failed', event.error), + ], }, }), }, diff --git a/packages/mosaic/src/features/user-profile/__tests__/user-profile-picture.controller.test.ts b/packages/mosaic/src/features/user-profile/__tests__/user-profile-picture.controller.test.ts index 6546543daf4..6850861809b 100644 --- a/packages/mosaic/src/features/user-profile/__tests__/user-profile-picture.controller.test.ts +++ b/packages/mosaic/src/features/user-profile/__tests__/user-profile-picture.controller.test.ts @@ -88,7 +88,7 @@ describe('useUserProfilePictureController', () => { await act(async () => result.current.onChange?.(file)); expect(result.current.error).toEqual({ code: 'generic' }); - expect(log).toHaveBeenCalledWith(failure); + expect(log).toHaveBeenCalledWith('[Clerk] Could not localize error', failure); log.mockRestore(); }); diff --git a/packages/mosaic/src/utils/errors.test.tsx b/packages/mosaic/src/utils/errors.test.tsx index ce5e1583e35..e1175b97ee0 100644 --- a/packages/mosaic/src/utils/errors.test.tsx +++ b/packages/mosaic/src/utils/errors.test.tsx @@ -65,7 +65,7 @@ describe('toLocalizableError', () => { expect(error).toEqual({ cause }); expect(isLocalizableError(error)).toBe(false); expect(errorText()(error, 'Unable to save.')).toBe('Unable to save.'); - expect(log).toHaveBeenCalledWith('[Mosaic] Could not localize error', cause); + expect(log).toHaveBeenCalledWith('[Clerk] Could not localize error', cause); }); it('keeps the cause of something that is not an error at all', () => { diff --git a/packages/mosaic/src/utils/errors.ts b/packages/mosaic/src/utils/errors.ts index d56100b0b2b..20fb5650616 100644 --- a/packages/mosaic/src/utils/errors.ts +++ b/packages/mosaic/src/utils/errors.ts @@ -104,7 +104,7 @@ export function toLocalizableError(cause: unknown): ErrorDescription { if (error) { return error; } - console.error('[Mosaic] Could not localize error', cause); + console.error('[Clerk] Could not localize error', cause); return { cause }; } @@ -113,6 +113,6 @@ export function toFormError(cause: unknown): For if (cause instanceof SaveError) { return cause.formError; } - console.error(cause); + console.error('[Clerk] Could not localize error', cause); return { global: UNEXPECTED_ERROR }; } From 9ed33a338a1102fd1e03a53a88872f6fbe3f7826 Mon Sep 17 00:00:00 2001 From: Alex Carpenter Date: Tue, 6 Oct 2026 08:35:37 -0400 Subject: [PATCH 6/7] fix(swingset): import Mosaic errors from utils/errors and give fixture errors a code --- .../src/stories/fixtures/user-profile-edit-name.ts | 4 ++-- .../stories/fixtures/user-profile-edit-username.ts | 4 ++-- .../stories/user-profile-account-section.stories.tsx | 12 +++++++----- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/packages/swingset/src/stories/fixtures/user-profile-edit-name.ts b/packages/swingset/src/stories/fixtures/user-profile-edit-name.ts index 083b7436224..9c0068093a6 100644 --- a/packages/swingset/src/stories/fixtures/user-profile-edit-name.ts +++ b/packages/swingset/src/stories/fixtures/user-profile-edit-name.ts @@ -1,6 +1,6 @@ import type { UserProfileEditNameValue } from '@clerk/mosaic/features/user-profile/user-profile-account-section/user-profile-edit-name.dialog'; -import type { FormError } from '@clerk/mosaic/utils/form-error'; -import { SaveError } from '@clerk/mosaic/utils/form-error'; +import type { FormError } from '@clerk/mosaic/utils/errors'; +import { SaveError } from '@clerk/mosaic/utils/errors'; import { useState } from 'react'; import { useChaosFixture } from '@/components/ChaosProvider'; diff --git a/packages/swingset/src/stories/fixtures/user-profile-edit-username.ts b/packages/swingset/src/stories/fixtures/user-profile-edit-username.ts index 67a08f99aea..5e1dc6115b3 100644 --- a/packages/swingset/src/stories/fixtures/user-profile-edit-username.ts +++ b/packages/swingset/src/stories/fixtures/user-profile-edit-username.ts @@ -1,5 +1,5 @@ -import type { FormError } from '@clerk/mosaic/utils/form-error'; -import { SaveError } from '@clerk/mosaic/utils/form-error'; +import type { FormError } from '@clerk/mosaic/utils/errors'; +import { SaveError } from '@clerk/mosaic/utils/errors'; import { useState } from 'react'; import { useChaosFixture } from '@/components/ChaosProvider'; diff --git a/packages/swingset/src/stories/user-profile-account-section.stories.tsx b/packages/swingset/src/stories/user-profile-account-section.stories.tsx index 54a81a99f38..407cae0a6f8 100644 --- a/packages/swingset/src/stories/user-profile-account-section.stories.tsx +++ b/packages/swingset/src/stories/user-profile-account-section.stories.tsx @@ -7,7 +7,7 @@ import { UserProfileAccountSectionView } from '@clerk/mosaic/features/user-profi import type { UserProfileAddPhoneDialogProps } from '@clerk/mosaic/features/user-profile/user-profile-account-section/user-profile-add-phone.dialog'; import { UserProfileVerifyEmailLinkDialog } from '@clerk/mosaic/features/user-profile/user-profile-account-section/user-profile-verify-email-link.dialog'; import { UserProfileVerifyEmailSsoDialog } from '@clerk/mosaic/features/user-profile/user-profile-account-section/user-profile-verify-email-sso.dialog'; -import type { FormError } from '@clerk/mosaic/utils/form-error'; +import type { FormError } from '@clerk/mosaic/utils/errors'; import { useState } from 'react'; import type { StoryMeta } from '@/lib/types'; @@ -224,8 +224,10 @@ export function EditNameFails() { ); @@ -236,8 +238,8 @@ export function EditUsernameFails() { ); From 7dee06bd77ff340a690321ed44a7b73c198fe984 Mon Sep 17 00:00:00 2001 From: Alex Carpenter Date: Tue, 6 Oct 2026 08:35:38 -0400 Subject: [PATCH 7/7] chore(mosaic): prefix dev warnings with [Clerk] --- packages/mosaic/src/components/dialog/dialog.tsx | 6 +++--- packages/mosaic/src/components/field/field.context.tsx | 2 +- packages/mosaic/src/components/field/field.test.tsx | 4 ++-- packages/mosaic/src/components/field/field.tsx | 2 +- .../mosaic/src/hooks/use-accessible-description-warning.ts | 2 +- packages/mosaic/src/hooks/use-accessible-name-warning.ts | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/mosaic/src/components/dialog/dialog.tsx b/packages/mosaic/src/components/dialog/dialog.tsx index 75d6f808d88..fd5353ff9a1 100644 --- a/packages/mosaic/src/components/dialog/dialog.tsx +++ b/packages/mosaic/src/components/dialog/dialog.tsx @@ -195,7 +195,7 @@ function useCloseButtonWarning(isAlert: boolean) { return; } console.warn( - '[clerk] is rendered inside an alert dialog. A corner X is a way out without answering; the cancel action in the surface is the way out.', + '[Clerk] is rendered inside an alert dialog. A corner X is a way out without answering; the cancel action in the surface is the way out.', ); }, [isAlert]); } @@ -309,7 +309,7 @@ function useNestedVariantWarning(isNestedInDialog: boolean, variant: DialogVaria return; } console.warn( - '[clerk] a variant="profile" Dialog opened inside another Dialog. A profile is a root-level surface that hosts what opens over it; open a card instead.', + '[Clerk] a variant="profile" Dialog opened inside another Dialog. A profile is a root-level surface that hosts what opens over it; open a card instead.', ); }, [isNestedInDialog, variant]); } @@ -326,7 +326,7 @@ function useCompactPlacementWarning(variant: DialogVariant, placement: DialogCom return; } console.warn( - `[clerk] — a profile fills the compact band and takes no placement. It was ignored.`, + `[Clerk] — a profile fills the compact band and takes no placement. It was ignored.`, ); }, [variant, placement]); } diff --git a/packages/mosaic/src/components/field/field.context.tsx b/packages/mosaic/src/components/field/field.context.tsx index d1895463cea..7c64c431e1b 100644 --- a/packages/mosaic/src/components/field/field.context.tsx +++ b/packages/mosaic/src/components/field/field.context.tsx @@ -63,7 +63,7 @@ export function FieldProvider({ children, orientation, disabled, required, inval ) { warnedAboutMultipleControls.current = true; console.warn( - '[clerk] supports a single form control. Use a separate for each control or native
semantics for grouped controls.', + '[Clerk] supports a single form control. Use a separate for each control or native
semantics for grouped controls.', ); } diff --git a/packages/mosaic/src/components/field/field.test.tsx b/packages/mosaic/src/components/field/field.test.tsx index d686061414d..07724fd91f5 100644 --- a/packages/mosaic/src/components/field/field.test.tsx +++ b/packages/mosaic/src/components/field/field.test.tsx @@ -347,7 +347,7 @@ describe('Mosaic Field', () => { expect(consoleWarn).toHaveBeenCalledTimes(1); expect(consoleWarn).toHaveBeenCalledWith( - '[clerk] supports a single form control. Use a separate for each control or native
semantics for grouped controls.', + '[Clerk] supports a single form control. Use a separate for each control or native
semantics for grouped controls.', ); consoleWarn.mockRestore(); }); @@ -747,7 +747,7 @@ describe('Mosaic Field', () => { render(}>Email); - expect(consoleWarn).toHaveBeenCalledWith('[clerk] must render a native `