diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index ed1d1768..d98b6b5d 100644 --- a/.devcontainer/Dockerfile +++ b/.devcontainer/Dockerfile @@ -34,5 +34,7 @@ RUN \ cargo install cargo-llvm-cov && \ rustup component add llvm-tools-preview && \ # rust crate structure diagram - cargo install cargo-modules + cargo install cargo-modules && \ + # expand rust macros (useful in debugging) + cargo install cargo-expand CMD ["bash", "-c", "sudo rm /var/run/docker.pid; sudo dockerd"] diff --git a/.devcontainer/gpu/Dockerfile b/.devcontainer/gpu/Dockerfile index c17ce2ff..dddb1829 100644 --- a/.devcontainer/gpu/Dockerfile +++ b/.devcontainer/gpu/Dockerfile @@ -33,6 +33,8 @@ RUN apt-get update && curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs rustup component add llvm-tools-preview && \ # rust crate structure diagram cargo install cargo-modules && \ + # expand rust macros (useful in debugging) + cargo install cargo-expand && \ apt-get clean ENV PATH=${PATH}:/root/.local/bin diff --git a/.github/workflows/docs.yaml b/.github/workflows/docs.yaml index 126b4469..9a3e2f39 100644 --- a/.github/workflows/docs.yaml +++ b/.github/workflows/docs.yaml @@ -22,11 +22,11 @@ jobs: - name: Generate crate diagram run: | mkdir -p docs + set -o pipefail cargo modules dependencies \ --no-uses --no-fns \ - --focus-on "orcapod::model::{Pod}" \ + --focus-on "orcapod::uniffi::model::{Pod}" \ --layout dot | \ - awk '/ERROR/ { print > "/dev/stderr"; next; }; 1' | \ dot -T svg > docs/crate_diagram.svg - name: Sync GitHub run: | diff --git a/.vscode/launch.json b/.vscode/launch.json index 369083f1..6d832068 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -72,13 +72,9 @@ "cargo": { "args": [ "build", - "--bin=exe_file_stem", - "--package=orcapod" + "--package=orcapod", + "--bin=exe_file_stem" ], - "filter": { - "name": "exe_file_stem", - "kind": "bin" - } }, "args": [], "cwd": "${workspaceFolder}", @@ -91,13 +87,9 @@ "args": [ "test", "--no-run", - "--bin=exe_file_stem", - "--package=orcapod" + "--package=orcapod", + "--bin=exe_file_stem" ], - "filter": { - "name": "exe_file_stem", - "kind": "bin" - } }, "args": [], "cwd": "${workspaceFolder}", diff --git a/Cargo.toml b/Cargo.toml index 278c3bd4..3a3c9e94 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,7 +36,7 @@ serde = { version = "1.0.210", features = ["derive"] } # serialization/deseriali serde_json = "1.0.137" # JSON in sharing memory with local docker orchestrator serde_yaml = "0.9.34" # YAML in filestore sha2 = "0.10.8" # checksums based on SHA256 -thiserror = "2.0.11" # library error handling API +snafu = { version = "0.8.5", features = ["futures"] } # library error handling API tokio = { version = "1.41.0", features = ["full"] } # a runtime for async applications tokio-util = "0.7.13" # utilities for async calls diff --git a/README.md b/README.md index a96c5c60..6bee904b 100644 --- a/README.md +++ b/README.md @@ -20,9 +20,9 @@ cargo llvm-cov --html -- --nocapture # integration tests w/ coverage report ```bash cargo doc --no-deps # gen api docs (target/doc/orcapod/index.html) -cargo modules dependencies --no-uses --no-fns --focus-on "orcapod::model::{Pod}" --layout dot > docs/crate_diagram.dot # orcapod diagram as DOT -cargo modules dependencies --no-uses --no-fns --focus-on "orcapod::model::{Pod}" --layout dot | dot -T png > docs/crate_diagram.png # orcapod diagram as PNG -cargo modules dependencies --no-uses --no-fns --focus-on "orcapod::model::{Pod}" --layout dot | dot -T svg > docs/crate_diagram.svg # orcapod diagram as SVG +cargo modules dependencies --no-uses --no-fns --focus-on "orcapod::uniffi::model::{Pod}" --layout dot > docs/crate_diagram.dot # orcapod diagram as DOT +cargo modules dependencies --no-uses --no-fns --focus-on "orcapod::uniffi::model::{Pod}" --layout dot | dot -T png > docs/crate_diagram.png # orcapod diagram as PNG +cargo modules dependencies --no-uses --no-fns --focus-on "orcapod::uniffi::model::{Pod}" --layout dot | dot -T svg > docs/crate_diagram.svg # orcapod diagram as SVG ``` ## Project Management diff --git a/src/core/crypto.rs b/src/core/crypto.rs index 453c385f..b18e3435 100644 --- a/src/core/crypto.rs +++ b/src/core/crypto.rs @@ -1,12 +1,13 @@ use crate::{ core::util::get, uniffi::{ - error::Result, + error::{Result, selector}, model::{Blob, BlobKind}, }, }; use serde_yaml; use sha2::{Digest as _, Sha256}; +use snafu::ResultExt as _; use std::{ collections::{BTreeMap, HashMap}, fs::File, @@ -47,7 +48,11 @@ pub fn hash_buffer(buffer: impl AsRef<[u8]>) -> String { /// /// Will return error if unable to access file. pub fn hash_file(filepath: impl AsRef) -> Result { - hash_stream(&mut File::open(filepath)?) + hash_stream( + &mut File::open(&filepath).context(selector::InvalidFilepath { + path: filepath.as_ref(), + })?, + ) } /// Evaluate checksum hash of a directory. /// diff --git a/src/core/error.rs b/src/core/error.rs index a644bae5..ea324f35 100644 --- a/src/core/error.rs +++ b/src/core/error.rs @@ -4,59 +4,92 @@ use glob; use serde_json; use serde_yaml; use std::{ - fmt::{self, Display, Formatter}, - io, path, + backtrace::{Backtrace, BacktraceStatus}, + fmt::{self, Formatter}, + io, + path::{self}, }; -impl Display for OrcaError { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.kind) - } -} impl From for OrcaError { fn from(error: BollardError) -> Self { - Self { - kind: Kind::BollardError(error), - } + Self(Kind::BollardError { + source: error, + backtrace: Some(Backtrace::capture()), + }) } } impl From for OrcaError { fn from(error: glob::PatternError) -> Self { - Self { - kind: Kind::GlobPatternError(error), - } + Self(Kind::GlobPatternError { + source: error, + backtrace: Some(Backtrace::capture()), + }) } } impl From for OrcaError { fn from(error: io::Error) -> Self { - Self { - kind: Kind::IoError(error), - } + Self(Kind::IoError { + source: error, + backtrace: Some(Backtrace::capture()), + }) } } impl From for OrcaError { fn from(error: path::StripPrefixError) -> Self { - Self { - kind: Kind::PathPrefixError(error), - } + Self(Kind::PathPrefixError { + source: error, + backtrace: Some(Backtrace::capture()), + }) } } impl From for OrcaError { fn from(error: serde_json::Error) -> Self { - Self { - kind: Kind::SerdeJsonError(error), - } + Self(Kind::SerdeJsonError { + source: error, + backtrace: Some(Backtrace::capture()), + }) } } impl From for OrcaError { fn from(error: serde_yaml::Error) -> Self { - Self { - kind: Kind::SerdeYamlError(error), - } + Self(Kind::SerdeYamlError { + source: error, + backtrace: Some(Backtrace::capture()), + }) } } -impl From for OrcaError { - fn from(kind: Kind) -> Self { - Self { kind } +fn format_stack(backtrace: Option<&Backtrace>) -> String { + backtrace.map_or( + String::new(), + |unpacked_backtrace| match unpacked_backtrace.status() { + BacktraceStatus::Captured => { + format!("\nstack backtrace:\n{unpacked_backtrace}") + } + BacktraceStatus::Disabled | BacktraceStatus::Unsupported | _ => String::new(), + }, + ) +} +impl fmt::Debug for OrcaError { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + match &self.0 { + Kind::EmptyResponseWhenLoadingContainerAltImage { backtrace, .. } + | Kind::GeneratedNamesOverflow { backtrace, .. } + | Kind::InvalidFilepath { backtrace, .. } + | Kind::InvalidPodResultTerminatedDatetime { backtrace, .. } + | Kind::KeyMissing { backtrace, .. } + | Kind::NoAnnotationFound { backtrace, .. } + | Kind::NoContainerNames { backtrace, .. } + | Kind::NoFileName { backtrace, .. } + | Kind::NoMatchingPodRun { backtrace, .. } + | Kind::NoTagFoundInContainerAltImage { backtrace, .. } + | Kind::BollardError { backtrace, .. } + | Kind::GlobPatternError { backtrace, .. } + | Kind::IoError { backtrace, .. } + | Kind::PathPrefixError { backtrace, .. } + | Kind::SerdeJsonError { backtrace, .. } + | Kind::SerdeYamlError { backtrace, .. } => { + write!(f, "{}{}", self.0, format_stack(backtrace.as_ref())) + } + } } } diff --git a/src/core/orchestrator/docker.rs b/src/core/orchestrator/docker.rs index 46ec04ac..7a7e813b 100644 --- a/src/core/orchestrator/docker.rs +++ b/src/core/orchestrator/docker.rs @@ -1,7 +1,7 @@ use crate::{ core::util::get, uniffi::{ - error::{Kind, OrcaError, Result}, + error::{Result, selector}, model::{Input, PodJob}, orchestrator::{RunInfo, Status, docker::LocalDockerOrchestrator}, }, @@ -14,6 +14,7 @@ use chrono::DateTime; use futures_util::future::join_all; use names::{Generator, Name}; use regex::Regex; +use snafu::OptionExt as _; use std::{ collections::HashMap, fs, @@ -81,9 +82,9 @@ impl LocalDockerOrchestrator { .to_string_lossy(), stream_info .path - .join(blob.location.path.file_name().ok_or( - Kind::InvalidPath { - path: blob.location.path.clone(), + .join(blob.location.path.file_name().context( + selector::NoFileName { + path: blob.location.path.clone() } )?) .to_string_lossy(), @@ -120,7 +121,7 @@ impl LocalDockerOrchestrator { let (input_binds, output_bind) = Self::prepare_mount_binds(namespace_lookup, pod_job)?; let container_name = Generator::with_naming(Name::Plain) .next() - .ok_or(OrcaError::from(Kind::GeneratedNamesOverflow))?; + .context(selector::GeneratedNamesOverflow)?; let labels = HashMap::from([ ("org.orcapod".to_owned(), "true".to_owned()), ( @@ -207,7 +208,7 @@ impl LocalDockerOrchestrator { let container_name = &container_summary .names .as_ref() - .ok_or(OrcaError::from(Kind::NoContainerNames))?[0][1..]; + .context(selector::NoContainerNames)?[0][1..]; Ok(( container_name.to_owned(), container_summary.clone(), diff --git a/src/core/store/filestore.rs b/src/core/store/filestore.rs index d0bded42..0548e22e 100644 --- a/src/core/store/filestore.rs +++ b/src/core/store/filestore.rs @@ -1,7 +1,7 @@ use crate::{ core::{model::to_yaml, util::get_type_name}, uniffi::{ - error::{Kind, OrcaError, Result}, + error::{Result, selector}, model::Annotation, store::{ModelID, ModelInfo, Store as _, filestore::LocalFileStore}, }, @@ -12,6 +12,7 @@ use heck::ToSnakeCase as _; use regex::Regex; use serde::{Serialize, de::DeserializeOwned}; use serde_yaml; +use snafu::OptionExt as _; use std::{ fs, path::{Path, PathBuf}, @@ -85,12 +86,10 @@ impl LocalFileStore { &self.make_path::("*", Self::make_annotation_relpath(name, version)), )? .next() - .ok_or_else(|| { - OrcaError::from(Kind::NoAnnotationFound { - class: get_type_name::().to_snake_case(), - name: name.to_owned(), - version: version.to_owned(), - }) + .context(selector::NoAnnotationFound { + class: get_type_name::().to_snake_case(), + name: name.to_owned(), + version: version.to_owned(), })?; Ok(model_info.hash) } @@ -128,11 +127,15 @@ impl LocalFileStore { }) { println!( - "Skip saving {} annotation since `{}`, `{}`, `{}` exists.", - model_type.bright_cyan(), - found_hash.bright_cyan(), - found_name.bright_cyan(), - found_version.bright_cyan(), + "{}", + format!( + "Skip saving {} annotation since `{}`, `{}`, `{}` exists.", + model_type.bright_cyan(), + found_hash.bright_cyan(), + found_name.bright_cyan(), + found_version.bright_cyan(), + ) + .yellow(), ); } else { Self::save_file( @@ -145,9 +148,13 @@ impl LocalFileStore { let spec_file = &self.make_path::(hash, Self::SPEC_RELPATH); if spec_file.exists() { println!( - "Skip saving {} model since `{}` exists.", - model_type.bright_cyan(), - hash.bright_cyan(), + "{}", + format!( + "Skip saving {} model since `{}` exists.", + model_type.bright_cyan(), + hash.bright_cyan(), + ) + .yellow(), ); } else { Self::save_file(spec_file, to_yaml(model)?)?; diff --git a/src/core/util.rs b/src/core/util.rs index ae110ee2..cdf14033 100644 --- a/src/core/util.rs +++ b/src/core/util.rs @@ -1,4 +1,5 @@ -use crate::uniffi::error::{Kind, Result}; +use crate::uniffi::error::{Result, selector}; +use snafu::OptionExt as _; use std::{any::type_name, collections::HashMap}; #[expect( @@ -14,7 +15,7 @@ pub fn get_type_name() -> String { } pub fn get<'map, T>(map: &'map HashMap, key: &str) -> Result<&'map T> { - Ok(map.get(key).ok_or(Kind::KeyMissing { + Ok(map.get(key).context(selector::KeyMissing { key: key.to_owned(), })?) } diff --git a/src/uniffi/error.rs b/src/uniffi/error.rs index 7bb92d16..34ad3d83 100644 --- a/src/uniffi/error.rs +++ b/src/uniffi/error.rs @@ -1,69 +1,118 @@ +#![expect( + clippy::field_scoped_visibility_modifiers, + reason = "Needed since SNAFU dynamically generating selectors." +)] + use bollard::errors::Error as BollardError; +use glob; +use serde_json; +use serde_yaml; +use snafu::prelude::Snafu; use std::{ + backtrace::Backtrace, io, path::{self, PathBuf}, result, }; -use thiserror::Error; /// Shorthand for a Result that returns an `OrcaError`. -pub type Result = result::Result; +pub type Result = result::Result; /// Possible errors you may encounter. -#[derive(Error, Debug)] +#[derive(Snafu, Debug)] +#[snafu(module(selector), visibility(pub(crate)), context(suffix(false)))] pub(crate) enum Kind { - #[error( - "Received an empty response when attempting to load the alternate container image file: {path}." - )] - EmptyResponseWhenLoadingContainerAltImage { path: PathBuf }, - #[error("Out of generated random names.")] - GeneratedNamesOverflow, - #[error("Path missing a file or directory name: {path}.")] - InvalidPath { path: PathBuf }, - #[error("An invalid datetime was set for pod result for pod job (hash: {pod_job_hash}).")] - InvalidPodResultTerminatedDatetime { pod_job_hash: String }, - #[error("Key '{key}' was not found in map.")] - KeyMissing { key: String }, - #[error("No annotation found for `{name}:{version}` {class}.")] + #[snafu(display( + "Received an empty response when attempting to load the alternate container image file: {path:?}." + ))] + EmptyResponseWhenLoadingContainerAltImage { + path: PathBuf, + backtrace: Option, + }, + #[snafu(display("Out of generated random names."))] + GeneratedNamesOverflow { backtrace: Option }, + #[snafu(display("{source} ({path:?})."))] + InvalidFilepath { + path: PathBuf, + source: io::Error, + backtrace: Option, + }, + #[snafu(display( + "An invalid datetime was set for pod result for pod job (hash: {pod_job_hash})." + ))] + InvalidPodResultTerminatedDatetime { + pod_job_hash: String, + backtrace: Option, + }, + #[snafu(display("Key '{key}' was not found in map."))] + KeyMissing { + key: String, + backtrace: Option, + }, + #[snafu(display("No annotation found for `{name}:{version}` {class}."))] NoAnnotationFound { class: String, name: String, version: String, + backtrace: Option, + }, + #[snafu(display("No known container names."))] + NoContainerNames { backtrace: Option }, + #[snafu(display("Missing file or directory name ({path:?})."))] + NoFileName { + path: PathBuf, + backtrace: Option, + }, + #[snafu(display("No corresponding pod run found for pod job (hash: {pod_job_hash})."))] + NoMatchingPodRun { + pod_job_hash: String, + backtrace: Option, + }, + #[snafu(display("No tags found in provided container alternate image: {path:?}."))] + NoTagFoundInContainerAltImage { + path: PathBuf, + backtrace: Option, + }, + #[snafu(transparent)] + BollardError { + source: BollardError, + backtrace: Option, + }, + #[snafu(transparent)] + GlobPatternError { + source: glob::PatternError, + backtrace: Option, + }, + #[snafu(transparent)] + IoError { + source: io::Error, + backtrace: Option, + }, + #[snafu(transparent)] + PathPrefixError { + source: path::StripPrefixError, + backtrace: Option, + }, + #[snafu(transparent)] + SerdeJsonError { + source: serde_json::Error, + backtrace: Option, + }, + #[snafu(transparent)] + SerdeYamlError { + source: serde_yaml::Error, + backtrace: Option, }, - #[error("No known container names.")] - NoContainerNames, - #[error("No corresponding pod run found for pod job (hash: {pod_job_hash}).")] - NoMatchingPodRun { pod_job_hash: String }, - #[error("No tags found in provided container alternate image: {path}.")] - NoTagFoundInContainerAltImage { path: PathBuf }, - #[error(transparent)] - BollardError(#[from] BollardError), - #[error(transparent)] - GlobPatternError(#[from] glob::PatternError), - #[error(transparent)] - IoError(#[from] io::Error), - #[error(transparent)] - PathPrefixError(#[from] path::StripPrefixError), - #[error(transparent)] - SerdeJsonError(#[from] serde_json::Error), - #[error(transparent)] - SerdeYamlError(#[from] serde_yaml::Error), } /// A stable error API interface. -#[expect( - clippy::field_scoped_visibility_modifiers, - reason = "Allow access from `core::error`." -)] -#[derive(Error, Debug)] -pub struct OrcaError { - /// Type of error returned. - pub(crate) kind: Kind, -} +#[derive(Snafu)] +pub struct OrcaError(pub(crate) Kind); + impl OrcaError { /// Returns `true` if the error was caused by an invalid model annotation. pub const fn is_invalid_annotation(&self) -> bool { - matches!(self.kind, Kind::NoAnnotationFound { .. }) + matches!(self.0, Kind::NoAnnotationFound { .. }) } /// Returns `true` if the error was caused by querying a purged pod run. pub const fn is_purged_pod_run(&self) -> bool { - matches!(self.kind, Kind::NoMatchingPodRun { .. }) + matches!(self.0, Kind::NoMatchingPodRun { .. }) } } diff --git a/src/uniffi/orchestrator/docker.rs b/src/uniffi/orchestrator/docker.rs index 6124d4cb..5f997a2b 100644 --- a/src/uniffi/orchestrator/docker.rs +++ b/src/uniffi/orchestrator/docker.rs @@ -1,7 +1,7 @@ use crate::{ core::{orchestrator::docker::RE_IMAGE_TAG, util::get}, uniffi::{ - error::{Kind, OrcaError, Result}, + error::{OrcaError, Result, selector}, model::{Pod, PodJob, PodResult}, orchestrator::{ImageKind, Orchestrator, PodRun, RunInfo}, }, @@ -12,6 +12,7 @@ use bollard::{ image::{CreateImageOptions, ImportImageOptions}, }; use futures_util::stream::{StreamExt as _, TryStreamExt as _}; +use snafu::{OptionExt as _, futures::TryFutureExt as _}; use std::{collections::HashMap, path::PathBuf}; use tokio::{fs::File, runtime::Runtime}; use tokio_util::{ @@ -79,27 +80,32 @@ impl Orchestrator for LocalDockerOrchestrator { )?, ImageKind::Tarball(image_info) => { let location = namespace_lookup[&image_info.namespace].join(&image_info.path); - let byte_stream = FramedRead::new(File::open(&location).await?, BytesCodec::new()) - .map_err(|err| -> Result { - let resolved_error = Err::(err.into())?; - Ok(resolved_error) - }) - .map(|result| result.ok().map_or(Bytes::new(), BytesMut::freeze)); + let byte_stream = FramedRead::new( + File::open(&location) + .context(selector::InvalidFilepath { path: &location }) + .await?, + BytesCodec::new(), + ) + .map_err(|err| -> Result<()> { + Err::(err.into())?; // raise on error since we discard below + Ok(()) + }) + .map(|result| result.ok().map_or(Bytes::new(), BytesMut::freeze)); let mut stream = self.api .import_image_stream(ImportImageOptions::default(), byte_stream, None); let mut local_image = String::new(); while let Some(response) = stream.next().await { local_image = RE_IMAGE_TAG - .captures_iter(&response?.stream.ok_or(OrcaError::from( - Kind::EmptyResponseWhenLoadingContainerAltImage { + .captures_iter(&response?.stream.context( + selector::EmptyResponseWhenLoadingContainerAltImage { path: location.clone(), }, - ))?) + )?) .find_map(|x| x.name("image").map(|name| name.as_str().to_owned())) - .ok_or(OrcaError::from(Kind::NoTagFoundInContainerAltImage { + .context(selector::NoTagFoundInContainerAltImage { path: location.clone(), - }))?; + })?; } Self::prepare_container_start_inputs( namespace_lookup, @@ -185,9 +191,9 @@ impl Orchestrator for LocalDockerOrchestrator { .list_containers(HashMap::from([("label".to_owned(), labels)])) .await? .next() - .ok_or(OrcaError::from(Kind::NoMatchingPodRun { + .context(selector::NoMatchingPodRun { pod_job_hash: pod_run.pod_job.hash.clone(), - }))?; + })?; Ok(run_info) } async fn get_result(&self, pod_run: &PodRun) -> Result { @@ -202,11 +208,11 @@ impl Orchestrator for LocalDockerOrchestrator { pod_run.assigned_name.clone(), result_info.status, result_info.created, - result_info.terminated.ok_or(OrcaError::from( - Kind::InvalidPodResultTerminatedDatetime { + result_info + .terminated + .context(selector::InvalidPodResultTerminatedDatetime { pod_job_hash: pod_run.pod_job.hash.clone(), - }, - ))?, + })?, ) } } diff --git a/tests/error.rs b/tests/error.rs new file mode 100644 index 00000000..835a16b1 --- /dev/null +++ b/tests/error.rs @@ -0,0 +1,51 @@ +#![expect(missing_docs, clippy::expect_used, reason = "OK in tests.")] + +pub mod fixture; +use fixture::{NAMESPACE_LOOKUP_READ_ONLY, pod_job_style}; +use glob::glob; +use orcapod::uniffi::{ + error::{OrcaError, Result}, + orchestrator::{Orchestrator as _, docker::LocalDockerOrchestrator}, +}; +use serde_json; +use serde_yaml; +use std::{collections::HashMap, fmt, fs, path::PathBuf, result}; + +fn check>(value: result::Result) -> String { + let error: OrcaError = value.expect_err("Did not return an expected error.").into(); + format!("{error:?}") +} + +#[test] +fn external_bollard() -> Result<()> { + let orch = LocalDockerOrchestrator::new()?; + let mut pod_job = pod_job_style(&NAMESPACE_LOOKUP_READ_ONLY)?; + pod_job.pod.image = "nonexistent_image".to_owned(); + check(orch.start_blocking(&NAMESPACE_LOOKUP_READ_ONLY, &pod_job)); + Ok(()) +} + +#[test] +fn external_glob() { + check(glob("a**/b")); +} + +#[test] +fn external_io() { + check(fs::read_to_string("nonexistent_file.txt")); +} + +#[test] +fn external_path_prefix() { + check(PathBuf::from("/fake/path").strip_prefix("/missing/path")); +} + +#[test] +fn external_json() { + check(serde_json::from_str::>("{")); +} + +#[test] +fn external_yaml() { + check(serde_yaml::from_str::>(":")); +}