fix: Handle ExceptionGroup errors in MCP client cleanup (#8561)
The MCP library internally uses TaskGroup for async operations, which can raise ExceptionGroup when cleanup fails. This was causing unhandled errors to propagate in production. Changes: - Update cleanup() method in AsyncBaseMCPClient to catch ExceptionGroup using except* syntax and log errors at debug level (best-effort cleanup) - Remove redundant try/except blocks in mcp_manager.py and mcp_server_manager.py that incorrectly re-raised cleanup exceptions Fixes #8560 🐾 Generated with [Letta Code](https://letta.com) Co-authored-by: letta-code <248085862+letta-code@users.noreply.github.com> Co-authored-by: Letta <noreply@letta.com> Co-authored-by: Kian Jones <11655409+kianjones9@users.noreply.github.com>
This commit is contained in:
committed by
Sarah Wooders
parent
282df3a3fe
commit
f67af1b13d
@@ -109,9 +109,21 @@ class AsyncBaseMCPClient:
|
||||
logger.error("MCPClient has not been initialized")
|
||||
raise RuntimeError("MCPClient has not been initialized")
|
||||
|
||||
# TODO: still hitting some async errors for voice agents, need to fix
|
||||
async def cleanup(self):
|
||||
await self.exit_stack.aclose()
|
||||
"""Clean up resources used by the MCP client.
|
||||
|
||||
This method handles ExceptionGroup errors that can occur when closing async context managers
|
||||
(e.g., from the MCP library's internal TaskGroup usage). Cleanup is a best-effort operation
|
||||
and errors are logged but not re-raised to prevent masking the original exception.
|
||||
"""
|
||||
try:
|
||||
await self.exit_stack.aclose()
|
||||
except* Exception as eg:
|
||||
# ExceptionGroup can be raised when closing async context managers that use TaskGroup
|
||||
# Log each sub-exception at debug level since cleanup errors are expected in some cases
|
||||
# (e.g., connection already closed, server unavailable)
|
||||
for exc in eg.exceptions:
|
||||
logger.debug(f"MCP client cleanup error (suppressed): {type(exc).__name__}: {exc}")
|
||||
|
||||
def to_sync_client(self):
|
||||
raise NotImplementedError("Subclasses must implement to_sync_client")
|
||||
|
||||
@@ -93,12 +93,7 @@ class MCPManager:
|
||||
raise e
|
||||
finally:
|
||||
if mcp_client:
|
||||
try:
|
||||
await mcp_client.cleanup()
|
||||
except* Exception as eg:
|
||||
for e in eg.exceptions:
|
||||
logger.warning(f"Error listing tools for MCP server {mcp_server_name}: {e}")
|
||||
raise e
|
||||
await mcp_client.cleanup()
|
||||
|
||||
@enforce_types
|
||||
async def execute_mcp_server_tool(
|
||||
|
||||
@@ -186,11 +186,7 @@ class MCPServerManager:
|
||||
raise e
|
||||
finally:
|
||||
if mcp_client:
|
||||
try:
|
||||
await mcp_client.cleanup()
|
||||
except Exception as e:
|
||||
logger.warning(f"Error listing tools for MCP server {mcp_server_id}: {e}")
|
||||
raise e
|
||||
await mcp_client.cleanup()
|
||||
|
||||
@enforce_types
|
||||
async def execute_mcp_server_tool(
|
||||
|
||||
Reference in New Issue
Block a user