Skip to content

Commit

Permalink
improve error messages with locations
Browse files Browse the repository at this point in the history
  • Loading branch information
rubenfiszel committed Feb 5, 2025
1 parent 7573285 commit 32b75b8
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 15 deletions.
38 changes: 31 additions & 7 deletions backend/windmill-common/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,10 @@ pub enum Error {
ExecutionErr(String),
#[error("IO error: {0}")]
IoErr(#[from] io::Error),
#[error("Sql error: {0}")]
SqlErr(#[from] sqlx::Error),
// #[error("Sql error: {0}")]
// SqlErr(#[from] sqlx::Error),
#[error("SqlErr: {error:#} @{location:#}")]
SqlErr { error: sqlx::Error, location: String },
#[error("Bad request: {0}")]
BadRequest(String),
#[error("Quota exceeded: {0}")]
Expand All @@ -58,8 +60,8 @@ pub enum Error {
DatabaseMigration(#[from] MigrateError),
#[error("Non-zero exit status: {0}")]
ExitStatus(i32),
#[error("Err: {0:#}")]
Anyhow(#[from] anyhow::Error),
#[error("Error: {error:#} @{location:#}")]
Anyhow { error: anyhow::Error, location: String },
#[error("Error: {0:#?}")]
JsonErr(serde_json::Value),
#[error("{0}")]
Expand All @@ -74,6 +76,27 @@ pub enum Error {
SerdeJson(#[from] serde_json::Error),
}

fn prettify_location(location: &'static Location<'static>) -> String {
location
.to_string()
.split("/")
.last()
.unwrap_or("unknown")
.to_string()
}
impl From<anyhow::Error> for Error {
#[track_caller]
fn from(e: anyhow::Error) -> Self {
Self::Anyhow { error: e, location: prettify_location(std::panic::Location::caller()) }
}
}

impl From<sqlx::Error> for Error {
fn from(e: sqlx::Error) -> Self {
Self::SqlErr { 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 {
Expand Down Expand Up @@ -109,9 +132,10 @@ impl IntoResponse for Error {
Self::NotFound(_) => axum::http::StatusCode::NOT_FOUND,
Self::NotAuthorized(_) => axum::http::StatusCode::UNAUTHORIZED,
Self::RequireAdmin(_) => axum::http::StatusCode::FORBIDDEN,
Self::SqlErr(_) | Self::BadRequest(_) | Self::AiError(_) | Self::QuotaExceeded(_) => {
axum::http::StatusCode::BAD_REQUEST
}
Self::SqlErr { .. }
| Self::BadRequest(_)
| Self::AiError(_)
| Self::QuotaExceeded(_) => axum::http::StatusCode::BAD_REQUEST,
_ => axum::http::StatusCode::INTERNAL_SERVER_ERROR,
};

Expand Down
2 changes: 1 addition & 1 deletion backend/windmill-common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,14 @@ pub mod job_s3_helpers_ee;
pub mod jobs;
pub mod more_serde;
pub mod oauth2;
pub mod teams_ee;
pub mod otel_ee;
pub mod queue;
pub mod s3_helpers;
pub mod schedule;
pub mod scripts;
pub mod server;
pub mod stats_ee;
pub mod teams_ee;
pub mod tracing_init;
pub mod users;
pub mod utils;
Expand Down
2 changes: 1 addition & 1 deletion backend/windmill-common/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ pub async fn fetch_mute_workspace(_db: &DB, workspace_id: &str) -> Result<bool>
workspace_id,
err
);
Err(Error::SqlErr(err))
return Err(err.into());
}
}
}
Expand Down
17 changes: 12 additions & 5 deletions backend/windmill-worker/src/bun_executor.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
#[cfg(feature = "deno_core")]
use std::time::Instant;
use std::{collections::HashMap, fs, io, path::Path, process::Stdio};
use std::{collections::HashMap, fs, path::Path, process::Stdio};

use anyhow::Context;
use base64::Engine;
use itertools::Itertools;

Expand Down Expand Up @@ -644,15 +645,17 @@ pub fn copy_recursively(
source: impl AsRef<Path>,
destination: impl AsRef<Path>,
skip: Option<&Vec<String>>,
) -> io::Result<()> {
) -> Result<()> {
let mut stack = Vec::new();
stack.push((
source.as_ref().to_path_buf(),
destination.as_ref().to_path_buf(),
0,
));
while let Some((current_source, current_destination, level)) = stack.pop() {
for entry in fs::read_dir(&current_source)? {
for entry in fs::read_dir(&current_source)
.context(format!("reading directory {current_source:?}"))?
{
let entry = entry?;
let filetype = entry.file_type()?;
let destination = current_destination.join(entry.file_name());
Expand All @@ -670,7 +673,11 @@ pub fn copy_recursively(
fs::create_dir_all(&destination)?;
stack.push((entry.path(), destination, level + 1));
} else {
fs::hard_link(&original, &destination)?
fs::hard_link(&original, &destination).map_err(|e| {
error::Error::InternalErr(format!(
"hard linking from {original:?} to {destination:?}: {e:#}"
))
})?;
}
}
}
Expand Down Expand Up @@ -984,7 +991,7 @@ pub async fn handle_bun_job(
"bunfig.toml".to_string(),
]),
) {
fs::remove_dir_all(&buntar_path)?;
fs::remove_dir_all(&buntar_path).context("deleting buntar directory")?;
tracing::error!("Could not create buntar: {e}");
}
}
Expand Down
2 changes: 1 addition & 1 deletion backend/windmill-worker/src/result_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ pub async fn process_result(
}
}
err @ _ => to_raw_value(&SerializedError {
message: format!("error during execution of the script:\n{}", err),
message: format!("error during execution of the script:\n{err:#}",),
name: "ExecutionErr".to_string(),
step_id: job.flow_step_id.clone(),
exit_code: None,
Expand Down

0 comments on commit 32b75b8

Please sign in to comment.