improve errors II

windmill-labs · Feb 5, 2025 · 8092429 · 8092429
1 parent 32b75b8
commit 8092429
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 24 deletions.
diff --git a/backend/windmill-common/src/error.rs b/backend/windmill-common/src/error.rs
@@ -22,8 +22,6 @@ pub type JsonResult<T> = std::result::Result<Json<T>, Error>;
 
 #[derive(Debug, Error)]
 pub enum Error {
-    #[error("Uuid Error {0}")]
-    UuidErr(#[from] uuid::Error),
     #[error("Bad config: {0}")]
     BadConfig(String),
     #[error("Connecting to database: {0}")]
@@ -40,12 +38,16 @@ pub enum Error {
     RequireAdmin(String),
     #[error("{0}")]
     ExecutionErr(String),
-    #[error("IO error: {0}")]
-    IoErr(#[from] io::Error),
-    // #[error("Sql error: {0}")]
-    // SqlErr(#[from] sqlx::Error),
+    #[error("IoErr: {error:#} @{location:#}")]
+    IoErr { error: io::Error, location: String },
+    #[error("Utf8Err: {error:#} @{location:#}")]
+    Utf8Err { error: std::string::FromUtf8Error, location: String },
+    #[error("UuidErr: {error:#} @{location:#}")]
+    UuidErr { error: uuid::Error, location: String },
     #[error("SqlErr: {error:#} @{location:#}")]
     SqlErr { error: sqlx::Error, location: String },
+    #[error("SerdeJson: {error:#} @{location:#}")]
+    SerdeJson { error: serde_json::Error, location: String },
     #[error("Bad request: {0}")]
     BadRequest(String),
     #[error("Quota exceeded: {0}")]
@@ -54,12 +56,12 @@ pub enum Error {
     InternalErr(String),
     #[error("Internal: {0}: {1}")]
     InternalErrAt(&'static Location<'static>, String),
-    #[error("Hexadecimal decoding error: {0}")]
-    HexErr(#[from] hex::FromHexError),
+    #[error("HexErr: {error:#} @{location:#}")]
+    HexErr { error: hex::FromHexError, location: String },
     #[error("Migrating database: {0}")]
     DatabaseMigration(#[from] MigrateError),
-    #[error("Non-zero exit status: {0}")]
-    ExitStatus(i32),
+    #[error("Non-zero exit status for {0}: {1}")]
+    ExitStatus(String, i32),
     #[error("Error: {error:#} @{location:#}")]
     Anyhow { error: anyhow::Error, location: String },
     #[error("Error: {0:#?}")]
@@ -70,10 +72,6 @@ pub enum Error {
     AlreadyCompleted(String),
     #[error("Find python error: {0}")]
     FindPythonError(String),
-    #[error("{0}")]
-    Utf8(#[from] std::string::FromUtf8Error),
-    #[error("Encoding/decoding error: {0}")]
-    SerdeJson(#[from] serde_json::Error),
 }
 
 fn prettify_location(location: &'static Location<'static>) -> String {
@@ -92,11 +90,47 @@ impl From<anyhow::Error> for Error {
 }
 
 impl From<sqlx::Error> for Error {
+    #[track_caller]
     fn from(e: sqlx::Error) -> Self {
         Self::SqlErr { error: e, location: prettify_location(std::panic::Location::caller()) }
     }
 }
 
+impl From<uuid::Error> for Error {
+    #[track_caller]
+    fn from(e: uuid::Error) -> Self {
+        Self::UuidErr { error: e, location: prettify_location(std::panic::Location::caller()) }
+    }
+}
+
+impl From<std::string::FromUtf8Error> for Error {
+    #[track_caller]
+    fn from(e: std::string::FromUtf8Error) -> Self {
+        Self::Utf8Err { error: e, location: prettify_location(std::panic::Location::caller()) }
+    }
+}
+
+impl From<io::Error> for Error {
+    #[track_caller]
+    fn from(e: io::Error) -> Self {
+        Self::IoErr { error: e, location: prettify_location(std::panic::Location::caller()) }
+    }
+}
+
+impl From<hex::FromHexError> for Error {
+    #[track_caller]
+    fn from(e: hex::FromHexError) -> Self {
+        Self::HexErr { error: e, location: prettify_location(std::panic::Location::caller()) }
+    }
+}
+
+impl From<serde_json::Error> for Error {
+    #[track_caller]
+    fn from(e: serde_json::Error) -> Self {
+        Self::SerdeJson { error: e, location: prettify_location(std::panic::Location::caller()) }
+    }
+}
+
 impl Error {
     /// https://docs.rs/anyhow/1/anyhow/struct.Error.html#display-representations
     pub fn alt(&self) -> String {

diff --git a/backend/windmill-worker/src/common.rs b/backend/windmill-worker/src/common.rs
@@ -570,7 +570,7 @@ impl OccupancyMetrics {
 pub async fn start_child_process(mut cmd: Command, executable: &str) -> Result<Child, Error> {
     return cmd
         .spawn()
-        .map_err(|err| tentatively_improve_error(Error::IoErr(err), executable));
+        .map_err(|err| tentatively_improve_error(err.into(), executable));
 }
 
 pub async fn resolve_job_timeout(

diff --git a/backend/windmill-worker/src/dedicated_worker.rs b/backend/windmill-worker/src/dedicated_worker.rs
@@ -78,7 +78,7 @@ pub async fn handle_dedicated_process(
     //do not cache local dependencies
 
     use crate::{handle_child::process_status, PROXY_ENVS};
-
+    let cmd_name = format!("dedicated {command_path}");
     let mut child = {
         let mut cmd = Command::new(command_path);
         cmd.current_dir(job_dir)
@@ -126,7 +126,7 @@ pub async fn handle_dedicated_process(
             .wait()
             .await
             .expect("child process encountered an error");
-        if let Err(e) = process_status(status) {
+        if let Err(e) = process_status(&cmd_name, status) {
             tracing::error!("child exit status was not success: {e:#}");
         } else {
             tracing::info!("child exit status was success");

diff --git a/backend/windmill-worker/src/handle_child.rs b/backend/windmill-worker/src/handle_child.rs
@@ -426,7 +426,7 @@ pub async fn handle_child(
         _ if *too_many_logs.borrow() => Err(Error::ExecutionErr(format!(
             "logs or result reached limit. (current max size: {MAX_RESULT_SIZE} characters)"
         ))),
-        Ok(Ok(status)) => process_status(status),
+        Ok(Ok(status)) => process_status(&child_name, status),
         Ok(Err(kill_reason)) => match kill_reason {
             KillReason::AlreadyCompleted => {
                 Err(Error::AlreadyCompleted("Job already completed".to_string()))
@@ -714,11 +714,11 @@ pub fn lines_to_stream<R: tokio::io::AsyncBufRead + Unpin>(
     })
 }
 
-pub fn process_status(status: ExitStatus) -> error::Result<()> {
+pub fn process_status(program: &str, status: ExitStatus) -> error::Result<()> {
     if status.success() {
         Ok(())
     } else if let Some(code) = status.code() {
-        Err(error::Error::ExitStatus(code))
+        Err(error::Error::ExitStatus(program.to_string(), code))
     } else {
         #[cfg(any(target_os = "linux", target_os = "macos"))]
         return Err(error::Error::ExecutionErr(format!(

diff --git a/backend/windmill-worker/src/result_processor.rs b/backend/windmill-worker/src/result_processor.rs
@@ -331,7 +331,7 @@ pub async fn process_result(
         }
         Err(e) => {
             let error_value = match e {
-                Error::ExitStatus(i) => {
+                Error::ExitStatus(program, i) => {
                     let res = read_result(job_dir).await.ok();
 
                     if res.as_ref().is_some_and(|x| !x.get().is_empty()) {
@@ -348,7 +348,7 @@ pub async fn process_result(
                             .last()
                             .unwrap_or(&last_10_log_lines);
 
-                        extract_error_value(log_lines, i, job.flow_step_id.clone())
+                        extract_error_value(&program, log_lines, i, job.flow_step_id.clone())
                     }
                 }
                 err @ _ => to_raw_value(&SerializedError {
@@ -662,10 +662,15 @@ pub struct SerializedError {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub exit_code: Option<i32>,
 }
-pub fn extract_error_value(log_lines: &str, i: i32, step_id: Option<String>) -> Box<RawValue> {
+pub fn extract_error_value(
+    program: &str,
+    log_lines: &str,
+    i: i32,
+    step_id: Option<String>,
+) -> Box<RawValue> {
     return to_raw_value(&SerializedError {
         message: format!(
-            "ExitCode: {i}, last log lines:\n{}",
+            "exit code for \"{program}\": {i}, last log lines:\n{}",
             ANSI_ESCAPE_RE.replace_all(log_lines.trim(), "").to_string()
         ),
         name: "ExecutionErr".to_string(),