Display API message on not found errors (#1935)

* display API message on not found errors

* add unit test, don't fall back because API error always has message

Author: david-crespo

app/test/e2e/instance-create.e2e.ts

@@ -217,3 +217,11 @@ test('add ssh key from instance create form', async ({ page }) => {
   await page.getByRole('link', { name: 'SSH Keys' }).click()
   await expectRowVisible(page.getByRole('table'), { Name: newKey, Description: 'hi' })
 })
+
+test('shows object not found error on no default pool', async ({ page }) => {
+  await page.goto('/projects/mock-project/instances-new')
+  await page.getByRole('textbox', { name: 'Name', exact: true }).fill('no-default-pool')
+  await page.getByRole('button', { name: 'Create instance' }).click()
+
+  await expect(page.getByText('Not found: default IP pool')).toBeVisible()
+})

libs/api-mocks/msw/db.ts

@@ -19,8 +19,10 @@ import { internalError, json } from './util'
 
 const notFoundBody = { error_code: 'ObjectNotFound' } as const
 export type NotFound = typeof notFoundBody
-export const notFoundErr = () =>
-  json({ error_code: 'ObjectNotFound' } as const, { status: 404 })
+export const notFoundErr = (msg?: string) => {
+  const message = msg ? `not found: ${msg}` : 'not found'
+  return json({ error_code: 'ObjectNotFound', message } as const, { status: 404 })
+}
 
 export const lookupById = <T extends { id: string }>(table: T[], id: string) => {
   const item = table.find((i) => i.id === id)

libs/api-mocks/msw/handlers.ts

@@ -292,6 +292,10 @@ export const handlers = makeHandlers({
   instanceCreate({ body, query }) {
     const project = lookup.project(query)
 
+    if (body.name === 'no-default-pool') {
+      throw notFoundErr('default IP pool for current silo')
+    }
+
     errIfExists(db.instances, { name: body.name, project_id: project.id }, 'instance')
 
     const instanceId = uuid()

libs/api/__tests__/errors.spec.ts

@@ -83,6 +83,21 @@ describe('processServerError', () => {
     })
   })
 
+  describe('ObjectNotFound', () => {
+    it('passes through the API error', () => {
+      const error = makeError({
+        errorCode: 'ObjectNotFound',
+        message: 'not found: whatever',
+        statusCode: 404,
+      })
+      expect(processServerError('fakeThingCreate', error)).toEqual({
+        errorCode: 'ObjectNotFound',
+        message: 'Not found: whatever',
+        statusCode: 404,
+      })
+    })
+  })
+
   it('falls back to server error message if code not found', () => {
     const error = makeError({ errorCode: 'WeirdError', message: 'whatever' })
     expect(processServerError('womp', error)).toEqual({
@@ -100,6 +115,6 @@ it.each([
   ['instanceNetworkInterfaceCreate', '', 'interface'],
   ['instanceNetworkInterfaceCreate', 'already exists: something else', 'something else'],
   ['doesNotContainC-reate', '', null],
-])('getResourceName', (method, message, resource) => {
+])('getResourceName: %s', (method, message, resource) => {
   expect(getResourceName(method, message)).toEqual(resource)
 })

libs/api/__tests__/hooks.spec.tsx

@@ -130,7 +130,7 @@ describe('useApiQuery', () => {
         const error = onError.mock.lastCall?.[0]
         expect(error).toEqual({
           errorCode: 'ObjectNotFound',
-          message: 'Object not found',
+          message: 'Not found',
           statusCode: 404,
         })
       })
@@ -208,7 +208,7 @@ describe('useApiMutation', () => {
 
       expect(result.current.error).toEqual({
         errorCode: 'ObjectNotFound',
-        message: 'Object not found',
+        message: 'Not found',
         statusCode: 404,
       })
     })

libs/api/errors.ts

@@ -7,7 +7,7 @@
  */
 import { camelCaseToWords, capitalize } from '@oxide/util'
 
-import type { ErrorResult } from '.'
+import type { ErrorResult } from './__generated__/Api'
 
 /**
  * Processed API error ready for display in the console. Note that it's possible
@@ -56,7 +56,7 @@ export function processServerError(method: string, resp: ErrorResult): ApiError
   if (code === 'Forbidden') {
     message = 'Action not authorized'
   } else if (code === 'ObjectNotFound') {
-    message = 'Object not found'
+    message = capitalize(resp.data.message)
   } else if (code === 'ObjectAlreadyExists') {
     const resource = getResourceName(method, resp.data.message)
     if (resource) {