mirror of
https://github.com/simstudioai/sim.git
synced 2026-04-06 03:00:16 -04:00
fix(mcp): remove client-side cache and reduce server cache from 5m to 30s (#2182)
* fix(mcp): remove client-side cache and reduce server cache from 5m to 30s * ack PR comments
This commit is contained in:
@@ -44,11 +44,14 @@ export class McpClient {
|
||||
|
||||
/**
|
||||
* Creates a new MCP client
|
||||
*
|
||||
* No session ID parameter (we disconnect after each operation).
|
||||
* The SDK handles session management automatically via Mcp-Session-Id header.
|
||||
*
|
||||
* @param config - Server configuration
|
||||
* @param securityPolicy - Optional security policy
|
||||
* @param sessionId - Optional session ID for session restoration (from previous connection)
|
||||
*/
|
||||
constructor(config: McpServerConfig, securityPolicy?: McpSecurityPolicy, sessionId?: string) {
|
||||
constructor(config: McpServerConfig, securityPolicy?: McpSecurityPolicy) {
|
||||
this.config = config
|
||||
this.connectionStatus = { connected: false }
|
||||
this.securityPolicy = securityPolicy ?? {
|
||||
@@ -65,7 +68,6 @@ export class McpClient {
|
||||
requestInit: {
|
||||
headers: this.config.headers,
|
||||
},
|
||||
sessionId,
|
||||
})
|
||||
|
||||
this.client = new Client(
|
||||
|
||||
@@ -42,42 +42,17 @@ interface CacheStats {
|
||||
|
||||
class McpService {
|
||||
private toolCache = new Map<string, ToolCache>()
|
||||
private readonly cacheTimeout = MCP_CONSTANTS.CACHE_TIMEOUT
|
||||
private readonly maxCacheSize = 1000
|
||||
private readonly cacheTimeout = MCP_CONSTANTS.CACHE_TIMEOUT // 30 seconds
|
||||
private readonly maxCacheSize = MCP_CONSTANTS.MAX_CACHE_SIZE // 1000
|
||||
private cleanupInterval: NodeJS.Timeout | null = null
|
||||
private cacheHits = 0
|
||||
private cacheMisses = 0
|
||||
private entriesEvicted = 0
|
||||
|
||||
private sessionCache = new Map<string, string>()
|
||||
|
||||
constructor() {
|
||||
this.startPeriodicCleanup()
|
||||
}
|
||||
|
||||
/**
|
||||
* Get cached session ID for a server
|
||||
*/
|
||||
private getCachedSessionId(serverId: string): string | undefined {
|
||||
return this.sessionCache.get(serverId)
|
||||
}
|
||||
|
||||
/**
|
||||
* Cache session ID for a server
|
||||
*/
|
||||
private cacheSessionId(serverId: string, sessionId: string): void {
|
||||
this.sessionCache.set(serverId, sessionId)
|
||||
logger.debug(`Cached session ID for server ${serverId}`)
|
||||
}
|
||||
|
||||
/**
|
||||
* Clear cached session ID for a server
|
||||
*/
|
||||
private clearCachedSessionId(serverId: string): void {
|
||||
this.sessionCache.delete(serverId)
|
||||
logger.debug(`Cleared cached session ID for server ${serverId}`)
|
||||
}
|
||||
|
||||
/**
|
||||
* Start periodic cleanup of expired cache entries
|
||||
*/
|
||||
@@ -341,49 +316,9 @@ class McpService {
|
||||
allowedOrigins: config.url ? [new URL(config.url).origin] : undefined,
|
||||
}
|
||||
|
||||
const cachedSessionId = this.getCachedSessionId(config.id)
|
||||
|
||||
const client = new McpClient(config, securityPolicy, cachedSessionId)
|
||||
|
||||
try {
|
||||
await client.connect()
|
||||
|
||||
const newSessionId = client.getSessionId()
|
||||
if (newSessionId) {
|
||||
this.cacheSessionId(config.id, newSessionId)
|
||||
}
|
||||
|
||||
return client
|
||||
} catch (error) {
|
||||
if (cachedSessionId && this.isSessionError(error)) {
|
||||
logger.debug(`Session restoration failed for server ${config.id}, retrying fresh`)
|
||||
this.clearCachedSessionId(config.id)
|
||||
|
||||
const freshClient = new McpClient(config, securityPolicy)
|
||||
await freshClient.connect()
|
||||
|
||||
const freshSessionId = freshClient.getSessionId()
|
||||
if (freshSessionId) {
|
||||
this.cacheSessionId(config.id, freshSessionId)
|
||||
}
|
||||
|
||||
return freshClient
|
||||
}
|
||||
|
||||
throw error
|
||||
}
|
||||
}
|
||||
|
||||
private isSessionError(error: unknown): boolean {
|
||||
if (error instanceof Error) {
|
||||
const message = error.message.toLowerCase()
|
||||
return (
|
||||
message.includes('no valid session') ||
|
||||
message.includes('invalid session') ||
|
||||
message.includes('session expired')
|
||||
)
|
||||
}
|
||||
return false
|
||||
const client = new McpClient(config, securityPolicy)
|
||||
await client.connect()
|
||||
return client
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -466,10 +401,12 @@ class McpService {
|
||||
})
|
||||
)
|
||||
|
||||
let failedCount = 0
|
||||
results.forEach((result, index) => {
|
||||
if (result.status === 'fulfilled') {
|
||||
allTools.push(...result.value)
|
||||
} else {
|
||||
failedCount++
|
||||
logger.warn(
|
||||
`[${requestId}] Failed to discover tools from server ${servers[index].name}:`,
|
||||
result.reason
|
||||
@@ -477,10 +414,16 @@ class McpService {
|
||||
}
|
||||
})
|
||||
|
||||
this.setCacheEntry(cacheKey, allTools)
|
||||
if (failedCount === 0) {
|
||||
this.setCacheEntry(cacheKey, allTools)
|
||||
} else {
|
||||
logger.warn(
|
||||
`[${requestId}] Skipping cache due to ${failedCount} failed server(s) - will retry on next request`
|
||||
)
|
||||
}
|
||||
|
||||
logger.info(
|
||||
`[${requestId}] Discovered ${allTools.length} tools from ${servers.length} servers`
|
||||
`[${requestId}] Discovered ${allTools.length} tools from ${servers.length - failedCount}/${servers.length} servers`
|
||||
)
|
||||
return allTools
|
||||
} catch (error) {
|
||||
|
||||
@@ -6,9 +6,10 @@ import type { McpApiResponse } from '@/lib/mcp/types'
|
||||
*/
|
||||
export const MCP_CONSTANTS = {
|
||||
EXECUTION_TIMEOUT: 60000,
|
||||
CACHE_TIMEOUT: 5 * 60 * 1000,
|
||||
CACHE_TIMEOUT: 30 * 1000,
|
||||
DEFAULT_RETRIES: 3,
|
||||
DEFAULT_CONNECTION_TIMEOUT: 30000,
|
||||
MAX_CACHE_SIZE: 1000,
|
||||
} as const
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user