refactor: surface silent fallbacks with eprintln! logging
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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
bb621fdff2
commit
e29dfadaa7
+28
-7
@@ -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<String> {
|
||||
}
|
||||
}
|
||||
}
|
||||
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 {
|
||||
|
||||
@@ -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<StreamEvent> {
|
||||
}
|
||||
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"))
|
||||
|
||||
+25
-7
@@ -61,13 +61,22 @@ impl AppStateRest {
|
||||
pub fn new(workspace_roots: Vec<PathBuf>, 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 {
|
||||
|
||||
@@ -31,7 +31,10 @@ fn resolve_provider_config() -> (String, String, Option<String>) {
|
||||
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<String>) {
|
||||
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()
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -244,7 +244,10 @@ fn handle_overlay_enter(state: &mut AppStateRest) -> Vec<Action> {
|
||||
let providers: Vec<String> = 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 {
|
||||
|
||||
@@ -8,7 +8,10 @@ pub fn load_session_agents(session_dir: &Path) -> Vec<AgentDefinition> {
|
||||
}
|
||||
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(),
|
||||
}
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user