Skip to content

Commit

Permalink
Add proper error handling to the Source trait
Browse files Browse the repository at this point in the history
  • Loading branch information
Michael-F-Bryan committed Jun 20, 2023
1 parent 917e7e4 commit 3f09d8d
Show file tree
Hide file tree
Showing 12 changed files with 241 additions and 85 deletions.
12 changes: 6 additions & 6 deletions lib/cli/src/commands/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ use wasmer_registry::Package;
use wasmer_wasix::{
bin_factory::BinaryPackage,
runners::{MappedDirectory, Runner},
runtime::{package_loader::PackageLoader, resolver::PackageSpecifier},
runtime::{
package_loader::PackageLoader,
resolver::{PackageSpecifier, QueryError},
},
WasiError,
};
use wasmer_wasix::{
Expand Down Expand Up @@ -117,10 +120,7 @@ impl Run {
// something that displays progress
let monitoring_runtime = MonitoringRuntime::new(runtime, pb.clone());

let target = self
.input
.resolve_target(&monitoring_runtime, &pb)
.with_context(|| format!("Unable to resolve \"{}\"", self.input))?;
let target = self.input.resolve_target(&monitoring_runtime, &pb)?;

pb.finish_and_clear();

Expand Down Expand Up @@ -841,7 +841,7 @@ impl wasmer_wasix::runtime::resolver::Source for MonitoringSource {
async fn query(
&self,
package: &PackageSpecifier,
) -> Result<Vec<wasmer_wasix::runtime::resolver::PackageSummary>, Error> {
) -> Result<Vec<wasmer_wasix::runtime::resolver::PackageSummary>, QueryError> {
self.progress.set_message(format!("Looking up {package}"));
self.inner.query(package).await
}
Expand Down
16 changes: 13 additions & 3 deletions lib/wasix/src/bin_factory/binary_package.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::sync::Arc;

use anyhow::Context;
use derivative::*;
use once_cell::sync::OnceCell;
use semver::Version;
Expand All @@ -9,7 +10,7 @@ use webc::{compat::SharedBytes, Container};
use crate::{
runtime::{
module_cache::ModuleHash,
resolver::{PackageId, PackageInfo, PackageSpecifier},
resolver::{PackageId, PackageInfo, PackageSpecifier, ResolveError},
},
Runtime,
};
Expand Down Expand Up @@ -100,11 +101,20 @@ impl BinaryPackage {
runtime: &dyn Runtime,
) -> Result<Self, anyhow::Error> {
let source = runtime.source();
let root_summary = source.latest(specifier).await?;
let root_summary =
source
.latest(specifier)
.await
.map_err(|error| ResolveError::Registry {
package: specifier.clone(),
error,
})?;
let root = runtime.package_loader().load(&root_summary).await?;
let id = root_summary.package_id();

let resolution = crate::runtime::resolver::resolve(&id, &root_summary.pkg, &source).await?;
let resolution = crate::runtime::resolver::resolve(&id, &root_summary.pkg, &source)
.await
.context("Dependency resolution failed")?;
let pkg = runtime
.package_loader()
.load_package_tree(&root, &resolution)
Expand Down
12 changes: 7 additions & 5 deletions lib/wasix/src/runtime/resolver/filesystem_source.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use anyhow::{Context, Error};
use anyhow::Context;
use webc::compat::Container;

use crate::runtime::resolver::{
DistributionInfo, PackageInfo, PackageSpecifier, PackageSummary, Source, WebcHash,
DistributionInfo, PackageInfo, PackageSpecifier, PackageSummary, QueryError, Source, WebcHash,
};

/// A [`Source`] that knows how to query files on the filesystem.
Expand All @@ -12,15 +12,15 @@ pub struct FileSystemSource {}
#[async_trait::async_trait]
impl Source for FileSystemSource {
#[tracing::instrument(level = "debug", skip_all, fields(%package))]
async fn query(&self, package: &PackageSpecifier) -> Result<Vec<PackageSummary>, Error> {
async fn query(&self, package: &PackageSpecifier) -> Result<Vec<PackageSummary>, QueryError> {
let path = match package {
PackageSpecifier::Path(path) => path.canonicalize().with_context(|| {
format!(
"Unable to get the canonical form for \"{}\"",
path.display()
)
})?,
_ => return Ok(Vec::new()),
_ => return Err(QueryError::Unsupported),
};

// FIXME: These two operations will block
Expand All @@ -32,8 +32,10 @@ impl Source for FileSystemSource {
let url = crate::runtime::resolver::utils::url_from_file_path(&path)
.ok_or_else(|| anyhow::anyhow!("Unable to turn \"{}\" into a URL", path.display()))?;

let pkg = PackageInfo::from_manifest(container.manifest())
.context("Unable to determine the package's metadata")?;
let summary = PackageSummary {
pkg: PackageInfo::from_manifest(container.manifest())?,
pkg,
dist: DistributionInfo {
webc: url,
webc_sha256,
Expand Down
14 changes: 10 additions & 4 deletions lib/wasix/src/runtime/resolver/in_memory_source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::{
use anyhow::{Context, Error};
use semver::Version;

use crate::runtime::resolver::{PackageSpecifier, PackageSummary, Source};
use crate::runtime::resolver::{PackageSpecifier, PackageSummary, QueryError, Source};

/// A [`Source`] that tracks packages in memory.
///
Expand Down Expand Up @@ -88,7 +88,7 @@ impl InMemorySource {
#[async_trait::async_trait]
impl Source for InMemorySource {
#[tracing::instrument(level = "debug", skip_all, fields(%package))]
async fn query(&self, package: &PackageSpecifier) -> Result<Vec<PackageSummary>, Error> {
async fn query(&self, package: &PackageSpecifier) -> Result<Vec<PackageSummary>, QueryError> {
match package {
PackageSpecifier::Registry { full_name, version } => {
match self.packages.get(full_name) {
Expand All @@ -106,12 +106,18 @@ impl Source for InMemorySource {
.collect::<Vec<_>>(),
);

if matches.is_empty() {
return Err(QueryError::NoMatches {
archived_versions: Vec::new(),
});
}

Ok(matches)
}
None => Ok(Vec::new()),
None => Err(QueryError::NotFound),
}
}
PackageSpecifier::Url(_) | PackageSpecifier::Path(_) => Ok(Vec::new()),
PackageSpecifier::Url(_) | PackageSpecifier::Path(_) => Err(QueryError::Unsupported),
}
}
}
Expand Down
10 changes: 9 additions & 1 deletion lib/wasix/src/runtime/resolver/inputs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,15 @@ impl FromStr for PackageSpecifier {
impl Display for PackageSpecifier {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
PackageSpecifier::Registry { full_name, version } => write!(f, "{full_name}@{version}"),
PackageSpecifier::Registry { full_name, version } => {
write!(f, "{full_name}")?;

if !version.comparators.is_empty() {
write!(f, "@{version}")?;
}

Ok(())
}
PackageSpecifier::Url(url) => Display::fmt(url, f),
PackageSpecifier::Path(path) => write!(f, "{}", path.display()),
}
Expand Down
6 changes: 3 additions & 3 deletions lib/wasix/src/runtime/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ pub use self::{
Command, Dependency, DistributionInfo, PackageInfo, PackageSpecifier, PackageSummary,
WebcHash,
},
multi_source::MultiSource,
multi_source::{MultiSource, MultiSourceStrategy},
outputs::{
DependencyGraph, Edge, ItemLocation, Node, PackageId, Resolution,
ResolvedFileSystemMapping, ResolvedPackage,
},
resolve::resolve,
source::Source,
resolve::{resolve, ResolveError},
source::{QueryError, Source},
wapm_source::WapmSource,
web_source::WebSource,
};
73 changes: 63 additions & 10 deletions lib/wasix/src/runtime/resolver/multi_source.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
use std::sync::Arc;

use anyhow::Error;

use crate::runtime::resolver::{PackageSpecifier, PackageSummary, Source};
use crate::runtime::resolver::{PackageSpecifier, PackageSummary, QueryError, Source};

/// A [`Source`] that works by querying multiple [`Source`]s in succession.
///
/// # Error Handling
///
/// A [`Source`] implementation can return certain non-fatal errors and,
/// depending on the [`MultiSourceStrategy`], the [`MultiSource`] can choose to
/// deal with it in different ways. Sometimes
///
///
/// The first [`Source`] to return one or more [`Summaries`][PackageSummary]
/// will be treated as the canonical source for that [`Dependency`][dep] and no
/// further [`Source`]s will be queried.
Expand All @@ -14,37 +19,85 @@ use crate::runtime::resolver::{PackageSpecifier, PackageSummary, Source};
#[derive(Debug, Clone)]
pub struct MultiSource {
sources: Vec<Arc<dyn Source + Send + Sync>>,
strategy: MultiSourceStrategy,
}

impl MultiSource {
pub const fn new() -> Self {
MultiSource {
sources: Vec::new(),
strategy: MultiSourceStrategy::default(),
}
}

pub fn add_source(&mut self, source: impl Source + Send + Sync + 'static) -> &mut Self {
self.add_shared_source(Arc::new(source));
self
self.add_shared_source(Arc::new(source))
}

pub fn add_shared_source(&mut self, source: Arc<dyn Source + Send + Sync>) -> &mut Self {
self.sources.push(source);
self
}

/// Override the strategy used when a [`Source`] returns a non-fatal error.
pub fn with_strategy(self, strategy: MultiSourceStrategy) -> Self {
MultiSource { strategy, ..self }
}
}

#[async_trait::async_trait]
impl Source for MultiSource {
#[tracing::instrument(level = "debug", skip_all, fields(%package))]
async fn query(&self, package: &PackageSpecifier) -> Result<Vec<PackageSummary>, Error> {
async fn query(&self, package: &PackageSpecifier) -> Result<Vec<PackageSummary>, QueryError> {
for source in &self.sources {
let result = source.query(package).await?;
if !result.is_empty() {
return Ok(result);
match source.query(package).await {
Ok(summaries) => return Ok(summaries),
Err(QueryError::Unsupported) if self.strategy.continue_if_unsupported => continue,
Err(QueryError::NotFound) if self.strategy.continue_if_not_found => continue,
Err(QueryError::NoMatches { .. }) if self.strategy.continue_if_no_matches => {
continue
}
Err(e) => return Err(e),
}
}

anyhow::bail!("Unable to find any packages that satisfy the query")
Err(QueryError::NotFound)
}
}

#[derive(Debug, Clone, PartialEq, Eq)]
#[non_exhaustive]
pub struct MultiSourceStrategy {
/// If encountered, treat [`QueryError::Unsupported`] as a non-fatal error
/// and query the next [`Source`] in turn.
///
/// This flag is **enabled** by default.
pub continue_if_unsupported: bool,
/// If encountered, treat [`QueryError::NotFound`] as a non-fatal error and
/// query the next [`Source`] in turn.
///
/// This flag is **enabled** by default and can be used to let earlier
/// [`Source`]s "override" later ones.
pub continue_if_not_found: bool,
/// If encountered, treat [`QueryError::NoMatches`] as a non-fatal error and
/// query the next [`Source`] in turn.
///
/// This flag is **disabled** by default.
pub continue_if_no_matches: bool,
}

impl MultiSourceStrategy {
pub const fn default() -> Self {
MultiSourceStrategy {
continue_if_unsupported: true,
continue_if_not_found: true,
continue_if_no_matches: true,
}
}
}

impl Default for MultiSourceStrategy {
fn default() -> Self {
MultiSourceStrategy::default()
}
}
39 changes: 31 additions & 8 deletions lib/wasix/src/runtime/resolver/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ use semver::Version;

use crate::runtime::resolver::{
outputs::{Edge, Node},
DependencyGraph, ItemLocation, PackageId, PackageInfo, PackageSummary, Resolution,
ResolvedPackage, Source,
DependencyGraph, ItemLocation, PackageId, PackageInfo, PackageSpecifier, PackageSummary,
QueryError, Resolution, ResolvedPackage, Source,
};

use super::ResolvedFileSystemMapping;
Expand All @@ -33,8 +33,12 @@ pub async fn resolve(

#[derive(Debug, thiserror::Error)]
pub enum ResolveError {
#[error(transparent)]
Registry(anyhow::Error),
#[error("{}", registry_error_message(.package))]
Registry {
package: PackageSpecifier,
#[source]
error: QueryError,
},
#[error("Dependency cycle detected: {}", print_cycle(_0))]
Cycle(Vec<PackageId>),
#[error(
Expand All @@ -47,6 +51,21 @@ pub enum ResolveError {
},
}

fn registry_error_message(specifier: &PackageSpecifier) -> String {
match specifier {
PackageSpecifier::Registry { full_name, version } if version.comparators.is_empty() => {
format!("Unable to find \"{full_name}\" in the registry")
}
PackageSpecifier::Registry { full_name, version } => {
format!("Unable to find \"{full_name}@{version}\" in the registry")
}
PackageSpecifier::Url(url) => format!("Unable to resolve \"{url}\""),
PackageSpecifier::Path(path) => {
format!("Unable to load \"{}\" from disk", path.display())
}
}
}

impl ResolveError {
pub fn as_cycle(&self) -> Option<&[PackageId]> {
match self {
Expand Down Expand Up @@ -117,10 +136,14 @@ async fn discover_dependencies(
// doing this more rigorously, we would be narrowing the version
// down using existing requirements and trying to reuse the same
// dependency when possible.
let dep_summary = source
.latest(&dep.pkg)
.await
.map_err(ResolveError::Registry)?;
let dep_summary =
source
.latest(&dep.pkg)
.await
.map_err(|error| ResolveError::Registry {
package: dep.pkg.clone(),
error,
})?;
let dep_id = dep_summary.package_id();

let PackageSummary { pkg, dist } = dep_summary;
Expand Down
Loading

0 comments on commit 3f09d8d

Please sign in to comment.