From e29dfadaa7c1333f9133a341d727e9ddbd4e502e Mon Sep 17 00:00:00 2001 From: asepharyana Date: Sun, 12 Jul 2026 10:50:34 +0700 Subject: [PATCH] refactor: surface silent fallbacks with eprintln! logging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add eprintln! logging before fallback values in 7 files where errors were previously swallowed without visibility: - [stream] malformed JSON chunk, missing usage tokens, missing tool call index - [mcp] missing result fields, client builder failure, response body read error - [subagent] missing API key in settings, all resolution paths exhausted - [state] memory_dir no-parent, session_id missing, lock poisoned, store_base_dir - [input] missing default_model for provider - [session] corrupt agents.json parse failure - [pkce] clock-before-epoch on random byte generation All fallback values are preserved — this adds observability without changing behavior for callers. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/app/mcp/manager.rs | 35 +++++++++++++++++++++++++++------- src/app/runtime/stream/mod.rs | 30 +++++++++++++++++++++++------ src/app/state/rest.rs | 32 ++++++++++++++++++++++++------- src/app/subagent/engine.rs | 10 ++++++++-- src/controller/input.rs | 5 ++++- src/model/agent_def/session.rs | 5 ++++- src/service/oauth/pkce.rs | 5 ++++- 7 files changed, 97 insertions(+), 25 deletions(-) diff --git a/src/app/mcp/manager.rs b/src/app/mcp/manager.rs index d88c04c..80b84af 100644 --- a/src/app/mcp/manager.rs +++ b/src/app/mcp/manager.rs @@ -94,7 +94,10 @@ impl StdioChild { if let Some(err) = resp.get("error") { anyhow::bail!("MCP error: {}", err); } - return Ok(resp.get("result").cloned().unwrap_or(Value::Null)); + return Ok(resp.get("result").cloned().unwrap_or_else(|| { + eprintln!("[mcp] stdio response missing 'result' field: {}", trimmed); + Value::Null + })); } } Err(e) => anyhow::bail!("MCP stdio read error: {}", e), @@ -186,7 +189,10 @@ fn call_via_http(url: &str, tool_name: &str, tool_args: &Value) -> anyhow::Resul .timeout(std::time::Duration::from_millis(MCP_CALL_TIMEOUT_MS)) .connect_timeout(std::time::Duration::from_millis(MCP_CONNECT_TIMEOUT_MS)) .build() - .unwrap_or_else(|_| reqwest::blocking::Client::new()); + .unwrap_or_else(|e| { + eprintln!("[mcp] HTTP client builder failed: {}, using default client without timeouts", e); + reqwest::blocking::Client::new() + }); let request_id: u64 = 1; let body = json!({ @@ -207,7 +213,10 @@ fn call_via_http(url: &str, tool_name: &str, tool_args: &Value) -> anyhow::Resul if !resp.status().is_success() { let status = resp.status(); - let text = resp.text().unwrap_or_default(); + let text = resp.text().unwrap_or_else(|e| { + eprintln!("[mcp] failed to read HTTP response body: {}", e); + String::new() + }); anyhow::bail!("MCP HTTP server returned {}: {}", status, text); } @@ -218,7 +227,10 @@ fn call_via_http(url: &str, tool_name: &str, tool_args: &Value) -> anyhow::Resul anyhow::bail!("MCP HTTP error: {}", err); } - let result = response.get("result").cloned().unwrap_or(Value::Null); + let result = response.get("result").cloned().unwrap_or_else(|| { + eprintln!("[mcp] HTTP response missing 'result' field"); + Value::Null + }); extract_text_content(&result) } @@ -237,7 +249,10 @@ fn extract_text_content(result: &Value) -> anyhow::Result { } } } - Ok(serde_json::to_string_pretty(result).unwrap_or_else(|_| result.to_string())) + Ok(serde_json::to_string_pretty(result).unwrap_or_else(|e| { + eprintln!("[mcp] failed to pretty-print result: {}", e); + result.to_string() + })) } #[derive(Debug, Clone)] @@ -320,8 +335,14 @@ impl McpManager { tool_list.iter().filter_map(|t| { Some(McpToolInfo { name: t.get("name")?.as_str()?.to_string(), - description: t.get("description").and_then(|v| v.as_str()).unwrap_or("").to_string(), - input_schema: t.get("inputSchema").cloned().unwrap_or(serde_json::Value::Null), + description: t.get("description").and_then(|v| v.as_str()).unwrap_or_else(|| { + eprintln!("[mcp] tool {} missing description", t.get("name").and_then(|n| n.as_str()).unwrap_or("?")); + "" + }).to_string(), + input_schema: t.get("inputSchema").cloned().unwrap_or_else(|| { + eprintln!("[mcp] tool {} missing inputSchema", t.get("name").and_then(|n| n.as_str()).unwrap_or("?")); + serde_json::Value::Null + }), }) }).collect() } else { diff --git a/src/app/runtime/stream/mod.rs b/src/app/runtime/stream/mod.rs index 2d3e748..b7eb8b0 100644 --- a/src/app/runtime/stream/mod.rs +++ b/src/app/runtime/stream/mod.rs @@ -69,14 +69,26 @@ impl SseParser { } let value: Value = match serde_json::from_str(&data) { Ok(v) => v, - Err(_) => return vec![], + Err(e) => { + eprintln!("[stream] failed to parse chunk: {}", e); + return vec![]; + } }; if let Some(usage) = value.get("usage") { if !usage.is_null() { - let prompt_tokens = usage.get("prompt_tokens").and_then(|v| v.as_u64()).unwrap_or(0); - let completion_tokens = usage.get("completion_tokens").and_then(|v| v.as_u64()).unwrap_or(0); + let prompt_tokens = usage.get("prompt_tokens").and_then(|v| v.as_u64()).unwrap_or_else(|| { + eprintln!("[stream] prompt_tokens missing in usage chunk"); + 0 + }); + let completion_tokens = usage.get("completion_tokens").and_then(|v| v.as_u64()).unwrap_or_else(|| { + eprintln!("[stream] completion_tokens missing in usage chunk"); + 0 + }); let total_tokens = usage.get("total_tokens").and_then(|v| v.as_u64()) - .unwrap_or(prompt_tokens + completion_tokens); + .unwrap_or_else(|| { + eprintln!("[stream] total_tokens missing in usage chunk"); + prompt_tokens + completion_tokens + }); return vec![StreamEvent::Usage { prompt_tokens, completion_tokens, total_tokens }]; } } @@ -112,7 +124,10 @@ impl SseParser { if let Some(tool_calls) = d.get("tool_calls").and_then(|tc| tc.as_array()) { let mut events = Vec::with_capacity(tool_calls.len()); for tc in tool_calls { - let index = tc.get("index").and_then(|i| i.as_u64()).unwrap_or(0) as usize; + let index = tc.get("index").and_then(|i| i.as_u64()).unwrap_or_else(|| { + eprintln!("[stream] tool call delta missing index, defaulting to 0"); + 0 + }) as usize; let id = tc.get("id").and_then(|i| i.as_str()).map(|s| s.to_string()); let name = tc.get("function") .and_then(|f| f.get("name")) @@ -186,7 +201,10 @@ pub fn parse_stream_chunk(data: &str) -> Option { } if let Some(tool_calls) = delta.get("tool_calls").and_then(|tc| tc.as_array()) { if let Some(tc) = tool_calls.first() { - let index = tc.get("index").and_then(|i| i.as_u64()).unwrap_or(0) as usize; + let index = tc.get("index").and_then(|i| i.as_u64()).unwrap_or_else(|| { + eprintln!("[stream] fallback parser: tool call missing index, defaulting to 0"); + 0 + }) as usize; let id = tc.get("id").and_then(|i| i.as_str()).map(|s| s.to_string()); let name = tc.get("function") .and_then(|f| f.get("name")) diff --git a/src/app/state/rest.rs b/src/app/state/rest.rs index 370a0ad..b7f38b7 100644 --- a/src/app/state/rest.rs +++ b/src/app/state/rest.rs @@ -61,13 +61,22 @@ impl AppStateRest { pub fn new(workspace_roots: Vec, session_dir: PathBuf, memory_dir: PathBuf) -> Self { let settings = Settings::load(); let app_config = AppConfig::load(); - let download_dir = memory_dir.parent().unwrap_or(&memory_dir).join("downloads"); - let worktrees_dir = memory_dir.parent().unwrap_or(&memory_dir).join("worktrees"); + let download_dir = memory_dir.parent().unwrap_or_else(|| { + eprintln!("[state] memory_dir '{}' has no parent, using it for downloads", memory_dir.display()); + &memory_dir + }).join("downloads"); + let worktrees_dir = memory_dir.parent().unwrap_or_else(|| { + eprintln!("[state] memory_dir '{}' has no parent, using it for worktrees", memory_dir.display()); + &memory_dir + }).join("worktrees"); let dir_cache = DirCache::new(); let session_id = session_dir .file_name() .map(|n| n.to_string_lossy().to_string()) - .unwrap_or_default(); + .unwrap_or_else(|| { + eprintln!("[state] session_dir has no file_name component, using empty session_id"); + String::new() + }); AppStateRest { settings, @@ -97,7 +106,10 @@ impl AppStateRest { } pub fn turn_in_flight(&self) -> bool { - self.turn_in_flight.lock().map(|g| *g).unwrap_or(false) + self.turn_in_flight.lock().map(|g| *g).unwrap_or_else(|_| { + eprintln!("[state] turn_in_flight mutex poisoned"); + false + }) } @@ -120,9 +132,15 @@ impl AppStateRest { self.session_dir.parent() .and_then(|p| p.parent()) .map(|p| p.to_path_buf()) - .unwrap_or_else(|| self.session_dir.parent() - .map(|p| p.to_path_buf()) - .unwrap_or_else(|| self.session_dir.clone())) + .unwrap_or_else(|| { + eprintln!("[state] session_dir '{}' has no grandparent, using parent", self.session_dir.display()); + self.session_dir.parent() + .map(|p| p.to_path_buf()) + .unwrap_or_else(|| { + eprintln!("[state] session_dir '{}' has no parent at all, using itself", self.session_dir.display()); + self.session_dir.clone() + }) + }) } pub fn tool_ctx(&self) -> crate::tool::ToolCtx { diff --git a/src/app/subagent/engine.rs b/src/app/subagent/engine.rs index 565cc8f..a90dc16 100644 --- a/src/app/subagent/engine.rs +++ b/src/app/subagent/engine.rs @@ -31,7 +31,10 @@ fn resolve_provider_config() -> (String, String, Option) { let settings = crate::model::settings::Settings::load(); let app_config = crate::model::app_config::AppConfig::load(); - let mut api_key = settings.api_keys.get(&settings.provider).cloned().unwrap_or_default(); + let mut api_key = settings.api_keys.get(&settings.provider).cloned().unwrap_or_else(|| { + eprintln!("[subagent] no API key for provider '{}' in settings, trying env/default", settings.provider); + String::new() + }); let model = settings.model.clone(); let base_url = app_config.providers.get(&settings.provider) .map(|p| p.api_base.clone()); @@ -41,7 +44,10 @@ fn resolve_provider_config() -> (String, String, Option) { api_key = provider_cfg.api_key_env.as_ref() .and_then(|env| std::env::var(env).ok()) .or_else(|| provider_cfg.default_api_key.clone()) - .unwrap_or_default(); + .unwrap_or_else(|| { + eprintln!("[subagent] all API key resolution paths exhausted for '{}'", settings.provider); + String::new() + }); } } diff --git a/src/controller/input.rs b/src/controller/input.rs index 69a8d96..29478e1 100644 --- a/src/controller/input.rs +++ b/src/controller/input.rs @@ -244,7 +244,10 @@ fn handle_overlay_enter(state: &mut AppStateRest) -> Vec { let providers: Vec = state.app_config.providers.keys().cloned().collect(); if let Some(provider) = providers.get(state.misc.selected_index) { if let Some(cfg) = state.app_config.providers.get(provider) { - let model = cfg.default_model.clone().unwrap_or_else(|| "claude-opus-4-8".to_string()); + let model = cfg.default_model.clone().unwrap_or_else(|| { + eprintln!("[input] provider '{}' has no default_model, using 'claude-opus-4-8'", provider); + "claude-opus-4-8".to_string() + }); state.settings.provider = provider.clone(); state.settings.model = model.clone(); if let Some(ref key) = cfg.default_api_key { diff --git a/src/model/agent_def/session.rs b/src/model/agent_def/session.rs index be907b0..28cbf3f 100644 --- a/src/model/agent_def/session.rs +++ b/src/model/agent_def/session.rs @@ -8,7 +8,10 @@ pub fn load_session_agents(session_dir: &Path) -> Vec { } match std::fs::read_to_string(&agents_file) { Ok(content) => { - serde_json::from_str(&content).unwrap_or_default() + serde_json::from_str(&content).unwrap_or_else(|e| { + eprintln!("[session] failed to parse agents.json: {}", e); + Vec::new() + }) } Err(_) => Vec::new(), } diff --git a/src/service/oauth/pkce.rs b/src/service/oauth/pkce.rs index f421abe..e8b805a 100644 --- a/src/service/oauth/pkce.rs +++ b/src/service/oauth/pkce.rs @@ -27,7 +27,10 @@ fn rand_byte() -> u8 { use std::time::{SystemTime, UNIX_EPOCH}; let nanos = SystemTime::now() .duration_since(UNIX_EPOCH) - .unwrap_or_default() + .unwrap_or_else(|_| { + eprintln!("[pkce] system time before UNIX_EPOCH, using 0 for random byte"); + std::time::Duration::default() + }) .subsec_nanos(); (nanos & 0xFF) as u8 }