From 9f5ba1dadb98ca8be7721d7fe9c2ccc3278df0fb Mon Sep 17 00:00:00 2001 From: jzbor Date: Sat, 20 Jul 2024 21:10:57 +0200 Subject: [PATCH] Improving error output --- src/barkeeper.rs | 32 ++++++++++++++++++++++++-------- src/error.rs | 2 +- src/job.rs | 25 ++++++++++++++++++++++--- src/main.rs | 6 +++++- src/queue.rs | 4 ++++ 5 files changed, 56 insertions(+), 13 deletions(-) diff --git a/src/barkeeper.rs b/src/barkeeper.rs index f1ff5c7..73eb783 100644 --- a/src/barkeeper.rs +++ b/src/barkeeper.rs @@ -13,7 +13,7 @@ pub trait ThreadStateTracker: Send { fn start(&self); fn set_prefix(&mut self, prefix: String); fn clear_status(&mut self); - fn cmd_output(&mut self, job: &str, out: &str, verbose: bool); + fn cmd_output(&mut self, out: &str, verbose: bool); fn flush_cmd_output(&mut self, job: &str, verbose: bool); fn trace(&mut self, cmd: &str); } @@ -33,7 +33,7 @@ pub struct ThreadBarkeeper { } pub struct DummyBarkeeper {} -pub struct DummyThreadBarkeeper {} +pub struct DummyThreadBarkeeper { prefix: String } #[cfg(feature = "progress")] @@ -93,7 +93,7 @@ impl StateTracker for DummyBarkeeper { fn for_threads(&self, nthreads: usize) -> Vec { (0..nthreads).map(|_| { - DummyThreadBarkeeper {} + DummyThreadBarkeeper { prefix: String::new() } }).collect() } } @@ -102,18 +102,27 @@ impl ThreadStateTracker for DummyThreadBarkeeper { fn job_completed(&self, job: JobRealization, state: JobState, error: Option) { println!("{}", job_finished_msg(job, state)); if let Some(e) = error { + if let ZinnError::ChildFailed(_, lines) = &e { + for line in lines { + println!("{}: {}", self.prefix.to_owned(), line); + } + } println!("{}", e); } } fn start(&self) {} - fn set_prefix(&mut self, _prefix: String) {} + fn set_prefix(&mut self, prefix: String) { + self.prefix = prefix; + } fn clear_status(&mut self) {} - fn cmd_output(&mut self, job: &str, out: &str, _verbose: bool) { - println!("{}: {}", job, out); + fn cmd_output(&mut self, out: &str, verbose: bool) { + if verbose { + println!("{}: {}", self.prefix, out); + } } fn flush_cmd_output(&mut self, _job: &str, _verbose: bool) {} @@ -134,6 +143,12 @@ impl ThreadStateTracker for ThreadBarkeeper { fn job_completed(&self, job: JobRealization, state: JobState, error: Option) { self.bar.println(job_finished_msg(job, state)); if let Some(e) = error { + if let ZinnError::ChildFailed(_, lines) = &e { + for line in lines { + let prefix = self.bar.prefix(); + self.bar.println(prefix + ": " + line); + } + } self.bar.println(e.to_string()); } self.main_bar.inc(1) @@ -147,12 +162,13 @@ impl ThreadStateTracker for ThreadBarkeeper { self.bar.set_message(""); } - fn cmd_output(&mut self, job: &str, out: &str, verbose: bool) { + fn cmd_output(&mut self, out: &str, verbose: bool) { self.bar.set_message(out.to_owned()); if verbose { if let Some(line) = self.last_line.take() { - self.bar.println(format!("{}: {}", job, line)); + let prefix = self.bar.prefix(); + self.bar.println(format!("{}: {}", prefix, line)); } self.last_line = Some(out.to_owned()); } diff --git a/src/error.rs b/src/error.rs index a7b9abf..16779a5 100644 --- a/src/error.rs +++ b/src/error.rs @@ -13,7 +13,7 @@ pub enum ZinnError { Yaml(#[from] serde_yaml::Error), #[error("Child exited with error {0}")] - ChildFailed(i32), + ChildFailed(i32, Vec), #[error("Child terminated by signal")] ChildSignaled(), diff --git a/src/job.rs b/src/job.rs index 99cd1e3..f318407 100644 --- a/src/job.rs +++ b/src/job.rs @@ -1,4 +1,5 @@ use std::collections::HashMap; +use std::collections::VecDeque; use std::fmt; use std::fs; use std::path::Path; @@ -15,6 +16,9 @@ use crate::queue::JobState; use crate::render_component; use crate::Options; +/// Maximum number of lines saved for a process to be printed on error. +const MAX_SAVED_OUTPUT_LINES: usize = 20; + /// Template for a job as described in the Zinnfile #[derive(Clone, Debug, Serialize, Deserialize)] @@ -217,6 +221,10 @@ impl InnerJobRealization { tracker.trace(self.cmd()); } + // in order to being able to discard the oldest lines a VecDeque is used to track output + // lines and reversed before returning the last N lines the caller + let mut out_lines = VecDeque::new(); + let cmd_with_exit_setting = format!("set -e; {}", self.run); let mut process = if self.interactive { // run job interactively @@ -235,7 +243,11 @@ impl InnerJobRealization { .spawn()?; for line in BufReader::new(io_reader).lines().map_while(Result::ok) { - tracker.cmd_output(&self.to_string(), &line, options.verbose); + tracker.cmd_output(&line, options.verbose); + + // append line to limited output buffer + out_lines.push_front(line); + out_lines.truncate(MAX_SAVED_OUTPUT_LINES); } tracker.flush_cmd_output(&self.to_string(), options.verbose); @@ -250,9 +262,11 @@ impl InnerJobRealization { } } + let out_lines = out_lines.into_iter().rev().collect(); + if !status.success() { match status.code() { - Some(code) => Err(ZinnError::ChildFailed(code)), + Some(code) => Err(ZinnError::ChildFailed(code, out_lines)), None => Err(ZinnError::ChildSignaled()), } } else { @@ -318,7 +332,12 @@ impl fmt::Display for InnerJobRealization { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "[{}]", self.name())?; if !self.param_values.is_empty() { - write!(f, " {}", self.param_values.join(" "))? + let param_str = self.param_values.iter() + .filter(|s| !s.is_empty()) + .map(|s| "'".to_owned() + s.trim() + "'") + .collect::>() + .join(" "); + write!(f, " {}", param_str)? } Ok(()) } diff --git a/src/main.rs b/src/main.rs index 2bfa51c..1131fd7 100644 --- a/src/main.rs +++ b/src/main.rs @@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize}; use std::collections::HashMap; use std::error::Error; use std::path::Path; -use std::{env, fs, thread}; +use std::{env, fs, process, thread}; use error::*; use job::*; @@ -209,6 +209,10 @@ where for thread in threads { let _ = thread.join(); } + + if queue.has_failed() { + process::exit(1); + } } fn main() { diff --git a/src/queue.rs b/src/queue.rs index d9ce3b5..8e4f702 100644 --- a/src/queue.rs +++ b/src/queue.rs @@ -80,6 +80,10 @@ impl Queue { self.inner.lock().unwrap().jobs.len() } + pub fn has_failed(&self) -> bool { + self.inner.lock().unwrap().failed + } + #[cfg(feature = "progress")] pub fn has_interactive(&self) -> bool { self.inner.lock().unwrap().jobs.iter().any(|j| j.is_interactive())