better exit status

crates/service/job/inference_job/src/job/job_loop/command_exit_status.rs

@@ -0,0 +1,81 @@
+use subprocess::ExitStatus;
+
+#[derive(Clone, Eq, PartialEq, Debug)]
+pub enum CommandExitStatus {
+  /// Process was executed and completed successfully
+  Success,
+
+  /// Process was executed and returned a failure (non-zero) exit code
+  Failure,
+
+  /// Process executed, but was terminated early for going over the timeout threshold.
+  Timeout,
+
+  /// Process was executed, but encountered some unknown other status.
+  Unknown,
+
+  /// Process could not be run due to some other condition.
+  FailureWithReason { reason: String },
+}
+
+impl CommandExitStatus {
+
+  pub fn from_exit_status(exit_status: ExitStatus) -> Self {
+    match exit_status {
+      ExitStatus::Exited(0) => CommandExitStatus::Success,
+      ExitStatus::Exited(1) => CommandExitStatus::Failure,
+      ExitStatus::Exited(_other_exit_code) => CommandExitStatus::Failure,
+      ExitStatus::Signaled(_) => CommandExitStatus::Unknown,
+      ExitStatus::Other(_) => CommandExitStatus::Unknown,
+      ExitStatus::Undetermined => CommandExitStatus::Unknown,
+    }
+  }
+
+  pub fn is_success(&self) -> bool {
+    match self {
+      CommandExitStatus::Success => true,
+      CommandExitStatus::Failure => false,
+      CommandExitStatus::Timeout => false,
+      CommandExitStatus::Unknown => false,
+      CommandExitStatus::FailureWithReason { .. } => false,
+    }
+  }
+}
+
+#[cfg(test)]
+mod tests {
+  use subprocess::ExitStatus;
+  use crate::job::job_loop::command_exit_status::CommandExitStatus;
+
+  #[test]
+  fn is_success() {
+    assert_eq!(CommandExitStatus::Success.is_success(), true);
+    assert_eq!(CommandExitStatus::Failure.is_success(), false);
+    assert_eq!(CommandExitStatus::Timeout.is_success(), false);
+    assert_eq!(CommandExitStatus::Unknown.is_success(), false);
+    assert_eq!(CommandExitStatus::FailureWithReason{ reason: "foo".to_string() }.is_success(), false);
+  }
+
+  #[test]
+  fn from_exit_status() {
+    // Success
+    assert_eq!(CommandExitStatus::from_exit_status(ExitStatus::Exited(0)), CommandExitStatus::Success);
+
+    // Linux process failure codes
+    assert_eq!(CommandExitStatus::from_exit_status(ExitStatus::Exited(1)), CommandExitStatus::Failure);
+    assert_eq!(CommandExitStatus::from_exit_status(ExitStatus::Exited(2)), CommandExitStatus::Failure);
+    assert_eq!(CommandExitStatus::from_exit_status(ExitStatus::Exited(100)), CommandExitStatus::Failure);
+    assert_eq!(CommandExitStatus::from_exit_status(ExitStatus::Exited(255)), CommandExitStatus::Failure);
+
+    // Process signal codes
+    assert_eq!(CommandExitStatus::from_exit_status(ExitStatus::Signaled(0)), CommandExitStatus::Unknown);
+    assert_eq!(CommandExitStatus::from_exit_status(ExitStatus::Signaled(1)), CommandExitStatus::Unknown);
+
+    // Process signal codes
+    assert_eq!(CommandExitStatus::from_exit_status(ExitStatus::Other(0)), CommandExitStatus::Unknown);
+    assert_eq!(CommandExitStatus::from_exit_status(ExitStatus::Other(1)), CommandExitStatus::Unknown);
+
+    // Unknown
+    assert_eq!(CommandExitStatus::from_exit_status(ExitStatus::Undetermined), CommandExitStatus::Unknown);
+  }
+}

crates/service/job/inference_job/src/job/job_loop/mod.rs

@@ -1,4 +1,5 @@
 pub mod clear_full_filesystem;
+pub mod command_exit_status;
 pub mod determine_dependency_status;
 pub mod job_success_result;
 pub mod main_loop;

