From ff268280559c4e17d0a0baccc91b1f3811d80afe Mon Sep 17 00:00:00 2001 From: Nando Vieira Date: Tue, 30 Jun 2026 12:32:16 -0700 Subject: [PATCH 1/4] Add output mode for human and JSON results. --- Cargo.lock | 1 + cmd/soroban-cli/Cargo.toml | 3 + cmd/soroban-cli/src/cli.rs | 40 ++++ .../src/commands/network/health.rs | 34 ++- cmd/soroban-cli/src/commands/network/info.rs | 32 ++- .../src/commands/network/settings.rs | 25 +- cmd/soroban-cli/src/lib.rs | 1 + cmd/soroban-cli/src/output.rs | 220 ++++++++++++++++++ 8 files changed, 321 insertions(+), 35 deletions(-) create mode 100644 cmd/soroban-cli/src/output.rs diff --git a/Cargo.lock b/Cargo.lock index 9d44e32e1b..fc66e3a311 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5413,6 +5413,7 @@ dependencies = [ "humantime", "indexmap 2.11.0", "itertools 0.10.5", + "jsonrpsee-types", "keyring", "mockito", "num-bigint", diff --git a/cmd/soroban-cli/Cargo.toml b/cmd/soroban-cli/Cargo.toml index 634cdc342f..6288942627 100644 --- a/cmd/soroban-cli/Cargo.toml +++ b/cmd/soroban-cli/Cargo.toml @@ -62,6 +62,9 @@ serde_json = { workspace = true } serde-aux = { workspace = true } hex = { workspace = true } num-bigint = "0.4" +# Pinned to match the version pulled in by soroban-rpc (jsonrpsee-core) so that +# downcasting RPC errors to `ErrorObjectOwned` resolves to the same type. +jsonrpsee-types = "0.26" tokio = { version = "1", features = ["full"] } tokio-util = { version = "0.7.11", features = ["io", "io-util", "compat"] } termcolor = { workspace = true } diff --git a/cmd/soroban-cli/src/cli.rs b/cmd/soroban-cli/src/cli.rs index a088f069cf..2649b6ab45 100644 --- a/cmd/soroban-cli/src/cli.rs +++ b/cmd/soroban-cli/src/cli.rs @@ -94,6 +94,15 @@ pub async fn main() { println!("{help}"); std::process::exit(1); } + // When JSON output is requested, render the error as JSON on stdout so + // machine consumers get parseable output instead of the human line. The + // process still exits non-zero: this is a failure, the JSON body is just + // a parseable representation of it. + if let Some(format) = output_format_from_raw_args() { + let output = crate::output::Output::new(format, root.global_args.quiet); + let _ = output.json_value(&crate::output::error_json(&e)); + std::process::exit(1); + } printer.errorln(format!("error: {e}")); std::process::exit(1); } @@ -115,6 +124,37 @@ fn set_env_from_config() { set_env_value_from_config("STELLAR_INCLUSION_FEE", config.defaults.inclusion_fee); } +// Detect a JSON `--output` value from the raw args so the top-level error +// handler can render errors as JSON. `--output` is per-command (not global), so +// the parsed args are unavailable once an error propagates this far; we mirror +// `config_dir_from_raw_args` and sniff the raw args instead. Returns `None` for +// non-JSON formats (text, xdr, pretty, …), which are rendered as the human line. +fn output_format_from_raw_args() -> Option { + use std::ffi::OsStr; + let mut iter = std::env::args_os().peekable(); + let format_for = |value: &str| match value { + "json" => Some(crate::output::Format::Json), + "json-formatted" => Some(crate::output::Format::JsonFormatted), + _ => None, + }; + while let Some(arg) = iter.next() { + if arg == OsStr::new("--") { + break; + } + if arg == OsStr::new("--output") { + return iter + .next() + .as_deref() + .and_then(OsStr::to_str) + .and_then(format_for); + } + if let Some(val) = arg.to_str().and_then(|s| s.strip_prefix("--output=")) { + return format_for(val); + } + } + None +} + fn config_dir_from_raw_args() -> Option { use std::ffi::OsStr; let mut iter = std::env::args_os().peekable(); diff --git a/cmd/soroban-cli/src/commands/network/health.rs b/cmd/soroban-cli/src/commands/network/health.rs index 581c308af8..932bc0efa0 100644 --- a/cmd/soroban-cli/src/commands/network/health.rs +++ b/cmd/soroban-cli/src/commands/network/health.rs @@ -1,6 +1,6 @@ use crate::commands::global; -use crate::config::network; -use crate::{config, print}; +use crate::config::{self, network}; +use crate::output::{Format, Output}; #[derive(thiserror::Error, Debug)] pub enum Error { @@ -23,6 +23,16 @@ pub enum OutputFormat { JsonFormatted, } +impl From for Format { + fn from(value: OutputFormat) -> Self { + match value { + OutputFormat::Text => Format::Readable, + OutputFormat::Json => Format::Json, + OutputFormat::JsonFormatted => Format::JsonFormatted, + } + } +} + #[derive(Debug, clap::Parser, Clone)] #[group(skip)] pub struct Cmd { @@ -35,25 +45,27 @@ pub struct Cmd { impl Cmd { pub async fn run(&self, global_args: &global::Args) -> Result<(), Error> { - let print = print::Print::new(global_args.quiet); + let output = Output::new(self.output.into(), global_args.quiet); let result = self.config.get_network()?.rpc_client()?.get_health().await; match result { - Ok(resp) => match self.output { - OutputFormat::Text => { + Ok(resp) => { + output.readable(|print| { if resp.status.eq_ignore_ascii_case("healthy") { print.checkln("Healthy"); } else { print.warnln(format!("Status: {}", resp.status)); } print.infoln(format!("Latest ledger: {}", resp.latest_ledger)); - } - OutputFormat::Json => println!("{}", serde_json::to_string(&resp)?), - OutputFormat::JsonFormatted => println!("{}", serde_json::to_string_pretty(&resp)?), - }, + }); + output.json_value(&resp)?; + } Err(err) => { - print.errorln("Unhealthy"); - print.errorln(format!("failed to fetch network health: {err}")); + output.readable(|print| { + print.errorln("Unhealthy"); + print.errorln(format!("failed to fetch network health: {err}")); + }); + output.json_value(&crate::output::error_json(&err))?; } } diff --git a/cmd/soroban-cli/src/commands/network/info.rs b/cmd/soroban-cli/src/commands/network/info.rs index 090f2cf707..c23dd1e308 100644 --- a/cmd/soroban-cli/src/commands/network/info.rs +++ b/cmd/soroban-cli/src/commands/network/info.rs @@ -2,6 +2,7 @@ use sha2::{Digest, Sha256}; use crate::commands::global; use crate::config::network; +use crate::output::{Format, Output}; use crate::utils::url::redact_url; use crate::{config, print, rpc}; @@ -28,6 +29,16 @@ pub enum OutputFormat { JsonFormatted, } +impl From for Format { + fn from(value: OutputFormat) -> Self { + match value { + OutputFormat::Text => Format::Readable, + OutputFormat::Json => Format::Json, + OutputFormat::JsonFormatted => Format::JsonFormatted, + } + } +} + #[derive(Debug, clap::Parser, Clone)] #[group(skip)] pub struct Cmd { @@ -66,23 +77,11 @@ impl Info { print.infoln(format!("Friendbot Url: {friendbot_url}")); } } - - fn print_json(&self) -> Result<(), serde_json::Error> { - let json = serde_json::to_string(&self)?; - println!("{json}"); - Ok(()) - } - - fn print_json_formatted(&self) -> Result<(), serde_json::Error> { - let json = serde_json::to_string_pretty(&self)?; - println!("{json}"); - Ok(()) - } } impl Cmd { pub async fn run(&self, global_args: &global::Args) -> Result<(), Error> { - let print = print::Print::new(global_args.quiet); + let output = Output::new(self.output.into(), global_args.quiet); let rpc_client = self.config.get_network()?.rpc_client()?; let network_result = rpc_client.get_network().await?; let version_result = rpc_client.get_version_info().await?; @@ -98,11 +97,8 @@ impl Cmd { passphrase: network_result.passphrase, }; - match self.output { - OutputFormat::Text => info.print_text(&print), - OutputFormat::Json => info.print_json()?, - OutputFormat::JsonFormatted => info.print_json_formatted()?, - } + output.readable(|print| info.print_text(print)); + output.json_value(&info)?; Ok(()) } diff --git a/cmd/soroban-cli/src/commands/network/settings.rs b/cmd/soroban-cli/src/commands/network/settings.rs index b24eabb696..b4c04b9094 100644 --- a/cmd/soroban-cli/src/commands/network/settings.rs +++ b/cmd/soroban-cli/src/commands/network/settings.rs @@ -1,5 +1,5 @@ use crate::config::network; -use crate::print::Print; +use crate::output::{Format, Output}; use crate::{commands::global, config}; use semver::Version; use stellar_xdr::{ @@ -34,6 +34,17 @@ pub enum OutputFormat { JsonFormatted, } +impl From for Format { + fn from(value: OutputFormat) -> Self { + match value { + // Xdr is a raw, human-facing rendering handled outside the JSON path. + OutputFormat::Xdr => Format::Readable, + OutputFormat::Json => Format::Json, + OutputFormat::JsonFormatted => Format::JsonFormatted, + } + } +} + #[derive(Debug, clap::Parser, Clone)] #[group(skip)] pub struct Cmd { @@ -50,7 +61,7 @@ pub struct Cmd { impl Cmd { pub async fn run(&self, global_args: &global::Args) -> Result<(), Error> { - let print = Print::new(global_args.quiet); + let output = Output::new(self.output.into(), global_args.quiet); let rpc = self.config.get_network()?.rpc_client()?; // If the network protocol version is ahead of the XDR version (which tracks the protocol @@ -60,7 +71,9 @@ impl Cmd { let network_version = rpc.get_version_info().await?.protocol_version; let self_version = Version::parse(stellar_xdr::VERSION.pkg)?.major; if self_version < network_version.into() { - print.warnln(format!("Network protocol version is {network_version} but the stellar-cli supports {self_version}. The config fetched may not represent the complete config settings for the network. Upgrade the stellar-cli.")); + // Diagnostic about data completeness; emitted regardless of format + // (to stderr) so JSON consumers reading stdout are unaffected. + output.print().warnln(format!("Network protocol version is {network_version} but the stellar-cli supports {self_version}. The config fetched may not represent the complete config settings for the network. Upgrade the stellar-cli.")); } // Collect the ledger entries for all the config settings. @@ -96,10 +109,10 @@ impl Cmd { updated_entry: settings.try_into().unwrap(), }; match self.output { + // Xdr is a distinct raw rendering, handled outside the JSON path. OutputFormat::Xdr => println!("{}", config_upgrade_set.to_xdr_base64(Limits::none())?), - OutputFormat::Json => println!("{}", serde_json::to_string(&config_upgrade_set)?), - OutputFormat::JsonFormatted => { - println!("{}", serde_json::to_string_pretty(&config_upgrade_set)?); + OutputFormat::Json | OutputFormat::JsonFormatted => { + output.json_value(&config_upgrade_set)?; } } Ok(()) diff --git a/cmd/soroban-cli/src/lib.rs b/cmd/soroban-cli/src/lib.rs index 884d4b3d5d..a575436af3 100644 --- a/cmd/soroban-cli/src/lib.rs +++ b/cmd/soroban-cli/src/lib.rs @@ -20,6 +20,7 @@ mod env_vars; pub mod get_spec; pub mod key; pub mod log; +pub mod output; pub mod print; pub mod resources; pub mod signer; diff --git a/cmd/soroban-cli/src/output.rs b/cmd/soroban-cli/src/output.rs new file mode 100644 index 0000000000..4da0131d38 --- /dev/null +++ b/cmd/soroban-cli/src/output.rs @@ -0,0 +1,220 @@ +//! Unified output abstraction for commands that support both human-readable and +//! JSON output. +//! +//! Commands construct an [`Output`] from their `--output` format and the global +//! `--quiet` flag, then route all output through it: +//! +//! * [`Output::readable`] runs its closure only in human-readable mode, handing +//! it a [`Print`] for progress/status messages and text rendering. +//! * [`Output::json`] / [`Output::json_value`] run only in JSON mode and write +//! the final machine-readable result to stdout. +//! +//! Output is never buffered: each call writes immediately, so long-running +//! operations stream progress to the terminal instead of holding it silently. + +use serde::Serialize; + +use crate::print::Print; + +/// Build the canonical JSON error envelope for an error: `{ "error": { … } }`. +/// +/// The inner object is always present and always has at least a `message` +/// string, so every command renders errors with the same shape. When the error +/// chain contains a JSON-RPC [`ErrorObject`](jsonrpsee_types::ErrorObjectOwned) +/// (whose own `Display` is just its debug representation), its structured +/// `{ code, message, data? }` form is used; every other error falls back to its +/// `Display` as the `message`. +#[must_use] +pub fn error_json(err: &(dyn std::error::Error + 'static)) -> serde_json::Value { + let mut current: Option<&(dyn std::error::Error + 'static)> = Some(err); + while let Some(source) = current { + if let Some(object) = source.downcast_ref::() { + if let Ok(value) = serde_json::to_value(object) { + return serde_json::json!({ "error": value }); + } + } + current = source.source(); + } + + serde_json::json!({ "error": { "message": err.to_string() } }) +} + +/// The format a command should render its output in. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum Format { + /// Human-readable output. + Readable, + /// Compact, single-line JSON. + Json, + /// Pretty-printed, multi-line JSON. + JsonFormatted, +} + +impl Format { + /// Whether this format is one of the JSON variants. + #[must_use] + pub fn is_json(self) -> bool { + matches!(self, Format::Json | Format::JsonFormatted) + } +} + +/// Wraps the output [`Format`] and a [`Print`] (carrying `--quiet`) so commands +/// can emit human-readable and JSON output through a single value. +#[derive(Clone)] +pub struct Output { + format: Format, + print: Print, +} + +impl Output { + /// Create an `Output` for the given format, honoring the global `quiet` flag + /// for human-readable output. + #[must_use] + pub fn new(format: Format, quiet: bool) -> Self { + Self { + format, + print: Print::new(quiet), + } + } + + /// Whether the output format is JSON-based. + #[must_use] + pub fn is_json(&self) -> bool { + self.format.is_json() + } + + /// Whether JSON output should be compact (single-line) rather than pretty. + #[must_use] + pub fn compact_json(&self) -> bool { + self.format == Format::Json + } + + /// The underlying [`Print`]. Output written through this is gated only by + /// `--quiet`, not by the output format, so it is still emitted in JSON mode + /// (to stderr). Use it for diagnostics that remain relevant alongside JSON; + /// prefer [`Output::readable`] for human-readable output that should be + /// suppressed entirely in JSON mode. + #[must_use] + pub fn print(&self) -> &Print { + &self.print + } + + /// Run `f` with a [`Print`] only when rendering human-readable output. A + /// no-op in JSON mode. + pub fn readable(&self, f: F) { + if !self.format.is_json() { + f(&self.print); + } + } + + /// Run `f` only when rendering JSON output. A no-op in human-readable mode. + /// Use this for streaming or custom JSON (e.g. NDJSON); for a single value + /// prefer [`Output::json_value`]. + pub fn json(&self, f: F) { + if self.format.is_json() { + f(self); + } + } + + /// Serialize `value` to stdout as the final JSON result, compact or + /// pretty-printed depending on the format. A no-op in human-readable mode. + /// + /// # Errors + /// If `value` fails to serialize. + pub fn json_value(&self, value: &T) -> Result<(), serde_json::Error> { + match self.format { + Format::Json => println!("{}", serde_json::to_string(value)?), + Format::JsonFormatted => println!("{}", serde_json::to_string_pretty(value)?), + Format::Readable => {} + } + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::cell::Cell; + + #[test] + fn format_is_json() { + assert!(!Format::Readable.is_json()); + assert!(Format::Json.is_json()); + assert!(Format::JsonFormatted.is_json()); + } + + #[test] + fn readable_runs_only_in_readable_mode() { + for (format, expected) in [ + (Format::Readable, true), + (Format::Json, false), + (Format::JsonFormatted, false), + ] { + let output = Output::new(format, false); + let ran = Cell::new(false); + output.readable(|_| ran.set(true)); + assert_eq!(ran.get(), expected, "format: {format:?}"); + } + } + + #[test] + fn json_runs_only_in_json_mode() { + for (format, expected) in [ + (Format::Readable, false), + (Format::Json, true), + (Format::JsonFormatted, true), + ] { + let output = Output::new(format, false); + let ran = Cell::new(false); + output.json(|_| ran.set(true)); + assert_eq!(ran.get(), expected, "format: {format:?}"); + } + } + + #[test] + fn compact_json_only_for_compact_format() { + assert!(Output::new(Format::Json, false).compact_json()); + assert!(!Output::new(Format::JsonFormatted, false).compact_json()); + assert!(!Output::new(Format::Readable, false).compact_json()); + } + + #[test] + fn error_json_uses_structured_rpc_error_object() { + let obj = jsonrpsee_types::ErrorObject::owned(-32603, "DB is empty", None::<()>); + assert_eq!( + error_json(&obj), + serde_json::json!({ "error": { "code": -32603, "message": "DB is empty" } }), + ); + } + + #[test] + fn error_json_walks_the_source_chain() { + #[derive(Debug)] + struct Wrapper(jsonrpsee_types::ErrorObjectOwned); + impl std::fmt::Display for Wrapper { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "wrapper") + } + } + impl std::error::Error for Wrapper { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + Some(&self.0) + } + } + + let obj = jsonrpsee_types::ErrorObject::owned(-32000, "boom", None::<()>); + assert_eq!( + error_json(&Wrapper(obj)), + serde_json::json!({ "error": { "code": -32000, "message": "boom" } }), + ); + } + + #[test] + fn error_json_falls_back_to_message_for_other_errors() { + let err = std::io::Error::new(std::io::ErrorKind::NotFound, "nope"); + assert_eq!( + error_json(&err), + serde_json::json!({ "error": { "message": "nope" } }), + ); + } +} From a5ce7e43b0596f005bd0cb6ba1d39a1adf196e6f Mon Sep 17 00:00:00 2001 From: Nando Vieira Date: Tue, 30 Jun 2026 13:40:46 -0700 Subject: [PATCH 2/4] Exit non-zero on failure and cover JSON errors. --- cmd/crates/soroban-test/tests/it/config.rs | 36 +++++++++++++++++ .../src/commands/network/health.rs | 40 ++++++++++--------- cmd/soroban-cli/src/output.rs | 6 ++- 3 files changed, 61 insertions(+), 21 deletions(-) diff --git a/cmd/crates/soroban-test/tests/it/config.rs b/cmd/crates/soroban-test/tests/it/config.rs index ab8233b373..f31a7982b1 100644 --- a/cmd/crates/soroban-test/tests/it/config.rs +++ b/cmd/crates/soroban-test/tests/it/config.rs @@ -50,6 +50,42 @@ fn set_and_remove_network() { }); } +// When `--output json*` is requested, a failing command must render a +// structured `{ "error": { "message": … } }` envelope on stdout and exit +// non-zero, so machine consumers can parse the failure and still detect it via +// the exit code. An unreachable RPC is a representative, network-free failure. +#[test] +fn json_output_renders_structured_errors_and_exits_non_zero() { + let sandbox = TestEnv::default(); + + for format in ["json", "json-formatted"] { + let stdout = sandbox + .new_assert_cmd("network") + .arg("health") + .args([ + "--rpc-url=http://127.0.0.1:1", + "--network-passphrase", + LOCAL_NETWORK_PASSPHRASE, + &format!("--output={format}"), + ]) + .assert() + .failure() + .stdout_as_str(); + + let value: serde_json::Value = serde_json::from_str(&stdout) + .unwrap_or_else(|e| panic!("invalid JSON ({format}): {e}\n{stdout}")); + + assert!( + value + .get("error") + .and_then(|error| error.get("message")) + .and_then(serde_json::Value::as_str) + .is_some(), + "expected an `error.message` string ({format}), got: {stdout}" + ); + } +} + fn add_network(sandbox: &TestEnv, name: &str) { sandbox .new_assert_cmd("network") diff --git a/cmd/soroban-cli/src/commands/network/health.rs b/cmd/soroban-cli/src/commands/network/health.rs index 932bc0efa0..bf11752485 100644 --- a/cmd/soroban-cli/src/commands/network/health.rs +++ b/cmd/soroban-cli/src/commands/network/health.rs @@ -9,6 +9,8 @@ pub enum Error { #[error(transparent)] Network(#[from] network::Error), #[error(transparent)] + Rpc(#[from] crate::rpc::Error), + #[error(transparent)] Serde(#[from] serde_json::Error), } @@ -46,28 +48,28 @@ pub struct Cmd { impl Cmd { pub async fn run(&self, global_args: &global::Args) -> Result<(), Error> { let output = Output::new(self.output.into(), global_args.quiet); - let result = self.config.get_network()?.rpc_client()?.get_health().await; - match result { - Ok(resp) => { - output.readable(|print| { - if resp.status.eq_ignore_ascii_case("healthy") { - print.checkln("Healthy"); - } else { - print.warnln(format!("Status: {}", resp.status)); - } - print.infoln(format!("Latest ledger: {}", resp.latest_ledger)); - }); - output.json_value(&resp)?; - } + // On failure, surface "Unhealthy" for humans then propagate the error so + // the process exits non-zero and the top-level handler renders it (as the + // structured JSON envelope in JSON mode). A reachable-but-unhealthy node + // instead returns `Ok` with a status, handled below. + let resp = match self.config.get_network()?.rpc_client()?.get_health().await { + Ok(resp) => resp, Err(err) => { - output.readable(|print| { - print.errorln("Unhealthy"); - print.errorln(format!("failed to fetch network health: {err}")); - }); - output.json_value(&crate::output::error_json(&err))?; + output.readable(|print| print.errorln("Unhealthy")); + return Err(err.into()); } - } + }; + + output.readable(|print| { + if resp.status.eq_ignore_ascii_case("healthy") { + print.checkln("Healthy"); + } else { + print.warnln(format!("Status: {}", resp.status)); + } + print.infoln(format!("Latest ledger: {}", resp.latest_ledger)); + }); + output.json_value(&resp)?; Ok(()) } diff --git a/cmd/soroban-cli/src/output.rs b/cmd/soroban-cli/src/output.rs index 4da0131d38..62aeff4254 100644 --- a/cmd/soroban-cli/src/output.rs +++ b/cmd/soroban-cli/src/output.rs @@ -9,8 +9,10 @@ //! * [`Output::json`] / [`Output::json_value`] run only in JSON mode and write //! the final machine-readable result to stdout. //! -//! Output is never buffered: each call writes immediately, so long-running -//! operations stream progress to the terminal instead of holding it silently. +//! `Output` adds no buffering of its own: each call writes straight to +//! stdout/stderr as it happens rather than accumulating a buffer to flush at the +//! end, so long-running operations don't hold their progress back. (The OS may +//! still buffer a redirected or piped stream, as usual.) use serde::Serialize; From 52f3962d146566bf09eb15cfeea6bfd37f93aeaf Mon Sep 17 00:00:00 2001 From: Nando Vieira Date: Tue, 30 Jun 2026 13:49:25 -0700 Subject: [PATCH 3/4] Cover structured JSON error from RPC error body. --- .../soroban-test/tests/it/rpc_provider.rs | 36 ++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/cmd/crates/soroban-test/tests/it/rpc_provider.rs b/cmd/crates/soroban-test/tests/it/rpc_provider.rs index 016b7b7c45..32cda5293a 100644 --- a/cmd/crates/soroban-test/tests/it/rpc_provider.rs +++ b/cmd/crates/soroban-test/tests/it/rpc_provider.rs @@ -1,7 +1,41 @@ use httpmock::{prelude::*, Mock}; use serde_json::json; use soroban_rpc::{GetEventsResponse, GetNetworkResponse}; -use soroban_test::{TestEnv, LOCAL_NETWORK_PASSPHRASE}; +use soroban_test::{AssertExt, TestEnv, LOCAL_NETWORK_PASSPHRASE}; + +// When the RPC responds with a JSON-RPC error body, `--output json` must surface +// the structured `{ code, message }` (not just the `{ message }` fallback). This +// exercises the `ErrorObjectOwned` downcast end-to-end, guarding the +// `jsonrpsee-types` version pin: if jsonrpsee drifts to two versions in the tree +// the downcast would silently degrade to the fallback and `error.code` would be +// absent, failing this test. +#[test] +fn json_error_envelope_includes_structured_rpc_code() { + let server = MockServer::start(); + let mock = server.mock(|when, then| { + when.method(POST).path("/"); + then.status(200).json_body(json!({ + "jsonrpc": "2.0", + "id": 0, + "error": { "code": -32603, "message": "DB is empty" } + })); + }); + + let sandbox = TestEnv::with_rpc_provider(&server.url(""), vec![]); + let stdout = sandbox + .new_assert_cmd("network") + .arg("health") + .arg("--output=json") + .assert() + .failure() + .stdout_as_str(); + + mock.assert(); + + let value: serde_json::Value = serde_json::from_str(&stdout).unwrap(); + assert_eq!(value["error"]["code"], json!(-32603)); + assert_eq!(value["error"]["message"], json!("DB is empty")); +} #[tokio::test] async fn test_use_rpc_provider_with_auth_header() { From 32652e7ee627ea0475b7eea3e1284da7ddea958a Mon Sep 17 00:00:00 2001 From: Nando Vieira Date: Mon, 6 Jul 2026 11:16:11 -0300 Subject: [PATCH 4/4] Scope JSON error output to migrated commands. --- cmd/crates/soroban-test/tests/it/config.rs | 57 ++++++++++++++++++++++ cmd/soroban-cli/src/cli.rs | 56 +++++++++------------ 2 files changed, 80 insertions(+), 33 deletions(-) diff --git a/cmd/crates/soroban-test/tests/it/config.rs b/cmd/crates/soroban-test/tests/it/config.rs index f31a7982b1..f004a25136 100644 --- a/cmd/crates/soroban-test/tests/it/config.rs +++ b/cmd/crates/soroban-test/tests/it/config.rs @@ -86,6 +86,63 @@ fn json_output_renders_structured_errors_and_exits_non_zero() { } } +// The JSON error envelope is scoped to commands migrated to the `Output` +// system. A command that is *not* migrated (here `ledger latest`) must keep its +// previous failure behavior even when invoked with `--output json`: no envelope +// on stdout, the human `error:` line on stderr, and a non-zero exit. This guards +// against silently changing the failure contract of unmigrated commands. +#[test] +fn non_migrated_command_keeps_human_error_line_in_json_mode() { + let sandbox = TestEnv::default(); + + sandbox + .new_assert_cmd("ledger") + .arg("latest") + .args([ + "--rpc-url=http://127.0.0.1:1", + "--network-passphrase", + LOCAL_NETWORK_PASSPHRASE, + "--output=json", + ]) + .assert() + .failure() + .stdout("") + .stderr(predicate::str::contains("error:")); +} + +// `network settings` defaults its `--output` to json, so a failure with no +// explicit flag must still render the structured JSON envelope on stdout (and +// exit non-zero). This is resolved by reading the parsed command's own output +// value rather than sniffing raw args, which cannot see per-command defaults. +#[test] +fn migrated_command_default_json_renders_structured_error() { + let sandbox = TestEnv::default(); + + let stdout = sandbox + .new_assert_cmd("network") + .arg("settings") + .args([ + "--rpc-url=http://127.0.0.1:1", + "--network-passphrase", + LOCAL_NETWORK_PASSPHRASE, + ]) + .assert() + .failure() + .stdout_as_str(); + + let value: serde_json::Value = + serde_json::from_str(&stdout).unwrap_or_else(|e| panic!("invalid JSON: {e}\n{stdout}")); + + assert!( + value + .get("error") + .and_then(|error| error.get("message")) + .and_then(serde_json::Value::as_str) + .is_some(), + "expected an `error.message` string, got: {stdout}" + ); +} + fn add_network(sandbox: &TestEnv, name: &str) { sandbox .new_assert_cmd("network") diff --git a/cmd/soroban-cli/src/cli.rs b/cmd/soroban-cli/src/cli.rs index 2649b6ab45..fa7d4989d3 100644 --- a/cmd/soroban-cli/src/cli.rs +++ b/cmd/soroban-cli/src/cli.rs @@ -94,11 +94,11 @@ pub async fn main() { println!("{help}"); std::process::exit(1); } - // When JSON output is requested, render the error as JSON on stdout so - // machine consumers get parseable output instead of the human line. The - // process still exits non-zero: this is a failure, the JSON body is just - // a parseable representation of it. - if let Some(format) = output_format_from_raw_args() { + // When a command migrated to the `Output` system requested JSON, render + // the error as JSON on stdout so machine consumers get parseable output + // instead of the human line. The process still exits non-zero: this is a + // failure, the JSON body is just a parseable representation of it. + if let Some(format) = json_error_format(&root.cmd) { let output = crate::output::Output::new(format, root.global_args.quiet); let _ = output.json_value(&crate::output::error_json(&e)); std::process::exit(1); @@ -124,35 +124,25 @@ fn set_env_from_config() { set_env_value_from_config("STELLAR_INCLUSION_FEE", config.defaults.inclusion_fee); } -// Detect a JSON `--output` value from the raw args so the top-level error -// handler can render errors as JSON. `--output` is per-command (not global), so -// the parsed args are unavailable once an error propagates this far; we mirror -// `config_dir_from_raw_args` and sniff the raw args instead. Returns `None` for -// non-JSON formats (text, xdr, pretty, …), which are rendered as the human line. -fn output_format_from_raw_args() -> Option { - use std::ffi::OsStr; - let mut iter = std::env::args_os().peekable(); - let format_for = |value: &str| match value { - "json" => Some(crate::output::Format::Json), - "json-formatted" => Some(crate::output::Format::JsonFormatted), - _ => None, +// Resolve the effective JSON output `Format` for the top-level error handler +// from the *parsed* command. Only commands migrated to the `Output` system +// participate: for those, we read their own `--output` field (honoring the +// per-command default), so a JSON failure renders as a structured envelope on +// stdout. Non-migrated commands return `None` and keep the human error line — +// this avoids silently changing their failure contract (and, for streaming +// commands like `events`, avoids splicing an error object into their stdout). +fn json_error_format(cmd: &commands::Cmd) -> Option { + use crate::commands::network; + use crate::output::Format; + + let format: Format = match cmd { + commands::Cmd::Network(network::Cmd::Health(cmd)) => cmd.output.into(), + commands::Cmd::Network(network::Cmd::Info(cmd)) => cmd.output.into(), + commands::Cmd::Network(network::Cmd::Settings(cmd)) => cmd.output.into(), + _ => return None, }; - while let Some(arg) = iter.next() { - if arg == OsStr::new("--") { - break; - } - if arg == OsStr::new("--output") { - return iter - .next() - .as_deref() - .and_then(OsStr::to_str) - .and_then(format_for); - } - if let Some(val) = arg.to_str().and_then(|s| s.strip_prefix("--output=")) { - return format_for(val); - } - } - None + + matches!(format, Format::Json | Format::JsonFormatted).then_some(format) } fn config_dir_from_raw_args() -> Option {