Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .changeset/mosaic-to-localizable-error.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
---
Comment thread
alexcarpenter marked this conversation as resolved.
45 changes: 45 additions & 0 deletions .claude/skills/mosaic/references/controllers.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,21 @@ 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 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:

- 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,
Expand Down Expand Up @@ -120,6 +135,36 @@ have a machine.
}
```

## Errors

One path, so every error a user sees goes through the `errors.*` catalog:

- **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
`[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
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**
(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.

## Testing

The feature test covers the controller by default. Hold the
Expand Down
12 changes: 9 additions & 3 deletions .claude/skills/mosaic/references/machines.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,20 @@ 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) })),
},
}),
}
```

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('[Clerk] User button action failed', event.error)] }`
(`user-button.controller.tsx`).

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").

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`.
5 changes: 5 additions & 0 deletions .claude/skills/mosaic/references/models.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
13 changes: 9 additions & 4 deletions .claude/skills/mosaic/references/views.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 15 additions & 0 deletions packages/mosaic/src/__tests__/clerk-errors.ts
Original file line number Diff line number Diff line change
@@ -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,
});
}
Original file line number Diff line number Diff line change
@@ -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/errors';
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' });
Expand Down Expand Up @@ -43,24 +50,26 @@ 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('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();
// 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(cause) });

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({ cause });
});

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' });

Expand All @@ -85,14 +94,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.'));
});
});
24 changes: 16 additions & 8 deletions packages/mosaic/src/blocks/confirmation/confirmation.controller.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import type { ErrorDescription } from '../../localization';
import { useErrorText } from '../../localization';
import { setup } from '../../machine/setup';
import { useMachine } from '../../machine/use-machine';
import { toLocalizableError } from '../../utils/errors';

export interface ConfirmationContext {
run: () => Promise<void>;
error: string | undefined;
error: ErrorDescription | undefined;
}

export type ConfirmationEvent = { type: 'OPEN' } | { type: 'CONFIRM'; run: () => Promise<void> } | { type: 'CANCEL' };
Expand All @@ -14,10 +17,6 @@ function notSeated(): Promise<never> {
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',
Expand All @@ -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) })),
},
}),
},
Expand All @@ -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,
};
}
66 changes: 60 additions & 6 deletions packages/mosaic/src/blocks/confirmation/confirmation.test.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -101,11 +103,15 @@ interface Member {

const preston: Member = { id: 'mem_1', name: 'Preston Booth' };

function renderWithHandle(onConfirm: ConfirmationHandleProps<Member>['onConfirm'] = () => Promise.resolve()) {
function renderWithHandle(
onConfirm: ConfirmationHandleProps<Member>['onConfirm'] = () => Promise.resolve(),
{ errorFallback, localization }: { errorFallback?: string; localization?: MosaicLocalization } = {},
) {
const handle = Confirmation.createHandle<Member>();
render(
<MosaicProvider>
<MosaicProvider localization={localization}>
<Confirmation
errorFallback={errorFallback}
handle={handle}
title='Remove member'
description={member => (
Expand All @@ -121,6 +127,10 @@ function renderWithHandle(onConfirm: ConfirmationHandleProps<Member>['onConfirm'
return handle;
}

afterEach(() => {
vi.restoreAllMocks();
});

const removeButton = () => screen.getByRole('button', { name: 'Remove Preston Booth' });

describe('Confirmation with a handle', () => {
Expand Down Expand Up @@ -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);
});
Expand All @@ -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());
Expand Down
Loading
Loading