From bb6b72c468ae544bbaca4fc17cb15dad995981fe Mon Sep 17 00:00:00 2001 From: Shravan Vasista Date: Tue, 21 Apr 2026 11:59:56 +0530 Subject: [PATCH 1/3] fix: update cargo build message format and improve error handling --- .../cargo-wdk/src/actions/build/build_task.rs | 26 ++++++------------- crates/cargo-wdk/src/actions/build/error.rs | 4 +-- crates/cargo-wdk/src/actions/build/tests.rs | 2 +- 3 files changed, 11 insertions(+), 21 deletions(-) diff --git a/crates/cargo-wdk/src/actions/build/build_task.rs b/crates/cargo-wdk/src/actions/build/build_task.rs index 83d741fc7..7d64b5c50 100644 --- a/crates/cargo-wdk/src/actions/build/build_task.rs +++ b/crates/cargo-wdk/src/actions/build/build_task.rs @@ -91,7 +91,7 @@ impl<'a> BuildTask<'a> { ) -> Result>, BuildTaskError> { debug!("Running cargo build"); let mut args = vec!["build".to_string()]; - args.push("--message-format=json".to_string()); + args.push("--message-format=json-render-diagnostics".to_string()); args.push("-p".to_string()); args.push(self.package_name.to_string()); if let Some(path) = self.manifest_path.to_str() { @@ -120,7 +120,10 @@ impl<'a> BuildTask<'a> { // is respected let output = self .command_exec - .run("cargo", &args, None, Some(self.working_dir))?; + .run("cargo", &args, None, Some(self.working_dir)) + .map_err(|_| { + BuildTaskError::CargoBuild("Compilation failed. See compiler errors above.".into()) + })?; debug!("cargo build done"); Ok(Message::parse_stream(std::io::Cursor::new(output.stdout))) @@ -205,7 +208,7 @@ mod tests { let verbosity = clap_verbosity_flag::Verbosity::default(); let expected_args = vec![ "build".to_string(), - "--message-format=json".to_string(), + "--message-format=json-render-diagnostics".to_string(), "-p".to_string(), "my-driver".to_string(), "--manifest-path".to_string(), @@ -292,22 +295,9 @@ mod tests { ); let err = task.run().err().expect("expected cargo failure"); - let BuildTaskError::CargoBuild(command_error) = err else { + let BuildTaskError::CargoBuild(msg) = err else { panic!("expected cargo build error"); }; - match command_error { - CommandError::CommandFailed { - command, - args, - stdout, - } => { - assert_eq!(command, "cargo"); - assert_eq!(args, vec!["build".to_string()]); - assert_eq!(stdout, "error"); - } - CommandError::IoError(_, _, err) => { - panic!("expected CommandFailed, got IoError: {err}") - } - } + assert_eq!(msg, "Compilation failed. See compiler errors above."); } } diff --git a/crates/cargo-wdk/src/actions/build/error.rs b/crates/cargo-wdk/src/actions/build/error.rs index ac8e636da..7943ec58d 100644 --- a/crates/cargo-wdk/src/actions/build/error.rs +++ b/crates/cargo-wdk/src/actions/build/error.rs @@ -48,8 +48,8 @@ pub enum BuildActionError { pub enum BuildTaskError { #[error("Empty manifest path found error")] EmptyManifestPath, - #[error("Error running cargo build command")] - CargoBuild(#[from] CommandError), + #[error("Error running cargo build command: {0}")] + CargoBuild(String), #[error(transparent)] FileIo(#[from] FileError), } diff --git a/crates/cargo-wdk/src/actions/build/tests.rs b/crates/cargo-wdk/src/actions/build/tests.rs index cb6aad715..6bb4f9973 100644 --- a/crates/cargo-wdk/src/actions/build/tests.rs +++ b/crates/cargo-wdk/src/actions/build/tests.rs @@ -1935,7 +1935,7 @@ impl TestBuildAction { .to_string(); let mut expected_cargo_build_args: Vec = vec![ "build", - "--message-format=json", + "--message-format=json-render-diagnostics", "-p", &driver_name, "--manifest-path", From 76f3e6b78034228b1992b9fe516ef8e95d9b86fd Mon Sep 17 00:00:00 2001 From: Shravan Vasista Date: Mon, 27 Apr 2026 17:02:42 +0530 Subject: [PATCH 2/3] fix: improve error handling for cargo build command and update error type Co-authored-by: Copilot --- .../cargo-wdk/src/actions/build/build_task.rs | 71 +++++++++++++++++-- crates/cargo-wdk/src/actions/build/error.rs | 4 +- 2 files changed, 67 insertions(+), 8 deletions(-) diff --git a/crates/cargo-wdk/src/actions/build/build_task.rs b/crates/cargo-wdk/src/actions/build/build_task.rs index 7d64b5c50..4ff883788 100644 --- a/crates/cargo-wdk/src/actions/build/build_task.rs +++ b/crates/cargo-wdk/src/actions/build/build_task.rs @@ -17,6 +17,7 @@ use wdk_build::CpuArchitecture; use crate::providers::exec::CommandExec; use crate::{ actions::{Profile, build::error::BuildTaskError, to_target_triple}, + providers::error::CommandError, trace, }; @@ -121,8 +122,17 @@ impl<'a> BuildTask<'a> { let output = self .command_exec .run("cargo", &args, None, Some(self.working_dir)) - .map_err(|_| { - BuildTaskError::CargoBuild("Compilation failed. See compiler errors above.".into()) + .map_err(|err| match err { + CommandError::CommandFailed { + command, + args, + stdout: _, + } => BuildTaskError::CargoBuild(CommandError::CommandFailed { + command, + args, + stdout: String::new(), // omit stdout to avoid printing potentially large output + }), + err @ CommandError::IoError(..) => BuildTaskError::CargoBuild(err), })?; debug!("cargo build done"); @@ -269,7 +279,7 @@ mod tests { } #[test] - fn run_returns_error_when_cargo_command_fails() { + fn run_returns_command_failed_error_with_empty_stdout_when_cargo_build_exits_nonzero() { let working_dir = PathBuf::from("C:/abs/driver"); let mut mock = MockCommandExec::new(); mock.expect_run().return_once(|_, _, _, _| { @@ -295,9 +305,58 @@ mod tests { ); let err = task.run().err().expect("expected cargo failure"); - let BuildTaskError::CargoBuild(msg) = err else { - panic!("expected cargo build error"); + let BuildTaskError::CargoBuild(CommandError::CommandFailed { + command, + args, + stdout, + }) = err + else { + panic!("expected CargoBuild(CommandFailed) error, got: {err:?}"); }; - assert_eq!(msg, "Compilation failed. See compiler errors above."); + assert_eq!(command, "cargo"); + assert!( + args.contains(&"build".to_string()), + "expected args to contain 'build', got: {args:?}" + ); + assert!( + stdout.is_empty(), + "expected stdout to be omitted, got: {stdout}" + ); + } + + #[test] + fn run_returns_io_error_when_cargo_build_command_invocation_fails() { + let working_dir = PathBuf::from("C:/abs/driver"); + let mut mock = MockCommandExec::new(); + mock.expect_run().return_once(|_, _, _, _| { + Err(CommandError::from_io_error( + "cargo", + &["build"], + std::io::Error::new(std::io::ErrorKind::NotFound, "program not found"), + )) + }); + + let task = BuildTask::new( + "my-driver", + &working_dir, + None, + None, + clap_verbosity_flag::Verbosity::default(), + &mock, + ); + + let err = task + .run() + .err() + .expect("expected command invocation failure"); + let BuildTaskError::CargoBuild(CommandError::IoError(command, args, io_err)) = err else { + panic!("expected CargoBuild(IoError) error, got: {err:?}"); + }; + assert_eq!(command, "cargo"); + assert!( + args.contains(&"build".to_string()), + "expected args to contain 'build', got: {args:?}" + ); + assert_eq!(io_err.kind(), std::io::ErrorKind::NotFound); } } diff --git a/crates/cargo-wdk/src/actions/build/error.rs b/crates/cargo-wdk/src/actions/build/error.rs index 7943ec58d..daa1a26e8 100644 --- a/crates/cargo-wdk/src/actions/build/error.rs +++ b/crates/cargo-wdk/src/actions/build/error.rs @@ -48,8 +48,8 @@ pub enum BuildActionError { pub enum BuildTaskError { #[error("Empty manifest path found error")] EmptyManifestPath, - #[error("Error running cargo build command: {0}")] - CargoBuild(String), + #[error("Error running cargo build command")] + CargoBuild(#[source] CommandError), #[error(transparent)] FileIo(#[from] FileError), } From 3105ea8ea3b36e5bfe165dc11c91218570c2fc95 Mon Sep 17 00:00:00 2001 From: Shravan Vasista Date: Thu, 30 Apr 2026 10:46:48 +0530 Subject: [PATCH 3/3] fix: improve error handling in `run` method, use if let instead of explicit map --- .../cargo-wdk/src/actions/build/build_task.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/crates/cargo-wdk/src/actions/build/build_task.rs b/crates/cargo-wdk/src/actions/build/build_task.rs index 4ff883788..e45b63316 100644 --- a/crates/cargo-wdk/src/actions/build/build_task.rs +++ b/crates/cargo-wdk/src/actions/build/build_task.rs @@ -122,17 +122,14 @@ impl<'a> BuildTask<'a> { let output = self .command_exec .run("cargo", &args, None, Some(self.working_dir)) - .map_err(|err| match err { - CommandError::CommandFailed { - command, - args, - stdout: _, - } => BuildTaskError::CargoBuild(CommandError::CommandFailed { - command, - args, - stdout: String::new(), // omit stdout to avoid printing potentially large output - }), - err @ CommandError::IoError(..) => BuildTaskError::CargoBuild(err), + .map_err(|mut err| { + // Drop stdout from CommandFailed so the noisy + // --message-format=json-render-diagnostics output isn't bubbled up + // in the wrapped error. + if let CommandError::CommandFailed { stdout, .. } = &mut err { + stdout.clear(); + } + BuildTaskError::CargoBuild(err) })?; debug!("cargo build done");