fix: surface failed component list loads in expose components form - #106
Conversation
A failed /tools, /resources, or /prompts request rendered identically to a successful empty one: the section showed a zero count with no error. Read the useQuery error per section, show the count line as "Failed to load ..." with an error notification and a retry action, and keep real counts (including legitimate zeros) for sections whose request succeeded. Signed-off-by: Li Fengmin <2080291162@qq.com>
gcgoncalves
left a comment
There was a problem hiding this comment.
Thanks @0717lee for this PR, the error handling is a nice improvement to the form. There are a couple suggestions for the proposed implementation:
First, it'd be nice to improve i18n (see comment below). Also, when a user clicks Retry for the tools section, toolsLoading becomes true. Then, the entire form is replaced by a spinner, including the resources and prompts sections, that are still valid.
My suggestion is using the individual loading states to add guards to each section:
{toolsLoading ? (
<span className="text-sm text-muted-foreground">
{intl.formatMessage({ id: "common.loading" })}
</span>
) : toolsError ? (
intl.formatMessage({ id: "gateways.exposeComponents.error.tools" })
) : (
intl.formatMessage({ id: "gateways.card.toolCount" }, { count: toolCount })
)}- Add gateways.exposeComponents.error.{tools,resources,prompts}(WithDetail)
and exposeComponents.promptCount keys to en-US, pt-BR, and es-ES; reuse
the existing gateways.card.*Count keys for tools and resources.
- Replace the form-level spinner on refetch with per-section count-row
states (loading / error / count); the full-form spinner now only covers
the initial load before any section has resolved, so retrying one failed
section no longer unmounts the healthy ones.
Signed-off-by: Li Fengmin <2080291162@qq.com>
|
Thanks for the review @gcgoncalves — both points addressed in 79cdee2:
Added a regression test ("should keep healthy sections mounted while a failed section retries") that pins this: during a hanging tools retry, the other sections stay mounted and no form-level spinner appears. Full vitest (3324 passed), |
Signed-off-by: Li Fengmin <2080291162@qq.com>
|
Thanks for the catch — I've localized the Retry action label as well. Changes:
Verification:
Pushed as |
Closes #6549
Summary
In the expose step, the three component lists are loaded with
useQuerybut onlydata,isLoading, andrefetchare destructured —erroris never read. A failed/tools,/resources, or/promptsrequest therefore renders identically to a successful empty one: the section shows "0 tools" / "0 resources" / "0 prompt templates" and invites pressing "Expose components" on what looks like a server offering nothing.STATUS_TONE_CLASS) instead of a zero count, and the section renders an errorInlineNotificationwith the API error message and a Retry action that refetches just that list.Unit tests cover the failed/empty distinction, per-section independence (some sections fail while others keep their counts), and recovery through Retry.