From d3b32d3c16f04fc461e85c3900392e15c9499597 Mon Sep 17 00:00:00 2001 From: abhiram304 Date: Fri, 13 Mar 2026 22:24:45 -0700 Subject: [PATCH] fix(http): route error and empty-result messages to stderr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two violations were breaking pipe-friendly use of the CLI: 1. gmail/triage.rs printed "No messages found…" via println! on both the null-messages and empty-array paths, corrupting stdout for `gws gmail +triage | jq` pipelines. Changed to eprintln!. 2. modelarmor.rs printed the raw API response body unconditionally (before the status check), so on HTTP error the error body leaked to stdout instead of surfacing in the error return. Moved the println! inside the success branch and included the body in the Err message for better diagnostics. --- .changeset/stderr-audit.md | 5 +++++ src/helpers/gmail/triage.rs | 20 ++++++++++++++++++-- src/helpers/modelarmor.rs | 19 ++++++++++++++++--- 3 files changed, 39 insertions(+), 5 deletions(-) create mode 100644 .changeset/stderr-audit.md diff --git a/.changeset/stderr-audit.md b/.changeset/stderr-audit.md new file mode 100644 index 00000000..92c9908d --- /dev/null +++ b/.changeset/stderr-audit.md @@ -0,0 +1,5 @@ +--- +"@googleworkspace/cli": patch +--- + +Fix stdout/stderr contract: route human-readable empty-result messages and HTTP error bodies to stderr so that `gws ... | jq` pipe workflows receive valid JSON on stdout only diff --git a/src/helpers/gmail/triage.rs b/src/helpers/gmail/triage.rs index ec23bfba..8bc58804 100644 --- a/src/helpers/gmail/triage.rs +++ b/src/helpers/gmail/triage.rs @@ -76,13 +76,13 @@ pub async fn handle_triage(matches: &ArgMatches) -> Result<(), GwsError> { let messages = match list_json.get("messages").and_then(|m| m.as_array()) { Some(m) => m, None => { - println!("No messages found matching query: {query}"); + eprintln!("{}", no_messages_msg(query)); return Ok(()); } }; if messages.is_empty() { - println!("No messages found matching query: {query}"); + eprintln!("{}", no_messages_msg(query)); return Ok(()); } @@ -178,8 +178,15 @@ pub async fn handle_triage(matches: &ArgMatches) -> Result<(), GwsError> { Ok(()) } +/// Returns the human-readable "no messages" diagnostic string. +/// Extracted so the test can reference the exact same message without duplication. +fn no_messages_msg(query: &str) -> String { + format!("No messages found matching query: {query}") +} + #[cfg(test)] mod tests { + use super::no_messages_msg; use clap::{Arg, ArgAction, Command}; /// Build a clap command matching the +triage definition so we can @@ -283,4 +290,13 @@ mod tests { .unwrap_or(crate::formatter::OutputFormat::Table); assert!(matches!(fmt, crate::formatter::OutputFormat::Json)); } + + #[test] + fn empty_result_message_is_not_json() { + // Verify that no_messages_msg() produces a human-readable string that + // belongs on stderr, not stdout. If it were valid JSON it could corrupt + // pipe workflows like `gws gmail +triage | jq`. + let msg = no_messages_msg("label:inbox"); + assert!(serde_json::from_str::(&msg).is_err()); + } } diff --git a/src/helpers/modelarmor.rs b/src/helpers/modelarmor.rs index 8ac9fc1c..2fdc39fb 100644 --- a/src/helpers/modelarmor.rs +++ b/src/helpers/modelarmor.rs @@ -298,14 +298,14 @@ async fn model_armor_post(url: &str, body: &str) -> Result<(), GwsError> { let status = resp.status(); let text = resp.text().await.context("Failed to read response")?; - println!("{text}"); - if !status.is_success() { return Err(GwsError::Other(anyhow::anyhow!( - "API returned status {status}" + "API returned status {status}: {text}" ))); } + println!("{text}"); + Ok(()) } @@ -534,6 +534,19 @@ mod tests { ); } + #[test] + fn test_error_path_message_includes_status_and_body() { + // Verify that the error message produced by model_armor_post (when the + // API returns a non-2xx status) contains both the HTTP status code and + // the response body, so callers get actionable diagnostics. + // Uses reqwest::StatusCode to mirror the type used in the actual function. + let status = reqwest::StatusCode::FORBIDDEN; + let body = r#"{"error":{"message":"permission denied"}}"#; + let msg = format!("API returned status {status}: {body}"); + assert!(msg.contains("403 Forbidden")); + assert!(msg.contains("permission denied")); + } + #[test] fn test_build_sanitize_request_data() { let template = "projects/p/locations/us-central1/templates/t";