crates/service/job/inference_job/src/job/job_types/vc/so_vits_svc/process_job.rs

@@ -193,7 +193,7 @@ pub async fn process_job(args: SoVitsSvcProcessJobArgs<'_>) -> Result<JobSuccess
 
   let inference_start_time = Instant::now();
 
-  let model_check_result = args.job_dependencies
+  let command_exit_status = args.job_dependencies
       .job_type_details
       .so_vits_svc
       .inference_command
@@ -212,12 +212,12 @@ pub async fn process_job(args: SoVitsSvcProcessJobArgs<'_>) -> Result<JobSuccess
 
   info!("Inference took duration to complete: {:?}", &inference_duration);
 
-  if let Err(err) = model_check_result {
-    error!("Inference failed: {:?}", err);
+  if !command_exit_status.is_success() {
+    error!("Inference failed: {:?}", command_exit_status);
     safe_delete_temp_file(&input_wav_path);
     safe_delete_temp_file(&output_audio_fs_path);
     safe_delete_temp_directory(&work_temp_dir);
-    return Err(ProcessSingleJobError::Other(err));
+    return Err(ProcessSingleJobError::Other(anyhow!("CommandExitStatus: {:?}", command_exit_status)));
   }
 
   // ==================== CHECK ALL FILES EXIST AND GET METADATA ==================== //

crates/service/job/inference_job/src/job/job_types/vc/so_vits_svc/so_vits_svc_inference_command.rs

@@ -12,6 +12,7 @@ use std::path::{Path, PathBuf};
 use std::time::Duration;
 use subprocess::{ExitStatus, Popen, PopenConfig, Redirection};
 use subprocess_common::docker_options::{DockerEnvVar, DockerFilesystemMount, DockerGpu, DockerOptions};
+use crate::job::job_loop::command_exit_status::CommandExitStatus;
 
 // These environment vars are not copied over to the subprocess
 // TODO/FIXME(bt, 2023-05-28): This is horrific security!
@@ -221,7 +222,17 @@ impl SoVitsSvcInferenceCommand {
   pub fn execute_inference<P: AsRef<Path>>(
     &self,
     args: InferenceArgs<P>,
-  ) -> AnyhowResult<()> {
+  ) -> CommandExitStatus {
+    match self.do_execute_inference(args) {
+      Ok(exit_status) => exit_status,
+      Err(error) => CommandExitStatus::FailureWithReason { reason: format!("error: {:?}", error) },
+    }
+  }
+
+  fn do_execute_inference<P: AsRef<Path>>(
+    &self,
+    args: InferenceArgs<P>,
+  ) -> AnyhowResult<CommandExitStatus> {
 
     let mut command = String::new();
     command.push_str(&format!("cd {}", path_to_string(&self.so_vits_svc_root_code_directory)));
@@ -380,21 +391,25 @@ impl SoVitsSvcInferenceCommand {
       None => {
         let exit_status = p.wait()?;
         info!("Subprocess exit status: {:?}", exit_status);
+        Ok(CommandExitStatus::from_exit_status(exit_status))
       }
       Some(timeout) => {
         info!("Executing with timeout: {:?}", &timeout);
         let exit_status = p.wait_timeout(timeout.clone())?;
 
-        if exit_status.is_none() {
-          // NB: If the program didn't successfully terminate, kill it.
-          info!("Subprocess didn't end after timeout: {:?}; terminating...", &timeout);
-          let _r = p.terminate()?;
-        } else {
-          info!("Subprocess timed wait exit status: {:?}", exit_status);
+        match exit_status {
+          None => {
+            // NB: If the program didn't successfully terminate, kill it.
+            info!("Subprocess didn't end after timeout: {:?}; terminating...", &timeout);
+            let _r = p.terminate()?;
+            Ok(CommandExitStatus::Timeout)
+          }
+          Some(exit_status) => {
+            info!("Subprocess timed wait exit status: {:?}", exit_status);
+            Ok(CommandExitStatus::from_exit_status(exit_status))
+          }
         }
       }
     }
-
-    Ok(())
   }
 }