Add proper error handling to the Source trait

This commit is contained in:
Michael-F-Bryan
2023-06-20 16:17:41 +08:00
parent 07e454cd15
commit 2def425f8b
12 changed files with 241 additions and 85 deletions

View File

@ -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.
@ -12,7 +12,7 @@ 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!(
@ -20,7 +20,7 @@ impl Source for FileSystemSource {
path.display()
)
})?,
_ => return Ok(Vec::new()),
_ => return Err(QueryError::Unsupported),
};
// FIXME: These two operations will block
@ -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,

View File

@ -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.
///
@ -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) {
@ -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),
}
}
}

View File

@ -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()),
}

View File

@ -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,
};

View File

@ -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.
@ -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()
}
}

View File

@ -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;
@ -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(
@ -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 {
@ -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;

View File

@ -1,6 +1,4 @@
use std::fmt::Debug;
use anyhow::Error;
use std::fmt::{Debug, Display};
use crate::runtime::resolver::{PackageSpecifier, PackageSummary};
@ -12,22 +10,23 @@ pub trait Source: Sync + Debug {
///
/// # Assumptions
///
/// It is not an error if there are no package versions that may satisfy
/// the dependency, even if the [`Source`] doesn't know of a package
/// with that name.
/// If this method returns a successful result, it is guaranteed that there
/// will be at least one [`PackageSummary`], otherwise implementations
/// should return [`QueryError::NotFound`] or [`QueryError::NoMatches`].
///
/// [dep]: crate::runtime::resolver::Dependency
/// [reg]: crate::runtime::resolver::Registry
async fn query(&self, package: &PackageSpecifier) -> Result<Vec<PackageSummary>, Error>;
async fn query(&self, package: &PackageSpecifier) -> Result<Vec<PackageSummary>, QueryError>;
/// Run [`Source::query()`] and get the [`PackageSummary`] for the latest
/// version.
async fn latest(&self, pkg: &PackageSpecifier) -> Result<PackageSummary, Error> {
async fn latest(&self, pkg: &PackageSpecifier) -> Result<PackageSummary, QueryError> {
let candidates = self.query(pkg).await?;
candidates
.into_iter()
.max_by(|left, right| left.pkg.version.cmp(&right.pkg.version))
.ok_or_else(|| Error::msg("Couldn't find a package version satisfying that constraint"))
.ok_or(QueryError::NoMatches {
archived_versions: Vec::new(),
})
}
}
@ -37,7 +36,60 @@ where
D: std::ops::Deref<Target = S> + Debug + Send + Sync,
S: Source + ?Sized + Send + Sync + 'static,
{
async fn query(&self, package: &PackageSpecifier) -> Result<Vec<PackageSummary>, Error> {
async fn query(&self, package: &PackageSpecifier) -> Result<Vec<PackageSummary>, QueryError> {
(**self).query(package).await
}
}
#[derive(Debug)]
pub enum QueryError {
Unsupported,
NotFound,
NoMatches {
archived_versions: Vec<semver::Version>,
},
Other(anyhow::Error),
}
impl From<anyhow::Error> for QueryError {
fn from(value: anyhow::Error) -> Self {
QueryError::Other(value)
}
}
impl Display for QueryError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
QueryError::Unsupported => {
f.write_str("This type of package specifier isn't supported")
}
QueryError::NotFound => f.write_str("Not found"),
QueryError::NoMatches { archived_versions } => match archived_versions.as_slice() {
[] => f.write_str(
"The package was found, but no published versions matched the constraint",
),
[version] => write!(
f,
"The only version satisfying the constraint, {version}, is archived"
),
[first, rest @ ..] => {
let num_others = rest.len();
write!(
f,
"Unable to satisfy the request. Version {first}, and {num_others} are all archived"
)
}
},
QueryError::Other(e) => Display::fmt(e, f),
}
}
}
impl std::error::Error for QueryError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
match self {
QueryError::Other(e) => Some(&**e),
QueryError::Unsupported | QueryError::NotFound | QueryError::NoMatches { .. } => None,
}
}
}

