From cad37009e155b57697dced9795a2b04d7ec8fe3e Mon Sep 17 00:00:00 2001 From: Biturd Date: Tue, 9 Sep 2025 00:28:12 +0800 Subject: [PATCH] fix: improve MCP server initialization error handling #3196 #2346 #2555 (#3243) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • I have signed the CLA by commenting the required sentence and triggered recheck. • Local checks are all green (fmt / clippy / test). • Could you please approve the pending GitHub Actions workflows (first-time contributor), and when convenient, help with one approving review so I can proceed? Thanks! ## Summary - Catch and log task panics during server initialization instead of propagating JoinError - Handle tool listing failures gracefully, allowing partial server initialization - Improve error resilience on macOS where init timeouts are more common ## Test plan - [x] Test MCP server initialization with timeout scenarios - [x] Verify graceful handling of tool listing failures - [x] Confirm improved error messages and logging - [x] Test on macOS ## Fix issue #3196 #2346 #2555 ### fix before: image image ### fix improved: image --------- Co-authored-by: Michael Bolin --- codex-rs/core/src/mcp_connection_manager.rs | 31 ++++++++++++++++++--- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/codex-rs/core/src/mcp_connection_manager.rs b/codex-rs/core/src/mcp_connection_manager.rs index 563a4e13..387ea173 100644 --- a/codex-rs/core/src/mcp_connection_manager.rs +++ b/codex-rs/core/src/mcp_connection_manager.rs @@ -187,7 +187,13 @@ impl McpConnectionManager { let mut clients: HashMap = HashMap::with_capacity(join_set.len()); while let Some(res) = join_set.join_next().await { - let (server_name, client_res) = res?; // JoinError propagation + let (server_name, client_res) = match res { + Ok((server_name, client_res)) => (server_name, client_res), + Err(e) => { + warn!("Task panic when starting MCP server: {e:#}"); + continue; + } + }; match client_res { Ok((client, startup_timeout)) => { @@ -205,7 +211,13 @@ impl McpConnectionManager { } } - let all_tools = list_all_tools(&clients).await?; + let all_tools = match list_all_tools(&clients).await { + Ok(tools) => tools, + Err(e) => { + warn!("Failed to list tools from some MCP servers: {e:#}"); + Vec::new() + } + }; let tools = qualify_tools(all_tools); @@ -270,8 +282,19 @@ async fn list_all_tools(clients: &HashMap) -> Result = Vec::with_capacity(join_set.len()); while let Some(join_res) = join_set.join_next().await { - let (server_name, list_result) = join_res?; - let list_result = list_result?; + let (server_name, list_result) = if let Ok(result) = join_res { + result + } else { + warn!("Task panic when listing tools for MCP server: {join_res:#?}"); + continue; + }; + + let list_result = if let Ok(result) = list_result { + result + } else { + warn!("Failed to list tools for MCP server '{server_name}': {list_result:#?}"); + continue; + }; for tool in list_result.tools { let tool_info = ToolInfo {