fix: improve client connection handling and tool listing in mcpService (#30)

2025-12-24 02:39:19 -05:00 · 2025-04-23 15:46:06 +08:00
parent afd1ee7a50
commit c1eabb5607
7 changed files with 175 additions and 38 deletions
--- a/frontend/src/components/ServerCard.tsx
+++ b/frontend/src/components/ServerCard.tsx
@@ -1,10 +1,11 @@
-import { useState } from 'react'
+import { useState, useRef, useEffect } from 'react'
 import { useTranslation } from 'react-i18next'
 import { Server } from '@/types'
-import { ChevronDown, ChevronRight } from '@/components/icons/LucideIcons'
+import { ChevronDown, ChevronRight, AlertCircle, Copy, Check } from 'lucide-react'
 import Badge from '@/components/ui/Badge'
 import ToolCard from '@/components/ui/ToolCard'
 import DeleteDialog from '@/components/ui/DeleteDialog'
+import { useToast } from '@/contexts/ToastContext'

 interface ServerCardProps {
  server: Server
@@ -15,9 +16,26 @@ interface ServerCardProps {

 const ServerCard = ({ server, onRemove, onEdit, onToggle }: ServerCardProps) => {
  const { t } = useTranslation()
+  const { showToast } = useToast()
  const [isExpanded, setIsExpanded] = useState(false)
  const [showDeleteDialog, setShowDeleteDialog] = useState(false)
  const [isToggling, setIsToggling] = useState(false)
+  const [showErrorPopover, setShowErrorPopover] = useState(false)
+  const [copied, setCopied] = useState(false)
+  const errorPopoverRef = useRef<HTMLDivElement>(null)
+
+  useEffect(() => {
+    const handleClickOutside = (event: MouseEvent) => {
+      if (errorPopoverRef.current && !errorPopoverRef.current.contains(event.target as Node)) {
+        setShowErrorPopover(false)
+      }
+    }
+
+    document.addEventListener('mousedown', handleClickOutside)
+    return () => {
+      document.removeEventListener('mousedown', handleClickOutside)
+    }
+  }, [])

  const handleRemove = (e: React.MouseEvent) => {
    e.stopPropagation()
@@ -41,6 +59,44 @@ const ServerCard = ({ server, onRemove, onEdit, onToggle }: ServerCardProps) =>
    }
  }

+  const handleErrorIconClick = (e: React.MouseEvent) => {
+    e.stopPropagation()
+    setShowErrorPopover(!showErrorPopover)
+  }
+
+  const copyToClipboard = (e: React.MouseEvent) => {
+    e.stopPropagation()
+    if (!server.error) return
+
+    if (navigator.clipboard && window.isSecureContext) {
+      navigator.clipboard.writeText(server.error).then(() => {
+        setCopied(true)
+        showToast(t('common.copySuccess') || 'Copied to clipboard', 'success')
+        setTimeout(() => setCopied(false), 2000)
+      })
+    } else {
+      // Fallback for HTTP or unsupported clipboard API
+      const textArea = document.createElement('textarea')
+      textArea.value = server.error
+      // Avoid scrolling to bottom
+      textArea.style.position = 'fixed'
+      textArea.style.left = '-9999px'
+      document.body.appendChild(textArea)
+      textArea.focus()
+      textArea.select()
+      try {
+        document.execCommand('copy')
+        setCopied(true)
+        showToast(t('common.copySuccess') || 'Copied to clipboard', 'success')
+        setTimeout(() => setCopied(false), 2000)
+      } catch (err) {
+        showToast(t('common.copyFailed') || 'Copy failed', 'error')
+        console.error('Copy to clipboard failed:', err)
+      }
+      document.body.removeChild(textArea)
+    }
+  }
+
  const handleConfirmDelete = () => {
    onRemove(server.name)
    setShowDeleteDialog(false)
@@ -56,6 +112,59 @@ const ServerCard = ({ server, onRemove, onEdit, onToggle }: ServerCardProps) =>
          <div className="flex items-center space-x-3">
            <h2 className={`text-xl font-semibold ${server.enabled === false ? 'text-gray-600' : 'text-gray-900'}`}>{server.name}</h2>
            <Badge status={server.status} />
+            
+            {server.error && (
+              <div className="relative">
+                <div 
+                  className="cursor-pointer" 
+                  onClick={handleErrorIconClick}
+                  aria-label={t('server.viewErrorDetails')}
+                >
+                  <AlertCircle className="text-red-500 hover:text-red-600" size={18} />
+                </div>
+                
+                {showErrorPopover && (
+                  <div 
+                    ref={errorPopoverRef}
+                    className="absolute z-10 mt-2 bg-white border border-gray-200 rounded-md shadow-lg p-0 w-120"
+                    style={{ 
+                      left: '-231px', 
+                      top: '24px', 
+                      maxHeight: '300px', 
+                      overflowY: 'auto', 
+                      width: '480px',
+                      transform: 'translateX(50%)'
+                    }}
+                    onClick={(e) => e.stopPropagation()}
+                  >
+                    <div className="flex justify-between items-center sticky top-0 bg-white py-2 px-4 border-b border-gray-200 z-20 shadow-sm">
+                      <div className="flex items-center space-x-2">
+                        <h4 className="text-sm font-medium text-red-600">{t('server.errorDetails')}</h4>
+                        <button 
+                          onClick={copyToClipboard}
+                          className="p-1 text-gray-400 hover:text-gray-600 transition-colors"
+                          title={t('common.copy')}
+                        >
+                          {copied ? <Check size={14} className="text-green-500" /> : <Copy size={14} />}
+                        </button>
+                      </div>
+                      <button 
+                        onClick={(e) => {
+                          e.stopPropagation()
+                          setShowErrorPopover(false)
+                        }}
+                        className="text-gray-400 hover:text-gray-600"
+                      >
+                        ✕
+                      </button>
+                    </div>
+                    <div className="p-4 pt-2">
+                      <pre className="text-sm text-gray-700 break-words whitespace-pre-wrap">{server.error}</pre>
+                    </div>
+                  </div>
+                )}
+              </div>
+            )}
          </div>
          <div className="flex space-x-2">
            <button
--- a/frontend/src/locales/en.json
+++ b/frontend/src/locales/en.json
@@ -68,7 +68,9 @@
    "namePlaceholder": "Enter server name",
    "urlPlaceholder": "Enter server URL",
    "commandPlaceholder": "Enter command",
-    "argumentsPlaceholder": "Enter arguments"
+    "argumentsPlaceholder": "Enter arguments",
+    "errorDetails": "Error Details",
+    "viewErrorDetails": "View error details"
  },
  "status": {
    "online": "Online",
@@ -95,7 +97,9 @@
    "create": "Create",
    "submitting": "Submitting...",
    "delete": "Delete",
-    "copy": "Copy"
+    "copy": "Copy",
+    "copySuccess": "Copied to clipboard",
+    "copyFailed": "Copy failed"
  },
  "nav": {
    "dashboard": "Dashboard",
--- a/frontend/src/locales/zh.json
+++ b/frontend/src/locales/zh.json
@@ -68,7 +68,9 @@
    "namePlaceholder": "请输入服务器名称",
    "urlPlaceholder": "请输入服务器URL",
    "commandPlaceholder": "请输入命令",
-    "argumentsPlaceholder": "请输入参数"
+    "argumentsPlaceholder": "请输入参数",
+    "errorDetails": "错误详情",
+    "viewErrorDetails": "查看错误详情"
  },
  "status": {
    "online": "在线",
@@ -95,7 +97,9 @@
    "create": "创建",
    "submitting": "提交中...",
    "delete": "删除",
-    "copy": "复制"
+    "copy": "复制",
+    "copySuccess": "已复制到剪贴板",
+    "copyFailed": "复制失败"
  },
  "nav": {
    "dashboard": "仪表盘",
--- a/frontend/src/pages/ServersPage.tsx
+++ b/frontend/src/pages/ServersPage.tsx
@@ -31,6 +31,7 @@ const ServersPage: React.FC = () => {

  const handleEditComplete = () => {
    setEditingServer(null);
+    triggerRefresh();
  };

  const handleRefresh = async () => {
--- a/frontend/src/types/index.ts
+++ b/frontend/src/types/index.ts
@@ -82,6 +82,7 @@ export interface ServerConfig {
 export interface Server {
  name: string;
  status: ServerStatus;
+  error?: string;
  tools?: Tool[];
  config?: ServerConfig;
  enabled?: boolean;
--- a/src/services/mcpService.ts
+++ b/src/services/mcpService.ts
@@ -6,7 +6,6 @@ import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js'
 import { ServerInfo, ServerConfig } from '../types/index.js';
 import { loadSettings, saveSettings, expandEnvVars } from '../config/index.js';
 import config from '../config/index.js';
-import { get } from 'http';
 import { getGroup } from './sseService.js';
 import { getServersInGroup } from './groupService.js';

@@ -53,6 +52,7 @@ export const initializeClientsFromSettings = (): ServerInfo[] => {
      serverInfos.push({
        name,
        status: 'disconnected',
+        error: null,
        tools: [],
        createTime: Date.now(),
        enabled: false,
@@ -89,6 +89,7 @@ export const initializeClientsFromSettings = (): ServerInfo[] => {
      serverInfos.push({
        name,
        status: 'disconnected',
+        error: 'Missing required configuration',
        tools: [],
        createTime: Date.now(),
      });
@@ -108,16 +109,54 @@ export const initializeClientsFromSettings = (): ServerInfo[] => {
        },
      },
    );
-    client.connect(transport, { timeout: Number(config.timeout) }).catch((error) => {
-      console.error(`Failed to connect client for server ${name} by error: ${error}`);
-      const serverInfo = getServerByName(name);
-      if (serverInfo) {
-        serverInfo.status = 'disconnected';
-      }
-    });
+    client
+      .connect(transport, { timeout: Number(config.timeout) })
+      .then(() => {
+        console.log(`Successfully connected client for server: ${name}`);
+
+        client
+          .listTools({}, { timeout: Number(config.timeout) })
+          .then((tools) => {
+            console.log(`Successfully listed ${tools.tools.length} tools for server: ${name}`);
+            const serverInfo = getServerByName(name);
+            if (!serverInfo) {
+              console.warn(`Server info not found for server: ${name}`);
+              return;
+            }
+
+            serverInfo.tools = tools.tools.map((tool) => ({
+              name: tool.name,
+              description: tool.description || '',
+              inputSchema: tool.inputSchema || {},
+            }));
+            serverInfo.status = 'connected';
+            serverInfo.error = null;
+          })
+          .catch((error) => {
+            console.error(
+              `Failed to list tools for server ${name} by error: ${error} with stack: ${error.stack}`,
+            );
+            const serverInfo = getServerByName(name);
+            if (serverInfo) {
+              serverInfo.status = 'disconnected';
+              serverInfo.error = `Failed to list tools: ${error.stack} `;
+            }
+          });
+      })
+      .catch((error) => {
+        console.error(
+          `Failed to connect client for server ${name} by error: ${error} with stack: ${error.stack}`,
+        );
+        const serverInfo = getServerByName(name);
+        if (serverInfo) {
+          serverInfo.status = 'disconnected';
+          serverInfo.error = `Failed to connect: ${error.stack} `;
+        }
+      });
    serverInfos.push({
      name,
      status: 'connecting',
+      error: null,
      tools: [],
      client,
      transport,
@@ -132,40 +171,18 @@ export const initializeClientsFromSettings = (): ServerInfo[] => {
 // Register all MCP tools
 export const registerAllTools = async (server: Server, forceInit: boolean): Promise<void> => {
  initializeClientsFromSettings();
-  for (const serverInfo of serverInfos) {
-    if (serverInfo.status === 'connected' && !forceInit) continue;
-    if (!serverInfo.client || !serverInfo.transport) continue;
-
-    try {
-      serverInfo.status = 'connecting';
-      console.log(`Connecting to server: ${serverInfo.name}...`);
-      const tools = await serverInfo.client.listTools({}, { timeout: Number(config.timeout) });
-      serverInfo.tools = tools.tools.map((tool) => ({
-        name: tool.name,
-        description: tool.description || '',
-        inputSchema: tool.inputSchema || {},
-      }));
-
-      serverInfo.status = 'connected';
-      console.log(`Successfully connected to server: ${serverInfo.name}`);
-    } catch (error) {
-      console.error(
-        `Failed to connect to server for client: ${serverInfo.name} by error: ${error}`,
-      );
-      serverInfo.status = 'disconnected';
-    }
-  }
 };

 // Get all server information
 export const getServersInfo = (): Omit<ServerInfo, 'client' | 'transport'>[] => {
  const settings = loadSettings();
-  const infos = serverInfos.map(({ name, status, tools, createTime }) => {
+  const infos = serverInfos.map(({ name, status, tools, createTime, error }) => {
    const serverConfig = settings.mcpServers[name];
    const enabled = serverConfig ? serverConfig.enabled !== false : true;
    return {
      name,
      status,
+      error,
      tools,
      createTime,
      enabled,
--- a/src/types/index.ts
+++ b/src/types/index.ts
@@ -100,6 +100,7 @@ export interface ServerConfig {
 export interface ServerInfo {
  name: string; // Unique name of the server
  status: 'connected' | 'connecting' | 'disconnected'; // Current connection status
+  error: string | null; // Error message if any
  tools: ToolInfo[]; // List of tools available on the server
  client?: Client; // Client instance for communication
  transport?: SSEClientTransport | StdioClientTransport; // Transport mechanism used