mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-05-13 16:07:30 +00:00
🛡️ fix: Handle MCP Tool Cache Lookup Failures (#12910)
* Handle MCP tool cache lookup failures * Harden MCP cached tool lookup * Cover full MCP tool cache outage * Guard MCP tool cache store lookup
This commit is contained in:
parent
74307e6dcc
commit
5b5e2b0286
4 changed files with 150 additions and 12 deletions
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -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<LCAvailableTools|null>} 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 = {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue