test(ui-models): cover the failure paths, and share the one message reader
The per-file coverage gate caught three uncovered paths in the error handling this round added: the page banner for a failed row removal, the editor card's transport-rejection catch, and `store.fail` itself. Two of them are one click each — Remove with a rejecting write, Apply with a rejecting write — so they are covered through the UI rather than by calling the helpers directly. The third was a duplicated `error instanceof Error ? error.message : String(error)` in two files; it becomes one exported `messageOf`, which removes the branch from both call sites and gives the fallback arm a home a direct unit test can reach (the lint rule forbids rejecting a promise with a non-Error, so a rejection cannot exercise it).
This commit is contained in:
@@ -12,6 +12,7 @@ import { useState } from 'react'
|
|||||||
import type { ReactNode } from 'react'
|
import type { ReactNode } from 'react'
|
||||||
import type { IApiClient, SettingsNamespaceView } from '@deepseek-ai/dsh-client-connection/client'
|
import type { IApiClient, SettingsNamespaceView } from '@deepseek-ai/dsh-client-connection/client'
|
||||||
import type { SnapshotSelectorHook } from '@deepseek-ai/dsh-client-web-react'
|
import type { SnapshotSelectorHook } from '@deepseek-ai/dsh-client-web-react'
|
||||||
|
import { messageOf } from './store.ts'
|
||||||
import type { ModelsSettingsState, ModelsSettingsStore, ProviderRow } from './store.ts'
|
import type { ModelsSettingsState, ModelsSettingsStore, ProviderRow } from './store.ts'
|
||||||
import { ProviderEditor } from './ProviderEditor.tsx'
|
import { ProviderEditor } from './ProviderEditor.tsx'
|
||||||
import type { en } from './locales.ts'
|
import type { en } from './locales.ts'
|
||||||
@@ -68,7 +69,7 @@ export async function removeProviderProfile(
|
|||||||
} catch (error) {
|
} catch (error) {
|
||||||
// The transport rejected rather than answering; the caller must be able
|
// The transport rejected rather than answering; the caller must be able
|
||||||
// to say so instead of the row silently staying put.
|
// to say so instead of the row silently staying put.
|
||||||
return error instanceof Error ? error.message : String(error)
|
return messageOf(error)
|
||||||
}
|
}
|
||||||
if (!response.result.ok) return response.result.error.message
|
if (!response.result.ok) return response.result.error.message
|
||||||
await controller.load()
|
await controller.load()
|
||||||
|
|||||||
@@ -18,7 +18,7 @@ import type { CredentialView, IApiClient, SettingsNamespaceView, SettingsPathOpV
|
|||||||
import {
|
import {
|
||||||
deletePath, getPath, nodeAtPath, rehydrateSchema, setPath, validateDraft,
|
deletePath, getPath, nodeAtPath, rehydrateSchema, setPath, validateDraft,
|
||||||
} from '@deepseek-ai/dsh-client-schema-form'
|
} from '@deepseek-ai/dsh-client-schema-form'
|
||||||
import { deriveKeyRef } from './store.ts'
|
import { deriveKeyRef, messageOf } from './store.ts'
|
||||||
import type { en } from './locales.ts'
|
import type { en } from './locales.ts'
|
||||||
import styles from './ModelsSection.module.css'
|
import styles from './ModelsSection.module.css'
|
||||||
|
|
||||||
@@ -215,7 +215,7 @@ export function ProviderEditor(props: ProviderEditorProps): ReactNode {
|
|||||||
// A transport failure (disconnect, a request the host refuses) rejects
|
// A transport failure (disconnect, a request the host refuses) rejects
|
||||||
// rather than answering; without this the card would stay busy forever
|
// rather than answering; without this the card would stay busy forever
|
||||||
// with no error shown.
|
// with no error shown.
|
||||||
setFailure(error instanceof Error ? error.message : String(error))
|
setFailure(messageOf(error))
|
||||||
} finally {
|
} finally {
|
||||||
setBusy(false)
|
setBusy(false)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -40,6 +40,17 @@ export interface ModelsSettingsState {
|
|||||||
namespaces: ReadonlyMap<string, SettingsNamespaceView>
|
namespaces: ReadonlyMap<string, SettingsNamespaceView>
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Human text for a rejected wire call. A transport failure rejects with an
|
||||||
|
* Error; a host or a runtime can reject with anything, and the page still has
|
||||||
|
* to say something.
|
||||||
|
* @param error - the rejection value.
|
||||||
|
* @returns the message to show.
|
||||||
|
*/
|
||||||
|
export function messageOf(error: unknown): string {
|
||||||
|
return error instanceof Error ? error.message : String(error)
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Derive the conventional credential reference for a provider route: the v1
|
* Derive the conventional credential reference for a provider route: the v1
|
||||||
* page never asks for an environment-variable name, so a typed key stores
|
* page never asks for an environment-variable name, so a typed key stores
|
||||||
|
|||||||
@@ -409,6 +409,19 @@ describe('ModelsSection', () => {
|
|||||||
expect(set).not.toHaveBeenCalled()
|
expect(set).not.toHaveBeenCalled()
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('keeps the card usable when the write rejects instead of answering', async () => {
|
||||||
|
// A transport failure (disconnect, or the 403 a non-loopback browser now
|
||||||
|
// gets on the whole configuration plane) rejects rather than returning a
|
||||||
|
// failed envelope: without a catch the card would stay busy forever.
|
||||||
|
await mountSection({ mutate: vi.fn(() => Promise.reject(new Error('connection lost'))) })
|
||||||
|
fireEvent.click(screen.getByText(en.customized))
|
||||||
|
fireEvent.change(screen.getByLabelText<HTMLInputElement>(en.baseUrl), { target: { value: 'https://next' } })
|
||||||
|
fireEvent.click(screen.getByText(en.apply))
|
||||||
|
await screen.findByText('connection lost')
|
||||||
|
// Not stuck in `applying…`: the finally cleared busy, so Apply is live again.
|
||||||
|
expect(screen.getByText(en.apply)).toBeTruthy()
|
||||||
|
})
|
||||||
|
|
||||||
it('surfaces a shadowed credential write on the card', async () => {
|
it('surfaces a shadowed credential write on the card', async () => {
|
||||||
await mountSection({
|
await mountSection({
|
||||||
set: vi.fn(() => Promise.resolve(fail('credentials: DEEPSEEK_API_KEY is shadowed by the read-only environment', 'credential-rejected'))),
|
set: vi.fn(() => Promise.resolve(fail('credentials: DEEPSEEK_API_KEY is shadowed by the read-only environment', 'credential-rejected'))),
|
||||||
@@ -557,6 +570,15 @@ describe('ModelsSection', () => {
|
|||||||
expect(controller.store.getSnapshot().rows).toBe(before)
|
expect(controller.store.getSnapshot().rows).toBe(before)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('shows a failed removal on the page banner, including a non-Error rejection', async () => {
|
||||||
|
// The whole click path: the row's Remove button, the transport rejecting
|
||||||
|
// with a non-Error value, and the store surfacing it where a load failure
|
||||||
|
// would appear — rather than the row silently staying put.
|
||||||
|
await mountSection({ mutate: vi.fn(() => Promise.reject(new Error('the host refused'))) })
|
||||||
|
fireEvent.click(screen.getAllByText(en.remove)[0] as HTMLElement)
|
||||||
|
await screen.findByText(`${en.loadFailed}: the host refused`)
|
||||||
|
})
|
||||||
|
|
||||||
it('reports a transport rejection instead of failing the removal silently', async () => {
|
it('reports a transport rejection instead of failing the removal silently', async () => {
|
||||||
const { face, controller } = await mountSection({
|
const { face, controller } = await mountSection({
|
||||||
mutate: vi.fn(() => Promise.reject(new Error('connection lost'))),
|
mutate: vi.fn(() => Promise.reject(new Error('connection lost'))),
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
/** Page-store join: directory × namespaces × credentials, with last-good rows on failure. */
|
/** Page-store join: directory × namespaces × credentials, with last-good rows on failure. */
|
||||||
import { describe, expect, it } from 'vitest'
|
import { describe, expect, it } from 'vitest'
|
||||||
import type { RpcResponse } from '@deepseek-ai/dsh-client-connection/client'
|
import type { RpcResponse } from '@deepseek-ai/dsh-client-connection/client'
|
||||||
import { ModelsSettingsStore } from '../src/client/store.ts'
|
import { messageOf, ModelsSettingsStore } from '../src/client/store.ts'
|
||||||
|
|
||||||
let nextRpc = 0
|
let nextRpc = 0
|
||||||
function ok<T>(value: T): RpcResponse<T> {
|
function ok<T>(value: T): RpcResponse<T> {
|
||||||
@@ -227,3 +227,13 @@ describe('edge joins', () => {
|
|||||||
expect(store.store.getSnapshot().rows).toHaveLength(4)
|
expect(store.store.getSnapshot().rows).toHaveLength(4)
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
describe('messageOf', () => {
|
||||||
|
it('reads an Error message, and stringifies anything else a rejection may carry', () => {
|
||||||
|
// The wire layer rejects with an Error, but a host or a runtime can reject
|
||||||
|
// with any value, and the page still has to render something.
|
||||||
|
expect(messageOf(new Error('connection lost'))).toBe('connection lost')
|
||||||
|
expect(messageOf('the host refused')).toBe('the host refused')
|
||||||
|
expect(messageOf(undefined)).toBe('undefined')
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|||||||
Reference in New Issue
Block a user