From 5c44ee91fbcdc6cee39f4e4e043bc87bade8f413 Mon Sep 17 00:00:00 2001 From: teernisse Date: Tue, 10 Mar 2026 17:10:52 -0400 Subject: [PATCH] fix(robot): propagate JSON serialization errors instead of silent failure Three robot-mode print functions used `serde_json::to_string().unwrap_or_default()` which silently outputs an empty string on failure (exit 0, no error). This diverged from the codebase standard in handlers.rs which uses `?` propagation. Changed to return Result<()> with proper LoreError::Other mapping: - explain.rs: print_explain_json() - file_history.rs: print_file_history_json() - trace.rs: print_trace_json() Updated callers in handlers.rs and explain.rs to propagate with `?`. While serde_json::to_string on a json!() Value is unlikely to fail in practice (only non-finite floats trigger it), the unwrap_or_default pattern violates the robot mode contract: callers expect either valid JSON on stdout or a structured error on stderr with a non-zero exit code, never empty output with exit 0. --- src/app/handlers.rs | 4 ++-- src/cli/commands/file_history.rs | 11 ++++++++--- src/cli/commands/trace.rs | 14 ++++++++++++-- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/app/handlers.rs b/src/app/handlers.rs index 6eb3760..4083322 100644 --- a/src/app/handlers.rs +++ b/src/app/handlers.rs @@ -1328,7 +1328,7 @@ fn handle_file_history( if robot_mode { let elapsed_ms = start.elapsed().as_millis() as u64; - print_file_history_json(&result, elapsed_ms); + print_file_history_json(&result, elapsed_ms)?; } else { print_file_history(&result); } @@ -1384,7 +1384,7 @@ fn handle_trace( if robot_mode { let elapsed_ms = start.elapsed().as_millis() as u64; - print_trace_json(&result, elapsed_ms, line_requested); + print_trace_json(&result, elapsed_ms, line_requested)?; } else { print_trace(&result); } diff --git a/src/cli/commands/file_history.rs b/src/cli/commands/file_history.rs index 588e87e..6bbec0e 100644 --- a/src/cli/commands/file_history.rs +++ b/src/cli/commands/file_history.rs @@ -5,7 +5,7 @@ use tracing::info; use crate::Config; use crate::cli::render::{self, Icons, Theme}; use crate::core::db::create_connection; -use crate::core::error::Result; +use crate::core::error::{LoreError, Result}; use crate::core::file_history::resolve_rename_chain; use crate::core::paths::get_db_path; use crate::core::project::resolve_project; @@ -391,7 +391,7 @@ pub fn print_file_history(result: &FileHistoryResult) { // ── Robot (JSON) output ───────────────────────────────────────────────────── -pub fn print_file_history_json(result: &FileHistoryResult, elapsed_ms: u64) { +pub fn print_file_history_json(result: &FileHistoryResult, elapsed_ms: u64) -> Result<()> { let output = serde_json::json!({ "ok": true, "data": { @@ -409,5 +409,10 @@ pub fn print_file_history_json(result: &FileHistoryResult, elapsed_ms: u64) { } }); - println!("{}", serde_json::to_string(&output).unwrap_or_default()); + println!( + "{}", + serde_json::to_string(&output) + .map_err(|e| LoreError::Other(format!("JSON serialization failed: {e}")))? + ); + Ok(()) } diff --git a/src/cli/commands/trace.rs b/src/cli/commands/trace.rs index e56ee07..120b87d 100644 --- a/src/cli/commands/trace.rs +++ b/src/cli/commands/trace.rs @@ -1,4 +1,5 @@ use crate::cli::render::{Icons, Theme}; +use crate::core::error::{LoreError, Result}; use crate::core::trace::{TraceChain, TraceResult}; /// Parse a path with optional `:line` suffix. @@ -152,7 +153,11 @@ fn truncate_body(body: &str, max: usize) -> String { format!("{}...", &body[..boundary]) } -pub fn print_trace_json(result: &TraceResult, elapsed_ms: u64, line_requested: Option) { +pub fn print_trace_json( + result: &TraceResult, + elapsed_ms: u64, + line_requested: Option, +) -> Result<()> { // Truncate discussion bodies for token efficiency in robot mode let chains: Vec = result .trace_chains @@ -205,7 +210,12 @@ pub fn print_trace_json(result: &TraceResult, elapsed_ms: u64, line_requested: O } }); - println!("{}", serde_json::to_string(&output).unwrap_or_default()); + println!( + "{}", + serde_json::to_string(&output) + .map_err(|e| LoreError::Other(format!("JSON serialization failed: {e}")))? + ); + Ok(()) } #[cfg(test)]