Skip to content

Commit

Permalink
fix: be more informative on command error
Browse files Browse the repository at this point in the history 
This addresses #43 raised by @robsyme.  Now, external
programs that are invoked using the `std::process::Command`
interface will be run using our `prog_utils::execute_command`
function.  This currently has the following behavior (which we
can modify or enchance in the future).

* If the program exits succesfully, print the message to the `info`
  log along with the command run (setting the log level above `INFO`
  will silence this). Propagate Ok(Output) to the caller.

* If the program is invoked, but exits unsuccsefully, then print
  a message to the `error` log along with the command run. Also
  print to the error log the capture `stdout` and `stderr` of the
  unsuccessful command. Propagate Ok(Output) to the caller.

* If the `Command::output()` function itself fails (returns an Err)
  then write a message to tthe `error` log signifying this along with
  the command run. Propagate Err(std::io::Error) to the caller.
  • Loading branch information
Rob Patro committed Feb 4, 2023
1 parent 49496b7 commit 44fb9cb
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 48 deletions.
47 changes: 25 additions & 22 deletions src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,7 @@ fn build_ref_and_index(af_home_path: PathBuf, index_args: Commands) -> anyhow::R
check_files_exist(&input_files)?;

let pyroe_start = Instant::now();
let cres = cmd.output()?;
let cres = prog_utils::execute_command(&mut cmd)?;
pyroe_duration = Some(pyroe_start.elapsed());

