refactor: add error boundaries, improve error handling

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,

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
 

src/app/AvatarMenu.tsx

@@ -5,13 +5,15 @@ 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'
 
 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,23 +27,26 @@ export default function AvatarMenu() {
             {user.isAdmin && <Link href='/admin'>Admin interface</Link>}
             {cfg.isDevMode && (
               <button
-                onClick={async () => {
-                  await setIsAdmin(!user.isAdmin)
-                  session.refetch()
+                onClick={() => {
+                  setIsAdmin(!user.isAdmin)
+                    .then(() => session.refetch())
+                    .catch(throwToBoundary)
                 }}
               >
                 {user.isAdmin ? '[DEV] Become non-admin' : '[DEV] Become admin'}
               </button>
             )}
             <button
               onClick={() => {
-                authClient.signOut({
-                  fetchOptions: {
-                    onSuccess: () => {
-                      router.push('/')
+                authClient
+                  .signOut({
+                    fetchOptions: {
+                      onSuccess: () => {
+                        router.push('/')
+                      },
                     },
-                  },
-                })
+                  })
+                  .catch(throwToBoundary)
               }}
             >
               Sign out
@@ -51,7 +56,15 @@ export default function AvatarMenu() {
       </>
     )
   } else if (!session.isPending && cfg.hasGithubAuth) {
-    return <button onClick={() => authClient.signIn.social({ provider: 'github' })}>Sign in via GitHub</button>
+    return (
+      <button
+        onClick={() => {
+          authClient.signIn.social({ provider: 'github' }).catch(throwToBoundary)
+        }}
+      >
+        Sign in via GitHub
+      </button>
+    )
   } else {
     return <></>
   }

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<TemplateInfo[], string>('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<string>('blank')
 
@@ -46,8 +37,6 @@ export function NewProjectForm() {
     )
   }
 
-  const error = templatesError ?? createError
-
   return (
     <form action={createAction} className='new-project' style={{ marginTop: 16 }}>
       <input
@@ -76,7 +65,7 @@ export function NewProjectForm() {
           </button>
         ))}
       </div>
-      {error && <div style={{ color: '#dc2626', fontSize: 13, marginBottom: 8 }}>{error}</div>}
+      {createError && <div style={{ color: '#dc2626', fontSize: 13, marginBottom: 8 }}>{createError}</div>}
       <div>
         <button type='submit' disabled={createPending}>
           Create

src/app/[userName]/[projectName]/page.tsx

@@ -1,7 +1,6 @@
 import { notFound } from 'next/navigation'
 import z from 'zod'
 
-import Error from '@/app/components/Error'
 import { requireAuth } from '@/lib/server/auth'
 import { getDb } from '@/lib/server/db'
 import { getEditorSessionManager } from '@/lib/server/editorSessions'
@@ -36,13 +35,8 @@ export default async function EditorSession({ params: params_ }: { params: Promi
   if (!project || !canAccessProject(viewer, project)) notFound()
 
   const manager = getEditorSessionManager()
-  let iframeSrc: string
-  try {
-    iframeSrc = await manager.ensureSession(viewer, owner, project)
-  } catch (err) {
-    console.error('Failed to start editor session:', (err as Error).message)
-    return <Error>Failed to start editor session: {String(err)}</Error>
-  }
+  // may throw to error boundary (e.g. if the project folder isn't accessible)
+  const iframeSrc = await manager.ensureSession(viewer, owner, project)
 
   // TODO: VSC should be sandboxed but can't be opaque-origin: need a subdomain.
   return <iframe id='editor-frame' src={iframeSrc} className='editor-session-iframe' />

src/app/[userName]/actions.ts

@@ -22,10 +22,13 @@ const zTemplateMetadata = z.object({
 
 type TemplateMetadata = z.infer<typeof zTemplateMetadata>
 
-async function readTemplateMetadata(templateDir: string): Promise<TemplateMetadata | null> {
+/**
+ * Read `metadata.json` from `templateDir`,
+ * raising an exception if the file is missing or unparseable.
+ */
+async function readTemplateMetadata(templateDir: string): Promise<TemplateMetadata> {
   const metaPath = path.join(templateDir, 'metadata.json')
-  const raw = await fs.readFile(metaPath, 'utf-8').catch(() => null)
-  if (raw === null) return null
+  const raw = await fs.readFile(metaPath, 'utf-8')
   return zTemplateMetadata.parse(JSON.parse(raw))
 }
 
@@ -43,26 +46,31 @@ export interface TemplateInfo {
 
 // --- Queries ---
 
-export async function listTemplates(): Promise<ActionResponse<TemplateInfo[]>> {
+export async function listTemplates(): Promise<TemplateInfo[]> {
   await requireAuth()
 
   const templatesDir = getTemplatesDir()
 
   const result: TemplateInfo[] = [{ id: 'blank', name: 'Blank', description: 'Empty workspace' }]
-  const entries = await fs.readdir(templatesDir, { withFileTypes: true }).catch(() => [])
+  const entries = await fs.readdir(templatesDir, { withFileTypes: true })
 
   for (const entry of entries) {
     if (!entry.isDirectory()) continue
-    const meta = await readTemplateMetadata(path.join(entry.parentPath, entry.name))
-    if (!meta) continue
+    let meta: TemplateMetadata
+    try {
+      meta = await readTemplateMetadata(path.join(entry.parentPath, entry.name))
+    } catch (err) {
+      console.error(`Skipping template '${entry.name}' due to metadata error`, err)
+      continue
+    }
     result.push({
       id: entry.name,
       name: meta.name,
       description: meta.description ?? '',
     })
   }
 
-  return { ok: result }
+  return result
 }
 
 // --- Mutations ---
@@ -84,13 +92,10 @@ export const createProject = serverAction(
     if (template !== 'blank') {
       const templateDir = path.join(getTemplatesDir(), template)
       const meta = await readTemplateMetadata(templateDir)
-      if (!meta) return { error: `Template "${template}" not found` }
       if (meta.packageSet) {
         const packagesFile = path.join(getPackageSetsDir(), meta.packageSet, 'packages.txt')
         if (!(await existsAsync(packagesFile))) {
-          return {
-            error: `Package set "${meta.packageSet}" not found. Run seed-volume.sh first.`,
-          }
+          throw new Error(`Package set "${meta.packageSet}" not found. Run seed-volume.sh first.`)
         }
       }
     }

src/app/admin/actions.ts

@@ -196,7 +196,8 @@ export async function fetchDiskUsage(): Promise<ActionResponse<{ workspaces: str
     const size = out.split('\t')[0] ?? '?'
     return { ok: { workspaces: size } }
   } catch (e: unknown) {
-    return { error: `Failed to compute disk usage: ${e instanceof Error ? e.message : String(e)}` }
+    console.error(`Failed to compute directory usage`, e)
+    return { error: `Could not compute size (the directory may be too large)` }
   }
 }
 

src/app/admin/components/OAuthConfig.tsx

@@ -1,14 +1,14 @@
 'use client'
 
 import { useState } from 'react'
-import useSWR from 'swr'
 
 import { fetchOAuthConfig, updateOAuthConfig } from '@/app/admin/actions'
-import { useServerAction } from '@/lib/client/util'
+import { useServerAction, useThrowingSWR } from '@/lib/client/util'
 import { formString } from '@/lib/util'
 
 export function OAuthConfig() {
-  const { data, mutate } = useSWR('adminOAuthConfig', () => fetchOAuthConfig())
+  const { data, mutate } = useThrowingSWR('adminOAuthConfig', () => fetchOAuthConfig())
+
   const [editing, setEditing] = useState(false)
 
   const [error, action, pending] = useServerAction(

src/app/components/Error.tsx → src/app/components/ErrorBox.tsx

@@ -1,4 +1,4 @@
-export default function Error({ children }: { children: React.ReactNode }) {
+export default function ErrorBox({ children }: { children: React.ReactNode }) {
   return (
     <div
       style={{

src/app/error.tsx

@@ -0,0 +1,53 @@
+'use client'
+
+import { useEffect } from 'react'
+import { useSWRConfig } from 'swr'
+
+import ErrorBox from './components/ErrorBox'
+
+interface ErrorBoundaryProps {
+  error: Error & { digest?: string }
+  retry: () => void
+}
+
+/**
+ * App-level error boundary
+ * https://nextjs.org/docs/app/api-reference/file-conventions/error
+ */
+export default function ErrorBoundary({ error, retry }: ErrorBoundaryProps) {
+  const { mutate } = useSWRConfig()
+  useEffect(() => {
+    console.error('Unhandled error:', error, error.digest)
+  }, [error])
+
+  return (
+    <div>
+      <h1>Something went wrong!</h1>
+      <ErrorBox>
+        {/* digest is only present in server-side errors */}
+        {error.digest && (
+          <p>
+            Something went wrong on the server. Try again, or contact your administrator with the error code{' '}
+            {error.digest} if the problem persists.
+          </p>
+        )}
+        {!error.digest && <p>An unexpected error occurred: {error.message}</p>}
+      </ErrorBox>
+      <button
+        onClick={async () => {
+          try {
+            // Invalidate all SWR state, but don't re-fetch automatically
+            await mutate(() => true, undefined, { revalidate: false })
+          } catch (e) {
+            console.error('Failed to clear SWR cache', e)
+          }
+
+          //Attempt to recover by re-rendering
+          retry()
+        }}
+      >
+        Try again
+      </button>
+    </div>
+  )
+}

src/app/forbidden.tsx

@@ -0,0 +1,12 @@
+import ErrorBox from './components/ErrorBox'
+
+export default function Forbidden() {
+  return (
+    <div>
+      <h1>Not allowed</h1>
+      <ErrorBox>
+        <p>You don&apos;t have access to this page.</p>
+      </ErrorBox>
+    </div>
+  )
+}

src/app/global-error.tsx

@@ -0,0 +1,21 @@
+'use client'
+
+interface ErrorBoundaryProps {
+  error: Error & { digest?: string }
+}
+
+/**
+ * Error boundary for the app root
+ * https://nextjs.org/docs/app/api-reference/file-conventions/error#global-error
+ */
+export default function GlobalError({ error }: ErrorBoundaryProps) {
+  return (
+    <html>
+      <body>
+        <h1>Something went wrong!</h1>
+        <p>Contact your administrator if you continue to see this message.</p>
+        {error.digest && <p>(Error code {error.digest})</p>}
+      </body>
+    </html>
+  )
+}