Skip to content

Commit d35bc77

Browse files
authored
fix(mcp): don't blank the MCP tools page when tool discovery fails (#5816)
* fix(mcp): don't blank the MCP tools page when tool discovery fails A tool-discovery error (one slow/failing server, e.g. a stalled transport) replaced the entire server list with an error banner. Gate the full-list replacement on serversError (the list genuinely failing to load) only; when the servers loaded, always render the list — each row already surfaces its own discovery state via toolsStateByServer, with a non-blocking notice above the list. Restores graceful degradation so one bad server can't hide the others. * fix(mcp): surface partial discovery failures per-row and in the notice When one server succeeds and another fails, the aggregate toolsError is suppressed (data exists), so the failure was hidden. Now: the notice renders on ANY per-server discovery error (not just all-fail), and each failed row surfaces its live discovery error instead of reading as '0 tools' before its stored status catches up.
1 parent 94b2334 commit d35bc77

1 file changed

Lines changed: 34 additions & 5 deletions

File tree

  • apps/sim/app/workspace/[workspaceId]/settings/components/mcp

apps/sim/app/workspace/[workspaceId]/settings/components/mcp/mcp.tsx

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ interface ServerListItemProps {
7171
isConnecting: boolean
7272
isLoadingTools?: boolean
7373
isRefreshing?: boolean
74+
discoveryError?: string | null
7475
onRemove: () => void
7576
onViewDetails: () => void
7677
onAuthorize: () => void
@@ -84,6 +85,7 @@ function ServerListItem({
8485
isConnecting,
8586
isLoadingTools = false,
8687
isRefreshing = false,
88+
discoveryError = null,
8789
onRemove,
8890
onViewDetails,
8991
onAuthorize,
@@ -95,8 +97,17 @@ function ServerListItem({
9597
server.lastError,
9698
server.authType
9799
)
100+
// A live discovery failure whose stored status hasn't caught up yet would otherwise read as
101+
// "0 tools"; surface it directly so a failed row reads as failed, not empty.
102+
const showDiscoveryError =
103+
Boolean(discoveryError) &&
104+
tools.length === 0 &&
105+
server.connectionStatus !== 'error' &&
106+
server.connectionStatus !== 'disconnected'
98107
const hasConnectionIssue =
99-
server.connectionStatus === 'error' || server.connectionStatus === 'disconnected'
108+
server.connectionStatus === 'error' ||
109+
server.connectionStatus === 'disconnected' ||
110+
showDiscoveryError
100111

101112
return (
102113
<div className='flex items-center justify-between gap-3'>
@@ -117,7 +128,9 @@ function ServerListItem({
117128
? 'Refreshing...'
118129
: isLoadingTools && tools.length === 0
119130
? 'Loading...'
120-
: toolsLabel}
131+
: showDiscoveryError
132+
? discoveryError
133+
: toolsLabel}
121134
</p>
122135
</div>
123136
<div className='flex flex-shrink-0 items-center gap-1'>
@@ -386,7 +399,15 @@ export function MCP() {
386399
return issues
387400
}
388401

389-
const error = toolsError || serversError
402+
// Only a failure to load the server LIST replaces the list. A tool-discovery failure
403+
// (`toolsError`) must not blank the page — the servers still render, each row surfacing its
404+
// own discovery state via `toolsStateByServer`, with a non-blocking notice above the list.
405+
const listError = serversError
406+
// Any per-server discovery failure — even a partial one where other servers succeeded (which
407+
// suppresses the aggregate `toolsError`) — so the notice below still surfaces it.
408+
const hasDiscoveryError =
409+
Boolean(toolsError) ||
410+
Array.from(toolsStateByServer.values()).some((state) => state.error != null)
390411
const hasServers = servers && servers.length > 0
391412
const showNoResults = searchTerm.trim() && filteredServers.length === 0 && servers.length > 0
392413

@@ -646,10 +667,10 @@ export function MCP() {
646667
: []
647668
}
648669
>
649-
{error ? (
670+
{listError ? (
650671
<div className='flex h-full flex-col items-center justify-center gap-2'>
651672
<p className='text-[var(--text-error)] text-xs leading-tight'>
652-
{getErrorMessage(error, 'Failed to load MCP servers')}
673+
{getErrorMessage(listError, 'Failed to load MCP servers')}
653674
</p>
654675
</div>
655676
) : serversLoading ? null : !hasServers ? (
@@ -658,6 +679,11 @@ export function MCP() {
658679
</SettingsEmptyState>
659680
) : (
660681
<div className='flex flex-col gap-2'>
682+
{hasDiscoveryError && (
683+
<p className='text-[var(--text-error)] text-xs leading-tight'>
684+
{getErrorMessage(toolsError, 'Some tools could not be discovered')}
685+
</p>
686+
)}
661687
{filteredServers.map((server) => {
662688
if (!server?.id) return null
663689
const tools = toolsByServer[server.id] || []
@@ -679,6 +705,9 @@ export function MCP() {
679705
refreshServerMutation.isPending &&
680706
refreshServerMutation.variables?.serverId === server.id
681707
}
708+
discoveryError={
709+
serverToolsState?.error ? getErrorMessage(serverToolsState.error) : null
710+
}
682711
onRemove={() => handleRemoveServer(server.id)}
683712
onViewDetails={() => handleViewDetails(server.id)}
684713
onAuthorize={() => startOauthForServer(server.id)}

0 commit comments

Comments
 (0)