if !cres.status.success() {
Expand Down Expand Up @@ -600,11 +600,14 @@ fn build_ref_and_index(af_home_path: PathBuf, index_args: Commands) -> anyhow::R
.arg(format!("{}", threads));

let index_start = Instant::now();
piscem_index_cmd
.output()
.expect("failed to run piscem build");
let cres = prog_utils::execute_command(&mut piscem_index_cmd)
.expect("failed to invoke piscem index command");
index_duration = index_start.elapsed();

if !cres.status.success() {
bail!("piscem index failed to build succesfully {:?}", cres.status);
}

// copy over the t2g file to the index
let mut t2g_out_path: Option<PathBuf> = None;
if let Some(t2g_file) = splici_t2g {
Expand Down Expand Up @@ -679,11 +682,14 @@ fn build_ref_and_index(af_home_path: PathBuf, index_args: Commands) -> anyhow::R
.arg(format!("{}", threads));

let index_start = Instant::now();
salmon_index_cmd
.output()
.expect("failed to run salmon index");
let cres = prog_utils::execute_command(&mut salmon_index_cmd)
.expect("failed to invoke salmon index command");
index_duration = index_start.elapsed();

if !cres.status.success() {
bail!("salmon index failed to build succesfully {:?}", cres.status);
}

// copy over the t2g file to the index
let mut t2g_out_path: Option<PathBuf> = None;
if let Some(t2g_file) = splici_t2g {
Expand Down Expand Up @@ -1176,12 +1182,12 @@ fn map_and_quant(af_home_path: PathBuf, quant_cmd: Commands) -> anyhow::Result<(
check_files_exist(&input_files)?;

let map_start = Instant::now();
let map_proc_out = piscem_quant_cmd
.output()
let cres = prog_utils::execute_command(&mut piscem_quant_cmd)
.expect("failed to execute piscem [mapping phase]");
map_duration = map_start.elapsed();
if !map_proc_out.status.success() {
bail!("mapping failed with exit status {:?}", map_proc_out.status);

if !cres.status.success() {
bail!("piscem mapping failed with exit status {:?}", cres.status);
}
}
IndexType::Salmon(index_base) => {
Expand Down Expand Up @@ -1243,12 +1249,12 @@ fn map_and_quant(af_home_path: PathBuf, quant_cmd: Commands) -> anyhow::Result<(
check_files_exist(&input_files)?;

let map_start = Instant::now();
let map_proc_out = salmon_quant_cmd
.output()
.expect("failed to execute salmon alevin [mapping phase]");
let cres = prog_utils::execute_command(&mut salmon_quant_cmd)
.expect("failed to execute salmon [mapping phase]");
map_duration = map_start.elapsed();
if !map_proc_out.status.success() {
bail!("mapping failed with exit status {:?}", map_proc_out.status);

if !cres.status.success() {
bail!("salmon mapping failed with exit status {:?}", cres.status);
}
}
IndexType::NoIndex => {
Expand Down Expand Up @@ -1282,8 +1288,7 @@ fn map_and_quant(af_home_path: PathBuf, quant_cmd: Commands) -> anyhow::Result<(
check_files_exist(&input_files)?;

let gpl_start = Instant::now();
let gpl_proc_out = alevin_gpl_cmd
.output()
let gpl_proc_out = prog_utils::execute_command(&mut alevin_gpl_cmd)
.expect("could not execute [generate permit list]");
let gpl_duration = gpl_start.elapsed();

Expand Down Expand Up @@ -1311,8 +1316,7 @@ fn map_and_quant(af_home_path: PathBuf, quant_cmd: Commands) -> anyhow::Result<(
check_files_exist(&input_files)?;

let collate_start = Instant::now();
let collate_proc_out = alevin_collate_cmd
.output()
let collate_proc_out = prog_utils::execute_command(&mut alevin_collate_cmd)
.expect("could not execute [collate]");
let collate_duration = collate_start.elapsed();

Expand Down Expand Up @@ -1345,8 +1349,7 @@ fn map_and_quant(af_home_path: PathBuf, quant_cmd: Commands) -> anyhow::Result<(
check_files_exist(&input_files)?;

let quant_start = Instant::now();
let quant_proc_out = alevin_quant_cmd
.output()
let quant_proc_out = prog_utils::execute_command(&mut alevin_quant_cmd)
.expect("could not execute [quant]");
let quant_duration = quant_start.elapsed();

Expand Down
48 changes: 22 additions & 26 deletions src/utils/af_utils.rs
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use anyhow::{anyhow, Result};
use cmd_lib::run_fun;
use seq_geom_parser::{AppendToCmdArgs, FragmentGeomDesc, PiscemGeomDesc, SalmonSeparateGeomDesc};
use std::collections::HashMap;
use std::path::{Path, PathBuf};
use seq_geom_parser::{AppendToCmdArgs, FragmentGeomDesc, PiscemGeomDesc, SalmonSeparateGeomDesc};
use tracing::error;

#[derive(Debug, Clone)]
Expand Down Expand Up @@ -95,21 +95,19 @@ pub fn add_chemistry_to_args_salmon(chem_str: &str, cmd: &mut std::process::Comm
Some(v) => {
cmd.arg(v);
}
None => {
match extract_geometry(chem_str) {
Ok(frag_desc) => {
let salmon_desc = SalmonSeparateGeomDesc::from_geom_pieces(
&frag_desc.read1_desc,
&frag_desc.read2_desc,
);
salmon_desc.append(cmd);
},
Err(e) => {
error!("{:?}", e);
return Err(e);
}
None => match extract_geometry(chem_str) {
Ok(frag_desc) => {
let salmon_desc = SalmonSeparateGeomDesc::from_geom_pieces(
&frag_desc.read1_desc,
&frag_desc.read2_desc,
);
salmon_desc.append(cmd);
}
}
Err(e) => {
error!("{:?}", e);
return Err(e);
}
},
}
Ok(())
}
Expand All @@ -121,19 +119,17 @@ pub fn add_chemistry_to_args_piscem(chem_str: &str, cmd: &mut std::process::Comm
Some(v) => {
cmd.arg("--geometry").arg(v);
}
None => {
match extract_geometry(chem_str) {
Ok(frag_desc) => {
let piscem_desc =
None => match extract_geometry(chem_str) {
Ok(frag_desc) => {
let piscem_desc =
PiscemGeomDesc::from_geom_pieces(&frag_desc.read1_desc, &frag_desc.read2_desc);
piscem_desc.append(cmd);
},
Err(e) => {
error!("{:?}", e);
return Err(e);
}
piscem_desc.append(cmd);
}
}
Err(e) => {
error!("{:?}", e);
return Err(e);
}
},
}
Ok(())
}
Expand Down
32 changes: 32 additions & 0 deletions src/utils/prog_utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,38 @@ use std::path::PathBuf;
use tracing::{error, info};
use which::which;

pub fn execute_command(
cmd: &mut std::process::Command,
) -> Result<std::process::Output, std::io::Error> {
match cmd.output() {
Ok(output) => {
if output.status.success() {
info!(
"command returned successfully ({}) : {:?}",
output.status, cmd
);
Ok(output)
} else {
error!("command unsuccesful ({}): {:?}", output.status, cmd);
error!(
"stdout :\n====\n{}====",
String::from_utf8_lossy(&output.stdout)
);
error!(
"stderr :\n====\n{}====",
String::from_utf8_lossy(&output.stderr)
);
Ok(output)
}
}
Err(e) => {
error!("command unsuccesful : {:?}", cmd);
error!("error : {}", e);
Err(e)
}
}
}

#[derive(Debug, Serialize, Deserialize, Clone)]
pub struct ProgInfo {
pub exe_path: PathBuf,
Expand Down

0 comments on commit 44fb9cb

Please sign in to comment.