Enhance MCP settings export with error handling and null value removal (#465)

frontend/src/components/ServerCard.tsx

@@ -106,6 +106,10 @@ const ServerCard = ({ server, onRemove, onEdit, onToggle, onRefresh }: ServerCar
     e.stopPropagation();
     try {
       const result = await exportMCPSettings(server.name);
+      if (!result || !result.success || !result.data) {
+        showToast(result?.message || t('common.copyFailed') || 'Copy failed', 'error');
+        return;
+      }
       const configJson = JSON.stringify(result.data, null, 2);
 
       if (navigator.clipboard && window.isSecureContext) {

src/controllers/configController.ts

@@ -4,6 +4,7 @@ import { loadSettings, loadOriginalSettings } from '../config/index.js';
 import { getDataService } from '../services/services.js';
 import { DataService } from '../services/dataService.js';
 import { IUser } from '../types/index.js';
+import { getServerDao } from '../dao/DaoFactory.js';
 
 const dataService: DataService = getDataService();
 
@@ -73,17 +74,39 @@ export const getPublicConfig = (req: Request, res: Response): void => {
   }
 };
 
+/**
+ * Recursively remove null values from an object
+ */
+const removeNullValues = <T>(obj: T): T => {
+  if (obj === null || obj === undefined) {
+    return obj;
+  }
+  if (Array.isArray(obj)) {
+    return obj.map((item) => removeNullValues(item)) as T;
+  }
+  if (typeof obj === 'object') {
+    const result: Record<string, unknown> = {};
+    for (const [key, value] of Object.entries(obj)) {
+      if (value !== null) {
+        result[key] = removeNullValues(value);
+      }
+    }
+    return result as T;
+  }
+  return obj;
+};
+
 /**
  * Get MCP settings in JSON format for export/copy
  * Supports both full settings and individual server configuration
  */
-export const getMcpSettingsJson = (req: Request, res: Response): void => {
+export const getMcpSettingsJson = async (req: Request, res: Response): Promise<void> => {
   try {
     const { serverName } = req.query;
-    const settings = loadOriginalSettings();
     if (serverName && typeof serverName === 'string') {
-      // Return individual server configuration
-      const serverConfig = settings.mcpServers[serverName];
+      // Return individual server configuration using DAO
+      const serverDao = getServerDao();
+      const serverConfig = await serverDao.findById(serverName);
       if (!serverConfig) {
         res.status(404).json({
           success: false,
@@ -92,16 +115,21 @@ export const getMcpSettingsJson = (req: Request, res: Response): void => {
         return;
       }
 
+      // Remove the 'name' field from config as it's used as the key
+      const { name, ...configWithoutName } = serverConfig;
+      // Remove null values from the config
+      const cleanedConfig = removeNullValues(configWithoutName);
       res.json({
         success: true,
         data: {
           mcpServers: {
-            [serverName]: serverConfig,
+            [name]: cleanedConfig,
           },
         },
       });
     } else {
       // Return full settings
+      const settings = loadOriginalSettings();
       res.json({
         success: true,
         data: settings,

tests/controllers/configController.test.ts

@@ -1,33 +1,43 @@
-import { getMcpSettingsJson } from '../../src/controllers/configController.js'
-import * as config from '../../src/config/index.js'
-import { Request, Response } from 'express'
+import { getMcpSettingsJson } from '../../src/controllers/configController.js';
+import * as config from '../../src/config/index.js';
+import * as DaoFactory from '../../src/dao/DaoFactory.js';
+import { Request, Response } from 'express';
 
 // Mock the config module
-jest.mock('../../src/config/index.js')
+jest.mock('../../src/config/index.js');
+// Mock the DaoFactory module
+jest.mock('../../src/dao/DaoFactory.js');
 
 describe('ConfigController - getMcpSettingsJson', () => {
-  let mockRequest: Partial<Request>
-  let mockResponse: Partial<Response>
-  let mockJson: jest.Mock
-  let mockStatus: jest.Mock
+  let mockRequest: Partial<Request>;
+  let mockResponse: Partial<Response>;
+  let mockJson: jest.Mock;
+  let mockStatus: jest.Mock;
+  let mockServerDao: { findById: jest.Mock };
 
   beforeEach(() => {
-    mockJson = jest.fn()
-    mockStatus = jest.fn().mockReturnThis()
+    mockJson = jest.fn();
+    mockStatus = jest.fn().mockReturnThis();
     mockRequest = {
       query: {},
-    }
+    };
     mockResponse = {
       json: mockJson,
       status: mockStatus,
-    }
+    };
+    mockServerDao = {
+      findById: jest.fn(),
+    };
+
+    // Setup ServerDao mock
+    (DaoFactory.getServerDao as jest.Mock).mockReturnValue(mockServerDao);
 
     // Reset mocks
-    jest.clearAllMocks()
-  })
+    jest.clearAllMocks();
+  });
 
   describe('Full Settings Export', () => {
-    it('should handle settings without users array', () => {
+    it('should handle settings without users array', async () => {
       const mockSettings = {
         mcpServers: {
           'test-server': {
@@ -35,11 +45,11 @@ describe('ConfigController - getMcpSettingsJson', () => {
             args: ['--test'],
           },
         },
-      }
+      };
 
-      ;(config.loadOriginalSettings as jest.Mock).mockReturnValue(mockSettings)
+      (config.loadOriginalSettings as jest.Mock).mockReturnValue(mockSettings);
 
-      getMcpSettingsJson(mockRequest as Request, mockResponse as Response)
+      await getMcpSettingsJson(mockRequest as Request, mockResponse as Response);
 
       expect(mockJson).toHaveBeenCalledWith({
         success: true,
@@ -47,40 +57,27 @@ describe('ConfigController - getMcpSettingsJson', () => {
           mcpServers: mockSettings.mcpServers,
           users: undefined,
         },
-      })
-    })
-  })
+      });
+    });
+  });
 
   describe('Individual Server Export', () => {
-    it('should return individual server configuration when serverName is specified', () => {
-      const mockSettings = {
-        mcpServers: {
-          'test-server': {
-            command: 'test',
-            args: ['--test'],
-            env: {
-              TEST_VAR: 'test-value',
-            },
-          },
-          'another-server': {
-            command: 'another',
-            args: ['--another'],
-          },
+    it('should return individual server configuration when serverName is specified', async () => {
+      const serverConfig = {
+        name: 'test-server',
+        command: 'test',
+        args: ['--test'],
+        env: {
+          TEST_VAR: 'test-value',
         },
-        users: [
-          {
-            username: 'admin',
-            password: '$2b$10$hashedpassword',
-            isAdmin: true,
-          },
-        ],
-      }
+      };
 
-      mockRequest.query = { serverName: 'test-server' }
-      ;(config.loadOriginalSettings as jest.Mock).mockReturnValue(mockSettings)
+      mockRequest.query = { serverName: 'test-server' };
+      mockServerDao.findById.mockResolvedValue(serverConfig);
 
-      getMcpSettingsJson(mockRequest as Request, mockResponse as Response)
+      await getMcpSettingsJson(mockRequest as Request, mockResponse as Response);
 
+      expect(mockServerDao.findById).toHaveBeenCalledWith('test-server');
       expect(mockJson).toHaveBeenCalledWith({
         success: true,
         data: {
@@ -94,46 +91,73 @@ describe('ConfigController - getMcpSettingsJson', () => {
             },
           },
         },
-      })
-    })
+      });
+    });
 
-    it('should return 404 when server does not exist', () => {
-      const mockSettings = {
-        mcpServers: {
-          'test-server': {
-            command: 'test',
-            args: ['--test'],
-          },
-        },
-      }
+    it('should return 404 when server does not exist', async () => {
+      mockRequest.query = { serverName: 'non-existent-server' };
+      mockServerDao.findById.mockResolvedValue(null);
 
-      mockRequest.query = { serverName: 'non-existent-server' }
-      ;(config.loadOriginalSettings as jest.Mock).mockReturnValue(mockSettings)
+      await getMcpSettingsJson(mockRequest as Request, mockResponse as Response);
 
-      getMcpSettingsJson(mockRequest as Request, mockResponse as Response)
-
-      expect(mockStatus).toHaveBeenCalledWith(404)
+      expect(mockServerDao.findById).toHaveBeenCalledWith('non-existent-server');
+      expect(mockStatus).toHaveBeenCalledWith(404);
       expect(mockJson).toHaveBeenCalledWith({
         success: false,
         message: "Server 'non-existent-server' not found",
-      })
-    })
-  })
+      });
+    });
+
+    it('should remove null values from server configuration', async () => {
+      const serverConfig = {
+        name: 'test-server',
+        command: 'test',
+        args: ['--test'],
+        url: null,
+        env: null,
+        headers: null,
+        options: {
+          timeout: 30,
+          retries: null,
+        },
+      };
+
+      mockRequest.query = { serverName: 'test-server' };
+      mockServerDao.findById.mockResolvedValue(serverConfig);
+
+      await getMcpSettingsJson(mockRequest as Request, mockResponse as Response);
+
+      expect(mockJson).toHaveBeenCalledWith({
+        success: true,
+        data: {
+          mcpServers: {
+            'test-server': {
+              command: 'test',
+              args: ['--test'],
+              options: {
+                timeout: 30,
+              },
+            },
+          },
+        },
+      });
+    });
+  });
 
   describe('Error Handling', () => {
-    it('should handle errors gracefully and return 500', () => {
-      const errorMessage = 'Failed to load settings'
-      ;(config.loadOriginalSettings as jest.Mock).mockImplementation(() => {
-        throw new Error(errorMessage)
-      })
+    it('should handle errors gracefully and return 500', async () => {
+      const errorMessage = 'Failed to load settings';
+      (config.loadOriginalSettings as jest.Mock).mockImplementation(() => {
+        throw new Error(errorMessage);
+      });
 
-      getMcpSettingsJson(mockRequest as Request, mockResponse as Response)
+      await getMcpSettingsJson(mockRequest as Request, mockResponse as Response);
 
-      expect(mockStatus).toHaveBeenCalledWith(500)
+      expect(mockStatus).toHaveBeenCalledWith(500);
       expect(mockJson).toHaveBeenCalledWith({
         success: false,
         message: 'Failed to get MCP settings',
-      })
-    })
-  })
-})
+      });
+    });
+  });
+});