diff --git a/api/server/controllers/mcp.js b/api/server/controllers/mcp.js index e31bb93bc6..8d20cbc82c 100644 --- a/api/server/controllers/mcp.js +++ b/api/server/controllers/mcp.js @@ -78,12 +78,20 @@ const getMCPTools = async (req, res) => { const mcpManager = getMCPManager(); const mcpServers = {}; - const cachePromises = configuredServers.map((serverName) => - getMCPServerTools(userId, serverName).then((tools) => ({ serverName, tools })), - ); - const cacheResults = await Promise.all(cachePromises); - const serverToolsMap = new Map(); + const cacheResults = await Promise.all( + configuredServers.map(async (serverName) => { + try { + return { + serverName, + tools: await getMCPServerTools(userId, serverName), + }; + } catch (error) { + logger.error(`[getMCPTools] Error fetching cached tools for ${serverName}:`, error); + return { serverName, tools: null }; + } + }), + ); for (const { serverName, tools } of cacheResults) { if (tools) { serverToolsMap.set(serverName, tools); diff --git a/api/server/routes/__tests__/mcp.spec.js b/api/server/routes/__tests__/mcp.spec.js index 7fabcc910a..89f4237b19 100644 --- a/api/server/routes/__tests__/mcp.spec.js +++ b/api/server/routes/__tests__/mcp.spec.js @@ -1947,6 +1947,106 @@ describe('MCP Routes', () => { }); }); + describe('GET /tools', () => { + it('should continue returning MCP tools when one server cache lookup fails', async () => { + const { Constants } = require('librechat-data-provider'); + const { logger } = require('@librechat/data-schemas'); + const { getMCPServerTools } = require('~/server/services/Config'); + + mockResolveAllMcpConfigs.mockResolvedValueOnce({ + 'bad-server': { + type: 'sse', + url: 'https://bad.example.com/sse', + }, + 'good-server': { + type: 'sse', + url: 'https://good.example.com/sse', + iconPath: '/icons/good.svg', + }, + }); + + // Mock order matches Object.keys() order from the config above. + getMCPServerTools + .mockRejectedValueOnce(new Error('cache unavailable')) + .mockResolvedValueOnce({ + [`search${Constants.mcp_delimiter}good-server`]: { + type: 'function', + function: { + name: `search${Constants.mcp_delimiter}good-server`, + description: 'Search good server', + parameters: { type: 'object' }, + }, + }, + }); + + const mockGetServerToolFunctions = jest.fn().mockResolvedValue(null); + require('~/config').getMCPManager.mockReturnValue({ + getServerToolFunctions: mockGetServerToolFunctions, + }); + + const response = await request(app).get('/api/mcp/tools'); + + expect(response.status).toBe(200); + expect(logger.error).toHaveBeenCalledWith( + '[getMCPTools] Error fetching cached tools for bad-server:', + expect.any(Error), + ); + expect(mockGetServerToolFunctions).toHaveBeenCalledWith('test-user-id', 'bad-server'); + expect(response.body.servers['good-server']).toMatchObject({ + name: 'good-server', + icon: '/icons/good.svg', + tools: [ + { + name: 'search', + pluginKey: `search${Constants.mcp_delimiter}good-server`, + description: 'Search good server', + }, + ], + }); + expect(response.body.servers['bad-server']).toMatchObject({ + name: 'bad-server', + tools: [], + }); + }); + + it('should return configured servers when all cache lookups fail', async () => { + const { logger } = require('@librechat/data-schemas'); + const { getMCPServerTools } = require('~/server/services/Config'); + + mockResolveAllMcpConfigs.mockResolvedValueOnce({ + 'first-server': { + type: 'sse', + url: 'https://first.example.com/sse', + }, + 'second-server': { + type: 'sse', + url: 'https://second.example.com/sse', + }, + }); + + getMCPServerTools.mockRejectedValue(new Error('cache unavailable')); + + const mockGetServerToolFunctions = jest.fn().mockResolvedValue(null); + require('~/config').getMCPManager.mockReturnValue({ + getServerToolFunctions: mockGetServerToolFunctions, + }); + + const response = await request(app).get('/api/mcp/tools'); + + expect(response.status).toBe(200); + expect(response.body.servers['first-server']).toMatchObject({ + name: 'first-server', + tools: [], + }); + expect(response.body.servers['second-server']).toMatchObject({ + name: 'second-server', + tools: [], + }); + expect(logger.error).toHaveBeenCalledTimes(2); + expect(mockGetServerToolFunctions).toHaveBeenCalledTimes(2); + }); + }); + describe('GET /servers', () => { // mockRegistryInstance is defined at the top of the file diff --git a/api/server/services/Config/__tests__/getCachedTools.spec.js b/api/server/services/Config/__tests__/getCachedTools.spec.js index 38d488ed38..71ae8b5a57 100644 --- a/api/server/services/Config/__tests__/getCachedTools.spec.js +++ b/api/server/services/Config/__tests__/getCachedTools.spec.js @@ -1,6 +1,12 @@ const { CacheKeys } = require('librechat-data-provider'); +jest.mock('@librechat/data-schemas', () => ({ + logger: { + error: jest.fn(), + }, +})); jest.mock('~/cache/getLogStores'); +const { logger } = require('@librechat/data-schemas'); const getLogStores = require('~/cache/getLogStores'); const mockCache = { get: jest.fn(), set: jest.fn(), delete: jest.fn() }; @@ -75,6 +81,30 @@ describe('getCachedTools', () => { expect(mockCache.get).toHaveBeenCalledWith(ToolCacheKeys.MCP_SERVER('user1', 'github')); }); + it('getMCPServerTools should return null when the cache lookup fails', async () => { + const error = new Error('cache unavailable'); + mockCache.get.mockRejectedValue(error); + + await expect(getMCPServerTools('user1', 'github')).resolves.toBeNull(); + expect(logger.error).toHaveBeenCalledWith( + '[getMCPServerTools] Error fetching cached tools for github:', + error, + ); + }); + + it('getMCPServerTools should return null when the cache store is unavailable', async () => { + const error = new Error('cache store unavailable'); + getLogStores.mockImplementationOnce(() => { + throw error; + }); + + await expect(getMCPServerTools('user1', 'github')).resolves.toBeNull(); + expect(logger.error).toHaveBeenCalledWith( + '[getMCPServerTools] Error fetching cached tools for github:', + error, + ); + }); + it('should NOT use CONFIG_STORE namespace', async () => { mockCache.get.mockResolvedValue(null); await getCachedTools(); diff --git a/api/server/services/Config/getCachedTools.js b/api/server/services/Config/getCachedTools.js index eb7a08305a..083cfae6ba 100644 --- a/api/server/services/Config/getCachedTools.js +++ b/api/server/services/Config/getCachedTools.js @@ -1,4 +1,5 @@ const { CacheKeys, Time } = require('librechat-data-provider'); +const { logger } = require('@librechat/data-schemas'); const getLogStores = require('~/cache/getLogStores'); /** @@ -89,14 +90,13 @@ async function invalidateCachedTools(options = {}) { * @returns {Promise} The available tools for the server */ async function getMCPServerTools(userId, serverName) { - const cache = getLogStores(CacheKeys.TOOL_CACHE); - const serverTools = await cache.get(ToolCacheKeys.MCP_SERVER(userId, serverName)); - - if (serverTools) { - return serverTools; + try { + const cache = getLogStores(CacheKeys.TOOL_CACHE); + return (await cache.get(ToolCacheKeys.MCP_SERVER(userId, serverName))) || null; + } catch (error) { + logger.error(`[getMCPServerTools] Error fetching cached tools for ${serverName}:`, error); + return null; } - - return null; } module.exports = {