Skip to content

feat: Add resilience to provider API response handling #223

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
geier wants to merge 6 commits into
base: main
Choose a base branch
from
+641 −126
Open

feat: Add resilience to provider API response handling #223

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
95d4499
feat: add resilience to provider API response handling in frontend
geier Apr 10, 2026
4181c35
fix: narrow issue 220 changes to provider resilience
geier Apr 11, 2026
537a64a
fix: harden settings provider error handling and reconnect retries
geier Apr 11, 2026
ef5930c
fix: harden settings resilience for provider and config errors
geier Apr 11, 2026
9337314
fix: await provider refetch retries in auth flows
geier Apr 11, 2026
b8c2f35
fix: surface settings async context errors via ErrorBoundary
geier Apr 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 29 additions & 6 deletions app-prefixable/src/context/config.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ export function ConfigProvider(props: ParentProps) {
let refreshSeq = 0
let lastUpdateAt = 0

// Validate that a response has the expected Config shape (must be a non-array object)
function isValidConfig(data: unknown): data is Config {
return !!data && typeof data === "object" && !Array.isArray(data)
}

async function refresh() {
const seq = ++refreshSeq
setLoading(true)
Expand All @@ -45,7 +50,11 @@ export function ConfigProvider(props: ParentProps) {
try {
const projRes = await sdk.client.config.get()
if (seq !== refreshSeq) return // superseded by newer refresh
setProject(reconcile((projRes?.data as Config) ?? {}))
const projData = projRes?.data
if (projData && !isValidConfig(projData)) {
console.error("[Config] Unexpected project config response shape:", projData)
}
setProject(reconcile(isValidConfig(projData) ? projData : {}))
} catch (e) {
console.error("[Config] Failed to fetch project config:", e)
if (seq !== refreshSeq) return
Expand All @@ -58,7 +67,11 @@ export function ConfigProvider(props: ParentProps) {
try {
const globalRes = await sdk.client.global.config.get()
if (seq !== refreshSeq) return
setGlobal(reconcile((globalRes?.data as Config) ?? {}))
const globalData = globalRes?.data
if (globalData && !isValidConfig(globalData)) {
console.error("[Config] Unexpected global config response shape:", globalData)
}
setGlobal(reconcile(isValidConfig(globalData) ? globalData : {}))
} catch (e) {
console.error("[Config] Failed to fetch global config:", e)
if (seq !== refreshSeq) return
Expand All @@ -76,8 +89,13 @@ export function ConfigProvider(props: ParentProps) {
setError(null)
try {
const res = await sdk.client.config.update({ config: patch })
const data = res.data as Config | undefined
if (data) {
const data = res.data
if (data && !isValidConfig(data)) {
console.error("[Config] Unexpected project update response shape:", data)
setError("Failed to save project configuration")
return null
}
Comment on lines 91 to +97
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When config.updateProject / config.updateGlobal receive a non-null response that fails isValidConfig, they currently just console.error(...) and return null while leaving error() cleared. Call sites (e.g. Settings) only show UI feedback via config.error() / saveError(), so this becomes a silent save failure. Consider setting setError(...) (or throwing) on invalid response shapes so the user gets a visible error state.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@geier geier Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in ef5930c: invalid non-object project update responses now set a user-visible config error before returning null.

if (isValidConfig(data)) {
lastUpdateAt = Date.now()
setProject(reconcile(data))
return data
Expand All @@ -101,8 +119,13 @@ export function ConfigProvider(props: ParentProps) {
? { ...patch, disabled_providers: global.disabled_providers }
: patch
const res = await sdk.client.global.config.update({ config: safePatch })
const data = res.data as Config | undefined
if (data) {
const data = res.data
if (data && !isValidConfig(data)) {
console.error("[Config] Unexpected global update response shape:", data)
setError("Failed to save global configuration")
return null
}
Comment on lines 121 to +127
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above for the global config save: if sdk.client.global.config.update(...) returns a non-object payload, the function returns null without setting error(), so the UI has no indication that the save failed. Setting an error message (or throwing) on invalid response shapes would make this failure mode user-visible.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@geier geier Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in ef5930c: invalid non-object global update responses now set a user-visible config error before returning null.

if (isValidConfig(data)) {
lastUpdateAt = Date.now()
setGlobal(reconcile(data))
return data
Expand Down
137 changes: 115 additions & 22 deletions app-prefixable/src/context/providers.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ interface ProviderContextValue {
providers: Provider[]
connected: string[]
defaults: Record<string, string>
providerError: string | null
authMethods: Record<string, ProviderAuthMethod[]>
agents: Agent[]
loading: boolean
Expand All @@ -89,7 +90,7 @@ interface ProviderContextValue {
setSelectedAgent: (agent: string) => void
getSessionModel: (sessionID: string) => ModelKey | null
setSessionModel: (sessionID: string, model: ModelKey | null) => void
refetch: () => void
refetch: () => Promise<void>
connectProvider: (providerID: string, apiKey: string) => Promise<boolean>
startOAuth: (providerID: string, methodIndex: number) => Promise<OAuthAuthorization | undefined>
completeOAuth: (providerID: string, methodIndex: number, code?: string) => Promise<boolean>
Expand Down Expand Up @@ -159,23 +160,106 @@ export function ProviderProvider(props: ParentProps) {
}
})

function isRecord(value: unknown): value is Record<string, unknown> {
return !!value && typeof value === "object" && !Array.isArray(value)
}

function normalizeModel(providerID: string, modelID: string, model: unknown): Model {
if (!isRecord(model)) {
throw new Error(`[Providers] Provider "${providerID}" model "${modelID}" must be an object`)
}
const name = typeof model.name === "string" && model.name ? model.name : modelID
const rawLimit = model.limit
const limit = isRecord(rawLimit) ? rawLimit : {}
return {
...(model as Omit<Model, "id" | "name" | "limit" | "providerID">),
id: typeof model.id === "string" && model.id ? model.id : modelID,
name,
providerID,
limit: {
context: typeof limit.context === "number" ? limit.context : 0,
input: typeof limit.input === "number" ? limit.input : undefined,
output: typeof limit.output === "number" ? limit.output : 0,
},
}
}

function normalizeProviderListData(data: unknown): ProviderListData {
if (!isRecord(data)) {
throw new Error("[Providers] Provider list response must be an object")
}
if (!Array.isArray(data.all)) {
throw new Error("[Providers] Provider list field \"all\" must be an array")
}
if (!Array.isArray(data.connected)) {
throw new Error("[Providers] Provider list field \"connected\" must be an array")
}
if (data.default !== undefined && !isRecord(data.default)) {
throw new Error("[Providers] Provider list field \"default\" must be an object")
}

const all = data.all.map((entry, i) => {
if (!isRecord(entry)) {
throw new Error(`[Providers] Provider entry at index ${i} must be an object`)
}
if (typeof entry.id !== "string" || !entry.id) {
throw new Error(`[Providers] Provider entry at index ${i} is missing a string \"id\"`)
}
if (typeof entry.name !== "string" || !entry.name) {
throw new Error(`[Providers] Provider \"${entry.id}\" is missing a string \"name\"`)
}

const rawModels = entry.models
if (rawModels === undefined || rawModels === null) {
return { ...(entry as Provider), id: entry.id, name: entry.name, models: {} }
}
if (!isRecord(rawModels)) {
throw new Error(`[Providers] Provider \"${entry.id}\" field \"models\" must be an object`)
}
const models = Object.fromEntries(
Object.entries(rawModels).map(([modelID, model]) => [modelID, normalizeModel(entry.id, modelID, model)])
)
return { ...(entry as Provider), id: entry.id, name: entry.name, models }
})

const connected = data.connected.map((id, i) => {
if (typeof id !== "string" || !id) {
throw new Error(`[Providers] Connected provider at index ${i} must be a string`)
}
return id
})

const defaults = Object.fromEntries(
Object.entries(data.default ?? {}).map(([k, v]) => {
if (typeof v !== "string") {
throw new Error(`[Providers] Default model for provider \"${k}\" must be a string`)
}
return [k, v]
})
)

return { all, connected, default: defaults }
}

async function refetchProvidersWithRetry(providerID: string): Promise<boolean> {
const delays = [0, 150, 300, 600, 1200]
for (const delay of delays) {
if (delay > 0) await new Promise((resolve) => setTimeout(resolve, delay))
const data = await refetchProviders()
if (data?.connected.includes(providerID)) return true
}
return false
}

// Fetch providers
const [providerData, { refetch: refetchProviders }] = createResource(async () => {
try {
const res = await client.provider.list()
const data = res.data as ProviderListData | undefined
if (!data) return undefined
// Inject providerID into each model since the SDK response doesn't include it
const all = data.all.map((provider) => ({
...provider,
models: Object.fromEntries(
Object.entries(provider.models).map(([k, m]) => [k, { ...m, providerID: provider.id }])
),
}))
return { ...data, all }
return normalizeProviderListData(res.data)
} catch (e) {
console.error("Failed to fetch providers:", e)
return undefined
const msg = e instanceof Error ? e.message : "Failed to fetch providers"
throw new Error(msg)
}
})

Expand Down Expand Up @@ -236,7 +320,12 @@ export function ProviderProvider(props: ParentProps) {
const [authData] = createResource(async () => {
try {
const res = await client.provider.auth()
return (res.data as Record<string, ProviderAuthMethod[]>) ?? {}
const data = res.data
if (!data || typeof data !== "object" || Array.isArray(data)) {
console.error("[Providers] Invalid auth methods response shape:", data)
return {}
}
return data as Record<string, ProviderAuthMethod[]>
} catch (e) {
console.error("Failed to fetch auth methods:", e)
return {}
Expand Down Expand Up @@ -304,10 +393,10 @@ export function ProviderProvider(props: ParentProps) {
providerID,
auth: { type: "api", key: apiKey },
})
// Dispose instance to reload provider state, then refresh
// Dispose instance to reload provider state, then wait for the
// server to reinitialize and provider state to be observable.
await client.instance.dispose()
await refetchProviders()
return true
return await refetchProvidersWithRetry(providerID)
} catch (e) {
console.error("Failed to connect provider:", e)
return false
Expand All @@ -334,19 +423,18 @@ export function ProviderProvider(props: ParentProps) {
method: methodIndex,
code,
})
// Dispose instance to reload provider state, then refresh
// Dispose instance to reload provider state, then wait for the
// server to reinitialize and provider state to be observable.
await client.instance.dispose()
await refetchProviders()
return true
return await refetchProvidersWithRetry(providerID)
} catch (e) {
console.error("Failed to complete OAuth:", e)
return false
}
}

function refetch() {
refetchProviders()
refetchAgents()
async function refetch() {
await Promise.all([refetchProviders(), refetchAgents()])
}

const value: ProviderContextValue = {
Expand All @@ -361,6 +449,11 @@ export function ProviderProvider(props: ParentProps) {
get defaults() {
return providerData()?.default ?? {}
},
get providerError() {
const err = providerData.error
if (!err) return null
return err instanceof Error ? err.message : String(err)
},
get authMethods() {
return authData() ?? {}
},
Expand Down
Loading