diff --git a/backend/windmill-common/src/error.rs b/backend/windmill-common/src/error.rs index 17094dc717d64..2fe00bc4ab1a6 100644 --- a/backend/windmill-common/src/error.rs +++ b/backend/windmill-common/src/error.rs @@ -22,8 +22,6 @@ pub type JsonResult = std::result::Result, 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 for Error { } impl From for Error { + #[track_caller] fn from(e: sqlx::Error) -> Self { Self::SqlErr { error: e, location: prettify_location(std::panic::Location::caller()) } } } +impl From for Error { + #[track_caller] + fn from(e: uuid::Error) -> Self { + Self::UuidErr { error: e, location: prettify_location(std::panic::Location::caller()) } + } +} + +impl From for Error { + #[track_caller] + fn from(e: std::string::FromUtf8Error) -> Self { + Self::Utf8Err { error: e, location: prettify_location(std::panic::Location::caller()) } + } +} + +impl From for Error { + #[track_caller] + fn from(e: io::Error) -> Self { + Self::IoErr { error: e, location: prettify_location(std::panic::Location::caller()) } + } +} + +impl From for Error { + #[track_caller] + fn from(e: hex::FromHexError) -> Self { + Self::HexErr { error: e, location: prettify_location(std::panic::Location::caller()) } + } +} + +impl From 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 index 099300f621598..5212d80e11c8b 100644 --- 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 { 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 index 56ea01c03846b..ff6b8cec79ebb 100644 --- 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 index 5dcdbfa28763e..32ec33f7f35dc 100644 --- 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( }) } -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 index f4acc66f29cdd..bb516cfda2fcb 100644 --- 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, } -pub fn extract_error_value(log_lines: &str, i: i32, step_id: Option) -> Box { +pub fn extract_error_value( + program: &str, + log_lines: &str, + i: i32, + step_id: Option, +) -> Box { 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(),