Skip to content

Commit

Permalink
Display API message on not found errors (#1935)
Browse files Browse the repository at this point in the history
* display API message on not found errors

* add unit test, don't fall back because API error always has message
  • Loading branch information
david-crespo authored Feb 2, 2024
1 parent 2aa720d commit 16b5b32
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 7 deletions.
8 changes: 8 additions & 0 deletions app/test/e2e/instance-create.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
6 changes: 4 additions & 2 deletions libs/api-mocks/msw/db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions libs/api-mocks/msw/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
17 changes: 16 additions & 1 deletion libs/api/__tests__/errors.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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)
})
4 changes: 2 additions & 2 deletions libs/api/__tests__/hooks.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
})
Expand Down Expand Up @@ -208,7 +208,7 @@ describe('useApiMutation', () => {

expect(result.current.error).toEqual({
errorCode: 'ObjectNotFound',
message: 'Object not found',
message: 'Not found',
statusCode: 404,
})
})
Expand Down
4 changes: 2 additions & 2 deletions libs/api/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 16b5b32

Please sign in to comment.