From 51db6d1cf84e44c154052c1ad9364b64828d9cbf Mon Sep 17 00:00:00 2001 From: Fritz Rehde Date: Sun, 21 Jan 2024 23:01:26 +0100 Subject: [PATCH] feat: improve error message on subcommand failure (#94) --- examples/file-manager.toml | 13 +- src/config/keybindings/mod.rs | 2 +- src/config/keybindings/operations/mod.rs | 2 +- .../keybindings/operations/operation.rs | 156 ++++++++++++------ src/config/mod.rs | 7 +- src/ui/mod.rs | 5 +- src/ui/state/env_variables/mod.rs | 48 +----- src/ui/state/mod.rs | 11 +- src/utils/command.rs | 138 ++++++---------- 9 files changed, 172 insertions(+), 210 deletions(-) diff --git a/examples/file-manager.toml b/examples/file-manager.toml index 09a58ce..b2d766d 100644 --- a/examples/file-manager.toml +++ b/examples/file-manager.toml @@ -20,26 +20,29 @@ interval = 3 # Delete (multiple) files "d" = [ - '''exec -- echo "$lines" | xargs -I {} rm "$pwd/{}"''', + 'exec -- echo "$lines" | xargs -I {} rm "$pwd/{}"', "reload" ] # Open file (blocking) -"o" = [ '''exec -- echo "$lines" | xargs -I {} xdg-open "$pwd/{}"''' ] +"o" = [ 'exec -- echo "$lines" | xargs -I {} xdg-open "$pwd/{}"' ] # Open file (non-blocking in background) -"O" = [ '''exec & -- echo "$lines" | xargs -I {} xdg-open "$pwd/{}"''' ] +"O" = [ 'exec & -- echo "$lines" | xargs -I {} xdg-open "$pwd/{}"' ] + +# Edit text file in TUI editor +"e" = [ 'exec tui -- echo "$line" | xargs -I {} $EDITOR "$pwd/{}"' ] # Traverse out of directories "h" = [ # Set $pwd to the parent dir of current dir - '''set-env pwd -- printf "$(dirname "$pwd")"''', + 'set-env pwd -- printf "$(dirname "$pwd")"', "reload" ] # Traverse into directories "l" = [ # Only update $pwd if it is a directory - '''set-env pwd -- new_pwd="$pwd/$line"; [ -d "$new_pwd" ] && pwd="$new_pwd"; printf "$pwd"''', + 'set-env pwd -- new_pwd="$pwd/$line"; [ -d "$new_pwd" ] && pwd="$new_pwd"; printf "$pwd"', "reload" ] diff --git a/src/config/keybindings/mod.rs b/src/config/keybindings/mod.rs index 4353af5..1e315f0 100644 --- a/src/config/keybindings/mod.rs +++ b/src/config/keybindings/mod.rs @@ -14,7 +14,7 @@ use tokio::sync::Mutex; use crate::ui::EnvVariables; pub use self::key::{KeyCode, KeyEvent, KeyModifier}; -pub use self::operations::{Operation, OperationParsed, Operations, OperationsParsed}; +pub use self::operations::{OperationExecutable, OperationParsed, Operations, OperationsParsed}; pub struct Keybindings(HashMap); diff --git a/src/config/keybindings/operations/mod.rs b/src/config/keybindings/operations/mod.rs index b971dae..2cec1d5 100644 --- a/src/config/keybindings/operations/mod.rs +++ b/src/config/keybindings/operations/mod.rs @@ -8,7 +8,7 @@ use tokio::sync::Mutex; use crate::ui::EnvVariables; -pub use self::operation::{Operation, OperationParsed}; +pub use self::operation::{Operation, OperationExecutable, OperationParsed}; #[derive(IntoIterator, From)] pub struct Operations(#[into_iterator(ref)] Vec); diff --git a/src/config/keybindings/operations/operation.rs b/src/config/keybindings/operations/operation.rs index 66b86d9..d990f62 100644 --- a/src/config/keybindings/operations/operation.rs +++ b/src/config/keybindings/operations/operation.rs @@ -1,4 +1,4 @@ -use anyhow::Result; +use anyhow::{Context, Result}; use parse_display::{Display, FromStr}; use std::str; use std::sync::Arc; @@ -6,12 +6,23 @@ use strum::{EnumIter, EnumMessage}; use tokio::sync::mpsc::{self, Sender}; use tokio::sync::Mutex; +use crate::config::KeyEvent; use crate::ui::{EnvVariable, EnvVariables, Event, RequestedAction, State}; use crate::utils::command::{ Blocking, CommandBuilder, InheritedIO, NonBlocking, NonInterruptible, WithEnv, WithOutput, }; -// TODO: use some rust pattern (with types) instead of hardcoded Operation{,Parsed} variants +#[derive(Display)] +#[display("{parsed}")] +pub struct Operation { + /// Used for executing an operation. + pub executable: OperationExecutable, + + /// Used for displaying an operation with `fmt::Display`. + parsed: OperationParsed, +} + +// TODO: use some rust pattern (with types) instead of hardcoded OperationParsed variant /// The version of Operation used for parsing and displaying. The reason we /// can't parse directly into Operation is because any operations that execute @@ -94,7 +105,7 @@ pub enum OperationParsed { HelpToggle, } -pub enum Operation { +pub enum OperationExecutable { Exit, Reload, HelpShow, @@ -132,46 +143,61 @@ pub enum SelectOperation { } impl Operation { + /// Execute the operation given the current `State` of the program. Perform + /// any additional async communication with the main event loop through the + /// `event_tx` channel. Also use the `key_event` that triggered this + /// operation for printing helpful error messages. pub async fn execute( &self, state: &mut State, event_tx: &Sender, + key_event: &KeyEvent, ) -> Result { - match self { - Self::MoveCursor(MoveCursor::Down(steps)) => state.move_down(*steps), - Self::MoveCursor(MoveCursor::Up(steps)) => state.move_up(*steps), - Self::MoveCursor(MoveCursor::First) => state.move_to_first(), - Self::MoveCursor(MoveCursor::Last) => state.move_to_last(), - Self::SelectLine(SelectOperation::Select) => state.select(), - Self::SelectLine(SelectOperation::Unselect) => state.unselect(), - Self::SelectLine(SelectOperation::ToggleSelection) => state.toggle_selection(), - Self::SelectLine(SelectOperation::SelectAll) => state.select_all(), - Self::SelectLine(SelectOperation::UnselectAll) => state.unselect_all(), - Self::HelpShow => state.show_help_menu().await, - Self::HelpHide => state.hide_help_menu(), - Self::HelpToggle => state.toggle_help_menu().await, - Self::Reload => return Ok(RequestedAction::ReloadWatchedCommand), - Self::Exit => return Ok(RequestedAction::Exit), - Self::ExecuteNonBlocking(non_blocking_cmd) => { + match &self.executable { + OperationExecutable::MoveCursor(MoveCursor::Down(steps)) => state.move_down(*steps), + OperationExecutable::MoveCursor(MoveCursor::Up(steps)) => state.move_up(*steps), + OperationExecutable::MoveCursor(MoveCursor::First) => state.move_to_first(), + OperationExecutable::MoveCursor(MoveCursor::Last) => state.move_to_last(), + OperationExecutable::SelectLine(SelectOperation::Select) => state.select(), + OperationExecutable::SelectLine(SelectOperation::Unselect) => state.unselect(), + OperationExecutable::SelectLine(SelectOperation::ToggleSelection) => { + state.toggle_selection() + } + OperationExecutable::SelectLine(SelectOperation::SelectAll) => state.select_all(), + OperationExecutable::SelectLine(SelectOperation::UnselectAll) => state.unselect_all(), + OperationExecutable::HelpShow => state.show_help_menu().await, + OperationExecutable::HelpHide => state.hide_help_menu(), + OperationExecutable::HelpToggle => state.toggle_help_menu().await, + OperationExecutable::Reload => return Ok(RequestedAction::ReloadWatchedCommand), + OperationExecutable::Exit => return Ok(RequestedAction::Exit), + OperationExecutable::ExecuteNonBlocking(non_blocking_cmd) => { state.add_cursor_and_selected_lines_to_env().await; non_blocking_cmd.execute().await?; state.remove_cursor_and_selected_lines_from_env().await; } - Self::ExecuteBlocking(blocking_cmd) => { + OperationExecutable::ExecuteBlocking(blocking_cmd) => { state.add_cursor_and_selected_lines_to_env().await; let blocking_cmd = Arc::clone(blocking_cmd); let event_tx = event_tx.clone(); + // TODO: inefficient: creating Strings that are only used in the (rare) error-case + let (op_to_string, key_to_string) = (self.to_string(), key_event.to_string()); tokio::spawn(async move { - let result = blocking_cmd.execute().await; + let result = blocking_cmd.execute().await.with_context(|| { + format!("Execution of blocking subcommand \"{}\", triggered by key event \"{}\", failed", op_to_string, key_to_string) + }); // Ignore whether the sender has closed channel. let _ = event_tx.send(Event::SubcommandCompleted(result)).await; }); + // Don't call state.remove_cursor_and_selected_lines_from_env() + // here, because it would race with the spawned Tokio task. It + // will be called once this subcommand completes. + return Ok(RequestedAction::ExecutingBlockingSubcommand); } - Self::ExecuteTUI(tui_cmd) => { + OperationExecutable::ExecuteTUI(tui_cmd) => { state.add_cursor_and_selected_lines_to_env().await; // Create channels for waiting until TUI has actually been hidden. @@ -179,19 +205,27 @@ impl Operation { let tui_cmd = Arc::clone(tui_cmd); let event_tx = event_tx.clone(); + // TODO: inefficient: creating Strings that are only used in the (rare) error-case + let (op_to_string, key_to_string) = (self.to_string(), key_event.to_string()); tokio::spawn(async move { // Wait until TUI has actually been hidden. let _ = tui_hidden_rx.recv().await; - let result = tui_cmd.execute().await; + let result = tui_cmd.execute().await.with_context(|| { + format!("Execution of TUI subcommand \"{}\", triggered by key event \"{}\", failed", op_to_string, key_to_string) + }); // Ignore whether the sender has closed channel. let _ = event_tx.send(Event::TUISubcommandCompleted(result)).await; }); + // Don't call state.remove_cursor_and_selected_lines_from_env() + // here, because it would race with the spawned Tokio task. It + // will be called once this subcommand completes. + return Ok(RequestedAction::ExecutingTUISubcommand(tui_hidden_tx)); } - Self::SetEnv(env_variable, blocking_cmd) => { + OperationExecutable::SetEnv(env_variable, blocking_cmd) => { state.add_cursor_and_selected_lines_to_env().await; let blocking_cmd = blocking_cmd.clone(); @@ -212,44 +246,54 @@ impl Operation { return Ok(RequestedAction::ExecutingBlockingSubcommandForEnv); } - Self::UnsetEnv(env) => state.unset_env(env).await, - Self::ReadIntoEnv(env) => state.read_into_env(env).await, + OperationExecutable::UnsetEnv(env) => state.unset_env(env).await, + OperationExecutable::ReadIntoEnv(env) => state.read_into_env(env).await, }; Ok(RequestedAction::Continue) } - /// Convert the parsed form into the normal, runtime Operation form. The + /// Convert the parsed form into the normal, runtime executable form. The /// `env_variables` is required so it can be passed to the `SetEnv` command. pub fn from_parsed(parsed: OperationParsed, env_variables: &Arc>) -> Self { - match parsed { - OperationParsed::Exit => Self::Exit, - OperationParsed::Reload => Self::Reload, - OperationParsed::MoveCursorUp(n) => Self::MoveCursor(MoveCursor::Up(n)), - OperationParsed::MoveCursorDown(n) => Self::MoveCursor(MoveCursor::Down(n)), - OperationParsed::MoveCursorFirst => Self::MoveCursor(MoveCursor::First), - OperationParsed::MoveCursorLast => Self::MoveCursor(MoveCursor::Last), - OperationParsed::SelectLine => Self::SelectLine(SelectOperation::Select), - OperationParsed::UnselectLine => Self::SelectLine(SelectOperation::Unselect), + let operation_executable = match parsed.clone() { + OperationParsed::Exit => OperationExecutable::Exit, + OperationParsed::Reload => OperationExecutable::Reload, + OperationParsed::MoveCursorUp(n) => OperationExecutable::MoveCursor(MoveCursor::Up(n)), + OperationParsed::MoveCursorDown(n) => { + OperationExecutable::MoveCursor(MoveCursor::Down(n)) + } + OperationParsed::MoveCursorFirst => OperationExecutable::MoveCursor(MoveCursor::First), + OperationParsed::MoveCursorLast => OperationExecutable::MoveCursor(MoveCursor::Last), + OperationParsed::SelectLine => OperationExecutable::SelectLine(SelectOperation::Select), + OperationParsed::UnselectLine => { + OperationExecutable::SelectLine(SelectOperation::Unselect) + } OperationParsed::ToggleLineSelection => { - Self::SelectLine(SelectOperation::ToggleSelection) + OperationExecutable::SelectLine(SelectOperation::ToggleSelection) } - OperationParsed::SelectAllLines => Self::SelectLine(SelectOperation::SelectAll), - OperationParsed::UnselectAllLines => Self::SelectLine(SelectOperation::UnselectAll), - OperationParsed::ExecuteBlocking(cmd) => Self::ExecuteBlocking(Arc::new( - CommandBuilder::new(cmd) - .blocking() - .with_env(env_variables.clone()), - )), - OperationParsed::ExecuteNonBlocking(cmd) => Self::ExecuteNonBlocking(Arc::new( - CommandBuilder::new(cmd).with_env(env_variables.clone()), - )), - OperationParsed::ExecuteTUI(cmd) => Self::ExecuteTUI(Arc::new( + OperationParsed::SelectAllLines => { + OperationExecutable::SelectLine(SelectOperation::SelectAll) + } + OperationParsed::UnselectAllLines => { + OperationExecutable::SelectLine(SelectOperation::UnselectAll) + } + OperationParsed::ExecuteBlocking(cmd) => { + OperationExecutable::ExecuteBlocking(Arc::new( + CommandBuilder::new(cmd) + .blocking() + .with_env(env_variables.clone()), + )) + } + OperationParsed::ExecuteNonBlocking(cmd) => OperationExecutable::ExecuteNonBlocking( + Arc::new(CommandBuilder::new(cmd).with_env(env_variables.clone())), + ), + OperationParsed::ExecuteTUI(cmd) => OperationExecutable::ExecuteTUI(Arc::new( CommandBuilder::new(cmd) .blocking() .inherited_io() .with_env(env_variables.clone()), )), - OperationParsed::SetEnv(env_var, cmd) => Self::SetEnv( + OperationParsed::SetEnv(env_var, cmd) => OperationExecutable::SetEnv( env_var, Arc::new( CommandBuilder::new(cmd) @@ -258,11 +302,15 @@ impl Operation { .with_env(env_variables.clone()), ), ), - OperationParsed::UnsetEnv(x) => Self::UnsetEnv(x), - OperationParsed::ReadIntoEnv(x) => Self::ReadIntoEnv(x), - OperationParsed::HelpShow => Self::HelpShow, - OperationParsed::HelpHide => Self::HelpHide, - OperationParsed::HelpToggle => Self::HelpToggle, + OperationParsed::UnsetEnv(x) => OperationExecutable::UnsetEnv(x), + OperationParsed::ReadIntoEnv(x) => OperationExecutable::ReadIntoEnv(x), + OperationParsed::HelpShow => OperationExecutable::HelpShow, + OperationParsed::HelpHide => OperationExecutable::HelpHide, + OperationParsed::HelpToggle => OperationExecutable::HelpToggle, + }; + Self { + executable: operation_executable, + parsed, } } } diff --git a/src/config/mod.rs b/src/config/mod.rs index 64c7c38..a2234b4 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -25,16 +25,13 @@ use crate::config::keybindings::{KeyCode, KeyModifier}; use crate::config::style::PrettyColor; use crate::utils::possible_enum_values::PossibleEnumValues; +use self::fields::{FieldSelections, FieldSeparator}; use self::keybindings::{KeybindingsParsed, StringKeybindings}; use self::style::{Boldness, Color, Style}; -use self::{ - fields::{FieldSelections, FieldSeparator}, - keybindings::ClapKeybindings, -}; pub use self::fields::{Fields, TableFormatter}; pub use self::keybindings::{ - KeyEvent, Keybindings, Operation, OperationParsed, Operations, OperationsParsed, + KeyEvent, Keybindings, OperationExecutable, OperationParsed, Operations, OperationsParsed, }; pub use self::style::Styles; diff --git a/src/ui/mod.rs b/src/ui/mod.rs index 0e43ba2..bcdc7fc 100644 --- a/src/ui/mod.rs +++ b/src/ui/mod.rs @@ -379,7 +379,10 @@ impl UI { ) -> Result { if let Some(ops) = self.keybindings.get_operations(&key) { for (idx, op) in ops.into_iter().enumerate().skip(starting_index) { - match op.execute(&mut self.state, &self.channels.event_tx).await? { + match op + .execute(&mut self.state, &self.channels.event_tx, &key) + .await? + { RequestedAction::Exit => return Ok(ControlFlow::Exit), RequestedAction::ReloadWatchedCommand => { // Send the command execution an interrupt signal diff --git a/src/ui/state/env_variables/mod.rs b/src/ui/state/env_variables/mod.rs index 2e6fc25..663785e 100644 --- a/src/ui/state/env_variables/mod.rs +++ b/src/ui/state/env_variables/mod.rs @@ -1,15 +1,10 @@ mod env_variable; -use anyhow::{bail, Result}; +use anyhow::Result; use itertools::Itertools; use std::{collections::HashMap, fmt, io::Write}; use tabwriter::TabWriter; -use crate::{ - config::{Operation, OperationParsed, Operations, OperationsParsed}, - utils::command::CommandBuilder, -}; - pub use self::env_variable::EnvVariable; #[derive(Default, Debug)] @@ -21,47 +16,6 @@ impl EnvVariables { Self(HashMap::new()) } - /// Receive parsed operations, but only execute the "set-env" operations. - pub async fn generate_initial(value: OperationsParsed) -> Result { - // TODO: consider trying to use async iterators to do this in one iterator pass (instead of the mut hashmap) once stable - let mut map = HashMap::new(); - for op in value.into_iter() { - match op { - OperationParsed::SetEnv(env_variable, command) => { - let output = CommandBuilder::new(command) - .blocking() - .with_output() - .execute() - .await?; - map.insert(env_variable, output); - } - other_op => bail!("Invalid operation for env variable setup (only \"set-env\" operations allowed here): {}", other_op), - } - } - Ok(Self(map)) - } - - /// Receive parsed operations, but only execute the "set-env" operations. - pub async fn generate_initial2(&mut self, operations: Operations) -> Result<()> { - // TODO: consider trying to use async iterators to do this in one iterator pass (instead of the mut hashmap) once stable - for op in operations.into_iter() { - match op { - Operation::SetEnv(env_variable, blocking_cmd) => { - log::info!("executing cmd"); - - let cmd_output = blocking_cmd.execute().await?; - - log::info!("finished executing cmd"); - - self.0.insert(env_variable, cmd_output); - } - // other_op => bail!("Invalid operation for env variable setup (only \"set-env\" operations allowed here): {}", other_op), - _ => bail!("Only \"set-env\" operations allowed here."), - } - } - Ok(()) - } - pub fn merge_new_envs(&mut self, env_variables: Self) { self.0.extend(env_variables.0); } diff --git a/src/ui/state/mod.rs b/src/ui/state/mod.rs index e1377ca..2eefd21 100644 --- a/src/ui/state/mod.rs +++ b/src/ui/state/mod.rs @@ -8,7 +8,7 @@ use ratatui::Frame; use std::sync::Arc; use tokio::sync::Mutex; -use crate::config::{Fields, Operation, Operations, OperationsParsed, Styles}; +use crate::config::{Fields, OperationExecutable, Operations, OperationsParsed, Styles}; use self::{ help_menu::HelpMenu, @@ -178,14 +178,15 @@ impl State { // TODO: consider trying to use async iterators to do this in one iterator pass (instead of the mut hashmap) once stable for (i, op) in initial_env_ops.into_iter().enumerate() { - match (i, op) { - (_, Operation::SetEnv(env_var, blocking_cmd)) => { + match (i, op.executable) { + (_, OperationExecutable::SetEnv(env_var, blocking_cmd)) => { let cmd_output = blocking_cmd.execute().await?; self.set_env(env_var, cmd_output).await; } (op_index, _) => { - // Delay retrieval of printable `OperationParsed` until error case - // => improve performance for correct use of "set-env". + // Delay retrieval of printable `OperationParsed` until + // this error case => improve performance for correct use + // of "set-env". let other_op = initial_env_ops_parsed .into_iter() .nth(op_index) diff --git a/src/utils/command.rs b/src/utils/command.rs index 47802a4..f6bdf4e 100644 --- a/src/utils/command.rs +++ b/src/utils/command.rs @@ -29,40 +29,31 @@ pub struct CommandBuilder Blocking behavior - /// The command is executed non-blockingly. pub struct NonBlocking; - /// The command is executed blockingly. pub struct Blocking; // Environment variables - /// The global `EnvVariables` are **not** made available in the child process. pub struct WithoutEnv; - /// The global `EnvVariables` are made available in the sub-process. pub struct WithEnv { env_variables: Arc>, } // Ouput/Input - /// The output of the executed command is **not** captured. pub struct NoOutput; - /// The output of the executed command is captured. pub struct WithOutput; - /// The output of the executed command is **not** captured, but the child /// process inherits stdin and stdout of the parent process. pub struct InheritedIO; // Interruptibility - /// The execution of the child process is **non**-interruptible. pub struct NonInterruptible; - /// The execution of the child process can be interrupted by sending an /// `InterruptSignal` to the `interrupt_rx` channel receiver. pub struct Interruptible { @@ -80,7 +71,6 @@ pub struct Interruptible { impl CommandBuilder { pub fn new(command: String) -> Self { CommandBuilder { - // TODO: i think we don't even need command anymore, just the tokiocommand command, blocking: NonBlocking, output: NoOutput, @@ -202,6 +192,7 @@ impl CommandBuilder { let mut command = TokioCommand::new(sh[0]); command.args(&sh[1..]); + command.stdin(&self.tokio_command.stdin); command.stdout(&self.tokio_command.stdout); command.stderr(&self.tokio_command.stderr); @@ -216,8 +207,9 @@ impl CommandBuilder { let sh = ["sh", "-c", &self.command]; let mut command = TokioCommand::new(sh[0]); - command.args(&sh[1..]); + + command.stdin(&self.tokio_command.stdin); command.stdout(&self.tokio_command.stdout); command.stderr(&self.tokio_command.stderr); @@ -234,13 +226,6 @@ impl CommandBuilder { impl CommandBuilder { pub async fn execute(&self) -> Result<()> { self.create_shell_command().await.spawn()?; - - // create_shell_command(&self.command) - // // We only need stderr in case of an error, stdout can be ignored - // .stdout(Stdio::null()) - // .stderr(Stdio::piped()) - // .spawn()?; - Ok(()) } } @@ -248,14 +233,6 @@ impl CommandBuilder { impl CommandBuilder { pub async fn execute(&self) -> Result<()> { self.create_shell_command().await.spawn()?; - - // let env_variables = self.env.env_variables.lock().await.deref().into(); - // create_shell_command_with_env(&self.command, env_variables) - // // We only need stderr in case of an error, stdout can be ignored - // .stdout(Stdio::null()) - // .stderr(Stdio::piped()) - // .spawn()?; - Ok(()) } } @@ -264,15 +241,9 @@ impl CommandBuilder { pub async fn execute(&self) -> Result<()> { let mut child = self.create_shell_command().await.spawn()?; - // let env_variables = self.env.env_variables.lock().await.deref().into(); - // let mut child = create_shell_command_with_env(&self.command, env_variables) - // // We only need stderr in case of an error, stdout can be ignored - // .stdout(Stdio::null()) - // .stderr(Stdio::piped()) - // .spawn()?; - let exit_status = child.wait().await?; - assert_child_exited_successfully(exit_status, &mut child.stderr).await?; + self.assert_child_exited_successfully(exit_status, &mut child.stderr) + .await?; Ok(()) } @@ -283,7 +254,8 @@ impl CommandBuilder { let mut child = self.create_shell_command().await.spawn()?; let exit_status = child.wait().await?; - assert_child_exited_successfully(exit_status, &mut child.stderr).await?; + self.assert_child_exited_successfully(exit_status, &mut child.stderr) + .await?; Ok(()) } @@ -293,16 +265,11 @@ impl CommandBuilder { pub async fn execute(&self) -> Result { let mut child = self.create_shell_command().await.spawn()?; - // let mut child = create_shell_command(&self.command) - // // Keep both stdout and stderr - // .stdout(Stdio::piped()) - // .stderr(Stdio::piped()) - // .spawn()?; - let exit_status = child.wait().await?; - assert_child_exited_successfully(exit_status, &mut child.stderr).await?; + self.assert_child_exited_successfully(exit_status, &mut child.stderr) + .await?; - // Read stdout + // Read stdout. let mut stdout = String::new(); child.stdout.unwrap().read_to_string(&mut stdout).await?; @@ -314,17 +281,11 @@ impl CommandBuilder { pub async fn execute(&self) -> Result { let mut child = self.create_shell_command().await.spawn()?; - // let env_variables = self.env.env_variables.lock().await.deref().into(); - // let mut child = create_shell_command_with_env(&self.command, env_variables) - // // Keep both stdout and stderr - // .stdout(Stdio::piped()) - // .stderr(Stdio::piped()) - // .spawn()?; - let exit_status = child.wait().await?; - assert_child_exited_successfully(exit_status, &mut child.stderr).await?; + self.assert_child_exited_successfully(exit_status, &mut child.stderr) + .await?; - // Read stdout + // Read stdout. let mut stdout = String::new(); child.stdout.unwrap().read_to_string(&mut stdout).await?; @@ -350,23 +311,15 @@ impl CommandBuilder { pub async fn execute(&mut self) -> Result { let mut child = self.create_shell_command().await.spawn()?; - // let env_variables = self.env.env_variables.lock().await.deref().into(); - - // let mut child = create_shell_command_with_env(&self.command, env_variables) - // // Keep both stdout and stderr - // .stdout(Stdio::piped()) - // .stderr(Stdio::piped()) - // .spawn()?; - tokio::select! { _ = self.interruptible.interrupt_rx.recv() => { child.kill().await?; Ok(ExecutionResult::Interrupted) }, exit_status = child.wait() => { - assert_child_exited_successfully(exit_status?, &mut child.stderr).await?; + self.assert_child_exited_successfully(exit_status?, &mut child.stderr).await?; - // Read stdout + // Read stdout. let mut stdout = String::new(); // TODO: remove unwrap() child.stdout.unwrap().read_to_string(&mut stdout).await?; @@ -395,36 +348,39 @@ impl CommandBuilder { } } -/// Return an error in case the exit status/exit code indicates failure, and -/// include stderr in error message. -async fn assert_child_exited_successfully( - exit_status: ExitStatus, - stderr: &mut Option, -) -> Result<()> { - if !exit_status.success() { - // Read exit code - let status_code_str = match exit_status.code() { - Some(code) => Cow::Owned(code.to_string()), - None => Cow::Borrowed("unknown"), - }; - - let stderr_str = match stderr { - Some(stderr) => { - // Read stderr - let mut stderr_str = String::new(); - stderr.read_to_string(&mut stderr_str).await?; - - Cow::Owned(format!("stderr:\n{}", stderr_str)) - } - None => Cow::Borrowed("unknown stderr"), - }; - bail!( - "Process exited with status code: {} and stderr: {}", - status_code_str, - stderr_str - ); +impl CommandBuilder { + /// Return an error in case the exit status/exit code indicates failure, and + /// include stderr in error message. + async fn assert_child_exited_successfully( + &self, + exit_status: ExitStatus, + stderr: &mut Option, + ) -> Result<()> { + if !exit_status.success() { + // Read exit code. + let status_code_str = match exit_status.code() { + Some(code) => Cow::Owned(format!("status code {}", code)), + None => Cow::Borrowed("unknown status code"), + }; + + let stderr_str = match stderr { + Some(stderr) => { + // Read stderr. + let mut stderr_str = String::new(); + stderr.read_to_string(&mut stderr_str).await?; + Cow::Owned(format!("--STDERR--\n{}\n----------", stderr_str)) + } + None => Cow::Borrowed("unknown STDERR"), + }; + bail!( + "Process \"{}\" exited with {}:\n{}", + self.command, + status_code_str, + stderr_str + ); + } + Ok(()) } - Ok(()) } // TODO: update tests