View File

@ -13,7 +13,8 @@ use webc::metadata::Manifest;
use crate::{
http::{HttpClient, HttpRequest, USER_AGENT},
runtime::resolver::{
DistributionInfo, PackageInfo, PackageSpecifier, PackageSummary, Source, WebcHash,
DistributionInfo, PackageInfo, PackageSpecifier, PackageSummary, QueryError, Source,
WebcHash,
},
};
@ -147,10 +148,10 @@ impl WapmSource {
#[async_trait::async_trait]
impl Source for WapmSource {
#[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 (full_name, version_constraint) = match package {
PackageSpecifier::Registry { full_name, version } => (full_name, version),
_ => return Ok(Vec::new()),
_ => return Err(QueryError::Unsupported),
};
let response: WapmWebQuery = self.lookup_package(full_name).await?;
@ -159,13 +160,24 @@ impl Source for WapmSource {
let versions = match response.data.get_package {
Some(WapmWebQueryGetPackage { versions }) => versions,
None => return Ok(Vec::new()),
None => return Err(QueryError::NotFound),
};
let mut archived_versions = Vec::new();
for pkg_version in versions {
tracing::trace!(?pkg_version, "checking package version");
let version = Version::parse(&pkg_version.version)?;
tracing::trace!(?pkg_version, "Checking a package version");
let version = match Version::parse(&pkg_version.version) {
Ok(v) => v,
Err(e) => {
tracing::debug!(
pkg.version = pkg_version.version.as_str(),
error = &e as &dyn std::error::Error,
"Skipping a version because it doesn't have a valid version numer",
);
continue;
}
};
if pkg_version.is_archived {
tracing::debug!(
@ -191,25 +203,10 @@ impl Source for WapmSource {
}
if summaries.is_empty() {
match archived_versions.as_slice() {
[] => {
// looks like this package couldn't be satisfied at all.
}
[version] => {
anyhow::bail!(
"The only version satisfying the constraint, {version}, is archived"
);
}
[first, rest @ ..] => {
let num_others = rest.len();
anyhow::bail!(
"Unable to satisfy the request, {first}, and {num_others} are all archived"
);
}
}
Err(QueryError::NoMatches { archived_versions })
} else {
Ok(summaries)
}
Ok(summaries)
}
}

View File

@ -16,7 +16,8 @@ use webc::compat::Container;
use crate::{
http::{HttpClient, HttpRequest},
runtime::resolver::{
DistributionInfo, PackageInfo, PackageSpecifier, PackageSummary, Source, WebcHash,
DistributionInfo, PackageInfo, PackageSpecifier, PackageSummary, QueryError, Source,
WebcHash,
},
};
@ -234,10 +235,10 @@ impl WebSource {
#[async_trait::async_trait]
impl Source for WebSource {
#[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 url = match package {
PackageSpecifier::Url(url) => url,
_ => return Ok(Vec::new()),
_ => return Err(QueryError::Unsupported),
};
let local_path = self
@ -246,12 +247,16 @@ impl Source for WebSource {
.context("Unable to get the locally cached file")?;
// FIXME: this will block
let webc_sha256 = WebcHash::for_file(&local_path)?;
let webc_sha256 = WebcHash::for_file(&local_path)
.with_context(|| format!("Unable to hash \"{}\"", local_path.display()))?;
// Note: We want to use Container::from_disk() rather than the bytes
// our HTTP client gave us because then we can use memory-mapped files
let container = Container::from_disk(&local_path)?;
let pkg = PackageInfo::from_manifest(container.manifest())?;
let container = Container::from_disk(&local_path)
.with_context(|| format!("Unable to load \"{}\"", local_path.display()))?;
let pkg = PackageInfo::from_manifest(container.manifest())
.context("Unable to determine the package's metadata")?;
let dist = DistributionInfo {
webc: url.clone(),
webc_sha256,