Adds server error messages to form actions bar

app/components/form/Form.tsx

@@ -1,4 +1,5 @@
 import type { ButtonProps } from '@oxide/ui'
+import { Error12Icon } from '@oxide/ui'
 import { Button } from '@oxide/ui'
 import { SideModal } from '@oxide/ui'
 import { useIsInSideModal } from '@oxide/ui'
@@ -18,7 +19,7 @@ import React, { cloneElement } from 'react'
 import invariant from 'tiny-invariant'
 import './form.css'
 import cn from 'classnames'
-import type { ErrorResponse } from '@oxide/api'
+import type { Error, ErrorResponse } from '@oxide/api'
 
 const PageActionsTunnel = tunnel('form-page-actions')
 const SideModalActionsTunnel = tunnel('form-sidebar-actions')
@@ -90,6 +91,7 @@ export function Form<Values>({
                       formId: id,
                       submitDisabled:
                         !props.dirty || !props.isValid || mutation.status === 'loading',
+                      error: mutation.error?.error,
                       onDismiss,
                     })}
                   </SideModalActionsTunnel.In>
@@ -100,6 +102,7 @@ export function Form<Values>({
                         formId: id,
                         submitDisabled:
                           !props.dirty || !props.isValid || mutation.status === 'loading',
+                        error: mutation.error?.error,
                       })}
                     </PageActionsContainer>
                   </PageActionsTunnel.In>
