Skip to content

Commit

Permalink
Improving error output
Browse files Browse the repository at this point in the history
  • Loading branch information
jzbor committed Jul 20, 2024
1 parent ed99f9d commit 9f5ba1d
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 13 deletions.
32 changes: 24 additions & 8 deletions src/barkeeper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -33,7 +33,7 @@ pub struct ThreadBarkeeper {
}

pub struct DummyBarkeeper {}
pub struct DummyThreadBarkeeper {}
pub struct DummyThreadBarkeeper { prefix: String }


#[cfg(feature = "progress")]
Expand Down Expand Up @@ -93,7 +93,7 @@ impl StateTracker for DummyBarkeeper {

fn for_threads(&self, nthreads: usize) -> Vec<DummyThreadBarkeeper> {
(0..nthreads).map(|_| {
DummyThreadBarkeeper {}
DummyThreadBarkeeper { prefix: String::new() }
}).collect()
}
}
Expand All @@ -102,18 +102,27 @@ impl ThreadStateTracker for DummyThreadBarkeeper {
fn job_completed(&self, job: JobRealization, state: JobState, error: Option<ZinnError>) {
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) {}
Expand All @@ -134,6 +143,12 @@ impl ThreadStateTracker for ThreadBarkeeper {
fn job_completed(&self, job: JobRealization, state: JobState, error: Option<ZinnError>) {
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)
Expand All @@ -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());
}
Expand Down
2 changes: 1 addition & 1 deletion src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ pub enum ZinnError {
Yaml(#[from] serde_yaml::Error),

#[error("Child exited with error {0}")]
ChildFailed(i32),
ChildFailed(i32, Vec<String>),

#[error("Child terminated by signal")]
ChildSignaled(),
Expand Down
25 changes: 22 additions & 3 deletions src/job.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::collections::HashMap;
use std::collections::VecDeque;
use std::fmt;
use std::fs;
use std::path::Path;
Expand All @@ -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)]
Expand Down Expand Up @@ -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
Expand All @@ -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);

Expand All @@ -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 {
Expand Down Expand Up @@ -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::<Vec<_>>()
.join(" ");
write!(f, " {}", param_str)?
}
Ok(())
}
Expand Down
6 changes: 5 additions & 1 deletion src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -209,6 +209,10 @@ where
for thread in threads {
let _ = thread.join();
}

if queue.has_failed() {
process::exit(1);
}
}

fn main() {
Expand Down
4 changes: 4 additions & 0 deletions src/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down

0 comments on commit 9f5ba1d

Please sign in to comment.