diff --git a/AGENTS.md b/AGENTS.md index 4bc66445..57bede2e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -22,10 +22,33 @@ For example, prefer one state of type `'loading' | { error: E } | { result: T }` to three states `[loading, setLoading] = useState(); [error, setError] = useState(); [result, setResult] = useState()`. - For UI actions that hit the server, -prefer `useServerAction` (`@/lib/util`) or (if `useServerAction` doesn't work) `useActionState` +prefer `useServerAction` (`@/lib/client/util`) or (if `useServerAction` doesn't work) `useActionState` over manually storing response/error state with `useState`. - To call Server Functions on mount (e.g. to fetch data), use SWR. - + +# Error handling + +- Next.js interrupts are preferred when there is something appropriate available (e.g. authentication errors, forbidden()) +- ActionResponse { error } states are only for failures the user is expected to be able to encounter during usual operation, + and is expected to be able to act on (e.g. "A project with that name already exists"). +- Everything else (bugs, unreachable services, misconfigured hosts) should throw to an error boundary. +- Errors thrown within the app should not be converted to ActionResponse errors, and vice versa. + +Some details/consequences: + +- Only render-phase throws hit error boundaries; + exceptions originating from callbacks, event handlers, and timeouts must not be silently ignored. + Next.js gives advice for handling these cases (https://nextjs.org/docs/app/getting-started/error-handling), + generally we want to show the error to the user in the component + or else re-raise to throw to an error boundary (`throwToBoundary` is useful here). + (depending on whether it's user-actionable or not, as described above). +- SWR puts a fetcher's throw in `error`; + use `useThrowingSWR` in the common case that this should be re-raised to an error boundary. + Sometimes SWR errors should be shown to the user instead (use the primitive `useSWR` in these cases), + but they should not be suppressed or ignored. +- Especially in administrative contexts, the expected/unexpected error distinction is blurry. + Don't let the guidelines prevent an admin from seeing useful information in the web interface. + # Agent instructions - Less code is better. After writing any new piece of code, diff --git a/doc/DEVELOPMENT.md b/doc/DEVELOPMENT.md index 5a7a6ce9..d88509d6 100644 --- a/doc/DEVELOPMENT.md +++ b/doc/DEVELOPMENT.md @@ -8,7 +8,7 @@ It describes how to locally run and test the workbench software. - Docker installed and running, with at least 16GB memory allocated (in Docker Desktop, go to Settings -> Resources -> Memory). -- Node v24 or later is needed for `make` to work +- Node v24 or later is needed for `make container` to work ## Running the workbench server diff --git a/src/app/AvatarMenu.tsx b/src/app/AvatarMenu.tsx index 49ff9434..2665ec31 100644 --- a/src/app/AvatarMenu.tsx +++ b/src/app/AvatarMenu.tsx @@ -5,6 +5,7 @@ import { useRouter } from 'next/navigation' import AvatarIcon from '@/app/components/AvatarIcon' import authClient from '@/lib/client/auth' +import { useThrowToBoundary } from '@/lib/client/util' import { useConfigCtx } from '@/lib/contexts' import { setIsAdmin } from '@/lib/server/actions' @@ -12,6 +13,7 @@ export default function AvatarMenu() { const session = authClient.useSession() const cfg = useConfigCtx() const router = useRouter() + const { throwToBoundary } = useThrowToBoundary() if (session.data) { const user = session.data.user @@ -25,9 +27,10 @@ export default function AvatarMenu() { {user.isAdmin && Admin interface} {cfg.isDevMode && ( + return ( + + ) } else { return <> } diff --git a/src/app/[userName]/NewProjectForm.tsx b/src/app/[userName]/NewProjectForm.tsx index 50f6fbd8..a3a12751 100644 --- a/src/app/[userName]/NewProjectForm.tsx +++ b/src/app/[userName]/NewProjectForm.tsx @@ -2,25 +2,16 @@ import { useRouter } from 'next/navigation' import { useState } from 'react' -import useSWR from 'swr' -import { useServerAction } from '@/lib/client/util' +import { useServerAction, useThrowingSWR } from '@/lib/client/util' import { formString } from '@/lib/util' -import { createProject, listTemplates, type TemplateInfo } from './actions' +import { createProject, listTemplates } from './actions' export function NewProjectForm() { const router = useRouter() const [open, setOpen] = useState(false) - const { - data: templates, - error: templatesError, - isLoading: templatesPending, - } = useSWR('listTemplates', async () => { - const result = await listTemplates() - if ('error' in result) throw new Error(result.error) - return result.ok - }) + const { data: templates, isLoading: templatesPending } = useThrowingSWR('listTemplates', listTemplates) const [chosenTemplate, setChosenTemplate] = useState('blank') @@ -46,8 +37,6 @@ export function NewProjectForm() { ) } - const error = templatesError ?? createError - return (
))} - {error &&
{error}
} + {createError &&
{createError}
}