@@ -122,6 +125,7 @@ interface FormActionsProps {
   children: React.ReactNode
   submitDisabled?: boolean
   onDismiss?: () => void
+  error?: Error | null
 }
 
 /**
@@ -135,6 +139,7 @@ Form.Actions = ({
   formId,
   submitDisabled = true,
   onDismiss,
+  error,
 }: FormActionsProps) => {
   // eslint-disable-next-line react-hooks/rules-of-hooks
   const isSideModal = useIsInSideModal()
@@ -157,10 +162,20 @@ Form.Actions = ({
   invariant(submit, 'Form.Actions must contain a Form.Submit component')
 
   return (
-    <div className={cn('flex gap-[0.625rem]', { 'flex-row-reverse': isSideModal })}>
+    <div
+      className={cn('flex w-full items-center gap-[0.625rem] children:shrink-0', {
+        'flex-row-reverse': isSideModal,
+      })}
+    >
       {cloneElement(submit, { form: formId, disabled: submitDisabled })}
       {isSideModal && cancel && cloneElement(cancel, { onClick: onDismiss })}
       {childArray}
+      {error && (
+        <div className="flex !shrink grow items-start justify-end text-mono-sm text-error">
+          <Error12Icon className="mx-2 mt-0.5 shrink-0" />
+          <span>{error.message}</span>
+        </div>
+      )}
     </div>
   )
 }

app/pages/project/networking/VpcPage/modals/firewall-rules.tsx

@@ -24,7 +24,6 @@ import {
   useApiQueryClient,
   firewallRuleGetToPut,
 } from '@oxide/api'
-import { getServerError } from 'app/util/errors'
 
 type FormProps = {
   error: ErrorResponse | null
@@ -352,7 +351,7 @@ const CommonForm = ({ id, error }: FormProps) => {
         </fieldset>
       </SideModal.Section>
       <SideModal.Section>
-        <div className="text-destructive">{getServerError(error)}</div>
+        <div className="text-destructive">{error?.error.message}</div>
       </SideModal.Section>
     </Form>
   )

app/pages/project/networking/VpcPage/modals/vpc-routers.tsx

@@ -4,7 +4,6 @@ import { Formik, Form } from 'formik'
 import { Button, FieldLabel, SideModal_old as SideModal, TextField } from '@oxide/ui'
 import type { VpcRouter, ErrorResponse } from '@oxide/api'
 import { useApiMutation, useApiQueryClient } from '@oxide/api'
-import { getServerError } from 'app/util/errors'
 
 // this will get a lot more interesting once the API is updated to allow us to
 // put rules in the router, which is the whole point of a router
@@ -41,7 +40,7 @@ const CommonForm = ({ error, id }: FormProps) => (
       </div>
     </SideModal.Section>
     <SideModal.Section>
-      <div className="text-destructive">{getServerError(error)}</div>
+      <div className="text-destructive">{error?.error.message}</div>
     </SideModal.Section>
   </Form>
 )

libs/api/__tests__/hooks.spec.ts

@@ -47,7 +47,7 @@ describe('useApiQuery', () => {
       const { result } = renderGetOrg503()
 
       await waitFor(() =>
-        expect(result.current.error?.error).toEqual({
+        expect(result.current.error?.error).toMatchObject({
           errorCode: 'ServiceUnavailable',
         })
       )
@@ -82,7 +82,7 @@ describe('useApiQuery', () => {
       await waitFor(() => {
         const error = result.error as ErrorResponse | undefined
         expect(error?.status).toEqual(404)
-        expect(error?.error).toEqual({ errorCode: 'ObjectNotFound' })
+        expect(error?.error).toMatchObject({ errorCode: 'ObjectNotFound' })
       })
     })
 
@@ -98,7 +98,7 @@ describe('useApiQuery', () => {
       )
 
       await waitFor(() =>
-        expect(result.current.error?.error).toEqual({
+        expect(result.current.error?.error).toMatchObject({
           errorCode: 'ObjectNotFound',
         })
       )
@@ -155,7 +155,7 @@ describe('useApiMutation', () => {
       act(() => result.current.mutate(projectPost404Params))
 
       await waitFor(() =>
-        expect(result.current.error?.error).toEqual({
+        expect(result.current.error?.error).toMatchObject({
           errorCode: 'ObjectNotFound',
         })
       )

libs/api/errors.ts

@@ -0,0 +1,114 @@
+import { navToLogin } from './nav-to-login'
+import type { ApiMethods, ErrorResponse, Error } from '.'
+import { camelCaseToWords, capitalize } from '@oxide/util'
+
+const errorCodeFormatter =
+  (method: keyof ApiMethods) =>
+  (errorCode: string, _: Error): string | undefined => {
+    switch (errorCode) {
+      case 'Forbidden':
+        return 'Action not authorized'
+
+      // TODO: This is a temporary fix for the API; better messages should be provided from there
+      case 'ObjectAlreadyExists':
+        if (method.endsWith('Post')) {
+          let resource = camelCaseToWords(method).slice(-2)[0]
+          resource = resource.endsWith('s') ? resource.slice(0, -1) : resource
+          return `${resource} name already exists`
+        }
+        return undefined
+      default:
+        return undefined
+    }
+  }
+
+export const handleErrors = (method: keyof ApiMethods) => (resp: ErrorResponse) => {
+  // TODO is this a valid failure condition?
+  if (!resp) throw 'unknown server error'
+
+  // if logged out, hit /login to trigger login redirect
+  if (resp.status === 401) {
+    // TODO-usability: for background requests, a redirect to login without
+    // warning could come as a surprise to the user, especially because
+    // sometimes background requests are not directly triggered by a user
+    // action, e.g., polling or refetching when window regains focus
+    navToLogin({ includeCurrent: true })
+  }
+  // we need to rethrow because that's how react-query knows it's an error
+  throw formatServerError(resp, errorCodeFormatter(method))
+}
+
+function formatServerError(
+  resp: ErrorResponse,
+  msgFromCode: (errorCode: string, error: Error) => string | undefined
+): ErrorResponse {
+  const code = resp.error.errorCode
+  const codeMsg = code && msgFromCode(code, resp.error)
+  const serverMsg = resp.error.message
+
+  resp.error.message =
+    codeMsg || getParseError(serverMsg) || serverMsg || 'Unknown server error'
+
+  return resp
+}
+
+function getParseError(message: string | undefined): string | undefined {
+  if (!message) return undefined
+  const inner = /^unable to parse body: (.+) at line \d+ column \d+$/.exec(message)?.[1]
+  return inner && capitalize(inner)
+}
+
+// -- TESTS ----------------
+
+if (import.meta.vitest) {
+  const { describe, it, expect } = import.meta.vitest
+  const parseError = {
+    error: {
+      requestId: '1',
+      errorCode: null,
+      message: 'unable to parse body: hello there, you have an error at line 129 column 4',
+    },
+  } as ErrorResponse
+
+  const alreadyExists = {
+    error: {
+      requestId: '2',
+      errorCode: 'ObjectAlreadyExists',
+      message: 'whatever',
+    },
+  } as ErrorResponse
+
+  describe('getParseError', () => {
+    it('extracts nice part of error message', () => {
+      expect(getParseError(parseError.error.message)).toEqual(
+        'Hello there, you have an error'
+      )
+    })
+
+    it('returns undefined if error does not match pattern', () => {
+      expect(getParseError('some nonsense')).toBeUndefined()
+    })
+  })
+
+  describe('getServerError', () => {
+    it('extracts message from parse errors', () => {
+      expect(formatServerError(parseError, () => undefined).error.message).toEqual(
+        'Hello there, you have an error'
+      )
+    })
+
+    it('uses message from code map if error code matches', () => {
+      expect(
+        formatServerError(alreadyExists, (code) =>
+          code === 'ObjectAlreadyExists' ? 'that already exists' : undefined
+        )
+      ).toEqual('that already exists')
+    })
+
+    it('falls back to server error message if code not found', () => {
+      expect(formatServerError(alreadyExists, () => undefined).error.message).toEqual(
+        'whatever'
+      )
+    })
+  })
+}