Unify dist error handling (#9659)

This came up when trying to improve the build error reporting.
Introduces `DistErrorKind` to avoid error variants for each case that
are only different in one line of the message.
This commit is contained in:
konsti 2024-12-06 02:54:14 +01:00 committed by GitHub
parent dc82a84841
commit 890fb10fa1
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 163 additions and 310 deletions

View file

@ -277,26 +277,12 @@ impl<'a> BuildContext for BuildDispatch<'a> {
.prepare(remote, &self.shared_state.in_flight)
.await
.map_err(|err| match err {
uv_installer::PrepareError::DownloadAndBuild(dist, chain, err) => {
uv_installer::PrepareError::Dist(kind, dist, chain, err) => {
debug_assert!(chain.is_empty());
let chain =
DerivationChainBuilder::from_resolution(resolution, (&*dist).into())
.unwrap_or_default();
uv_installer::PrepareError::DownloadAndBuild(dist, chain, err)
}
uv_installer::PrepareError::Download(dist, chain, err) => {
debug_assert!(chain.is_empty());
let chain =
DerivationChainBuilder::from_resolution(resolution, (&*dist).into())
.unwrap_or_default();
uv_installer::PrepareError::Download(dist, chain, err)
}
uv_installer::PrepareError::Build(dist, chain, err) => {
debug_assert!(chain.is_empty());
let chain =
DerivationChainBuilder::from_resolution(resolution, (&*dist).into())
.unwrap_or_default();
uv_installer::PrepareError::Build(dist, chain, err)
uv_installer::PrepareError::Dist(kind, dist, chain, err)
}
_ => err,
})?

View file

@ -1,7 +1,30 @@
use std::fmt::{Display, Formatter};
use uv_normalize::{ExtraName, GroupName, PackageName};
use uv_pep440::Version;
use version_ranges::Ranges;
/// The operation(s) that failed when reporting an error with a distribution.
#[derive(Debug)]
pub enum DistErrorKind {
Download,
DownloadAndBuild,
Build,
BuildBackend,
Read,
}
impl Display for DistErrorKind {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
DistErrorKind::Download => f.write_str("Failed to download"),
DistErrorKind::DownloadAndBuild => f.write_str("Failed to download and build"),
DistErrorKind::Build => f.write_str("Failed to build"),
DistErrorKind::BuildBackend => f.write_str("Failed to build"),
DistErrorKind::Read => f.write_str("Failed to read"),
}
}
}
/// A chain of derivation steps from the root package to the current package, to explain why a
/// package is included in the resolution.
#[derive(Debug, Default, Clone, PartialEq, Eq, Hash)]

View file

@ -54,8 +54,8 @@ pub use crate::any::*;
pub use crate::buildable::*;
pub use crate::cached::*;
pub use crate::dependency_metadata::*;
pub use crate::derivation::*;
pub use crate::diagnostic::*;
pub use crate::dist_error::*;
pub use crate::error::*;
pub use crate::file::*;
pub use crate::hash::*;
@ -77,8 +77,8 @@ mod any;
mod buildable;
mod cached;
mod dependency_metadata;
mod derivation;
mod diagnostic;
mod dist_error;
mod error;
mod file;
mod hash;
@ -553,6 +553,15 @@ impl Dist {
}
}
impl<'a> From<&'a Dist> for DistRef<'a> {
fn from(dist: &'a Dist) -> Self {
match dist {
Dist::Built(built) => DistRef::Built(built),
Dist::Source(source) => DistRef::Source(source),
}
}
}
impl<'a> From<&'a SourceDist> for DistRef<'a> {
fn from(dist: &'a SourceDist) -> Self {
DistRef::Source(dist)

View file

@ -9,8 +9,8 @@ use uv_cache::Cache;
use uv_configuration::BuildOptions;
use uv_distribution::{DistributionDatabase, LocalWheel};
use uv_distribution_types::{
BuildableSource, BuiltDist, CachedDist, DerivationChain, Dist, Hashed, Identifier, Name,
RemoteSource, SourceDist,
BuildableSource, CachedDist, DerivationChain, Dist, DistErrorKind, Hashed, Identifier, Name,
RemoteSource,
};
use uv_pep508::PackageName;
use uv_platform_tags::Tags;
@ -203,21 +203,10 @@ pub enum Error {
NoBuild(PackageName),
#[error("Using pre-built wheels is disabled, but attempted to use `{0}`")]
NoBinary(PackageName),
#[error("Failed to download `{0}`")]
Download(
Box<BuiltDist>,
DerivationChain,
#[source] uv_distribution::Error,
),
#[error("Failed to download and build `{0}`")]
DownloadAndBuild(
Box<SourceDist>,
DerivationChain,
#[source] uv_distribution::Error,
),
#[error("Failed to build `{0}`")]
Build(
Box<SourceDist>,
#[error("{0} `{1}`")]
Dist(
DistErrorKind,
Box<Dist>,
DerivationChain,
#[source] uv_distribution::Error,
),
@ -228,16 +217,17 @@ pub enum Error {
impl Error {
/// Create an [`Error`] from a distribution error.
fn from_dist(dist: Dist, cause: uv_distribution::Error) -> Self {
match dist {
Dist::Built(dist) => Self::Download(Box::new(dist), DerivationChain::default(), cause),
let kind = match &dist {
Dist::Built(_) => DistErrorKind::Download,
Dist::Source(dist) => {
if dist.is_local() {
Self::Build(Box::new(dist), DerivationChain::default(), cause)
DistErrorKind::Build
} else {
Self::DownloadAndBuild(Box::new(dist), DerivationChain::default(), cause)
}
DistErrorKind::DownloadAndBuild
}
}
};
Self::Dist(kind, Box::new(dist), DerivationChain::default(), cause)
}
}

View file

@ -4,7 +4,8 @@ pub use crate::source_tree::*;
pub use crate::sources::*;
pub use crate::specification::*;
pub use crate::unnamed::*;
use uv_distribution_types::{BuiltDist, DerivationChain, Dist, GitSourceDist, SourceDist};
use uv_distribution_types::{DerivationChain, Dist, DistErrorKind, GitSourceDist, SourceDist};
use uv_git::GitUrl;
use uv_pypi_types::{Requirement, RequirementSource};
@ -18,23 +19,10 @@ pub mod upgrade;
#[derive(Debug, thiserror::Error)]
pub enum Error {
#[error("Failed to download `{0}`")]
Download(
Box<BuiltDist>,
DerivationChain,
#[source] uv_distribution::Error,
),
#[error("Failed to download and build `{0}`")]
DownloadAndBuild(
Box<SourceDist>,
DerivationChain,
#[source] uv_distribution::Error,
),
#[error("Failed to build `{0}`")]
Build(
Box<SourceDist>,
#[error("{0} `{1}`")]
Dist(
DistErrorKind,
Box<Dist>,
DerivationChain,
#[source] uv_distribution::Error,
),
@ -53,13 +41,24 @@ impl Error {
/// Create an [`Error`] from a distribution error.
pub(crate) fn from_dist(dist: Dist, cause: uv_distribution::Error) -> Self {
match dist {
Dist::Built(dist) => Self::Download(Box::new(dist), DerivationChain::default(), cause),
Dist::Built(dist) => Self::Dist(
DistErrorKind::Download,
Box::new(Dist::Built(dist)),
DerivationChain::default(),
cause,
),
Dist::Source(dist) => {
if dist.is_local() {
Self::Build(Box::new(dist), DerivationChain::default(), cause)
let kind = if dist.is_local() {
DistErrorKind::Build
} else {
Self::DownloadAndBuild(Box::new(dist), DerivationChain::default(), cause)
}
DistErrorKind::DownloadAndBuild
};
Self::Dist(
kind,
Box::new(Dist::Source(dist)),
DerivationChain::default(),
cause,
)
}
}
}

View file

@ -10,8 +10,8 @@ use rustc_hash::FxHashMap;
use tracing::trace;
use uv_distribution_types::{
BuiltDist, DerivationChain, IndexCapabilities, IndexLocations, IndexUrl, InstalledDist,
SourceDist,
DerivationChain, Dist, DistErrorKind, IndexCapabilities, IndexLocations, IndexUrl,
InstalledDist,
};
use uv_normalize::{ExtraName, PackageName};
use uv_pep440::{LocalVersionSlice, Version};
@ -97,23 +97,10 @@ pub enum ResolveError {
#[error(transparent)]
ParsedUrl(#[from] uv_pypi_types::ParsedUrlError),
#[error("Failed to download `{0}`")]
Download(
Box<BuiltDist>,
DerivationChain,
#[source] Arc<uv_distribution::Error>,
),
#[error("Failed to download and build `{0}`")]
DownloadAndBuild(
Box<SourceDist>,
DerivationChain,
#[source] Arc<uv_distribution::Error>,
),
#[error("Failed to read `{0}`")]
Read(
Box<BuiltDist>,
#[error("{0} `{1}`")]
Dist(
DistErrorKind,
Box<Dist>,
DerivationChain,
#[source] Arc<uv_distribution::Error>,
),
@ -122,13 +109,6 @@ pub enum ResolveError {
#[error("Failed to read metadata from installed package `{0}`")]
ReadInstalled(Box<InstalledDist>, DerivationChain, #[source] anyhow::Error),
#[error("Failed to build `{0}`")]
Build(
Box<SourceDist>,
DerivationChain,
#[source] Arc<uv_distribution::Error>,
),
#[error(transparent)]
NoSolution(#[from] NoSolutionError),

View file

@ -23,10 +23,10 @@ use tracing::{debug, info, instrument, trace, warn, Level};
use uv_configuration::{Constraints, Overrides};
use uv_distribution::{ArchiveMetadata, DistributionDatabase};
use uv_distribution_types::{
BuiltDist, CompatibleDist, DerivationChain, Dist, DistributionMetadata, IncompatibleDist,
IncompatibleSource, IncompatibleWheel, IndexCapabilities, IndexLocations, IndexUrl,
InstalledDist, PythonRequirementKind, RemoteSource, ResolvedDist, ResolvedDistRef, SourceDist,
VersionOrUrlRef,
BuiltDist, CompatibleDist, DerivationChain, Dist, DistErrorKind, DistributionMetadata,
IncompatibleDist, IncompatibleSource, IncompatibleWheel, IndexCapabilities, IndexLocations,
IndexUrl, InstalledDist, PythonRequirementKind, RemoteSource, ResolvedDist, ResolvedDistRef,
SourceDist, VersionOrUrlRef,
};
use uv_git::GitResolver;
use uv_normalize::{ExtraName, GroupName, PackageName};
@ -957,35 +957,36 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
// TODO(charlie): Add derivation chain for URL dependencies. In practice, this isn't
// critical since we fetch URL dependencies _prior_ to invoking the resolver.
let chain = DerivationChain::default();
return Err(match &**dist {
let (kind, dist) = match &**dist {
Dist::Built(built_dist @ BuiltDist::Path(_)) => {
ResolveError::Read(Box::new(built_dist.clone()), chain, (*err).clone())
(DistErrorKind::Read, Dist::Built(built_dist.clone()))
}
Dist::Source(source_dist @ SourceDist::Path(_)) => {
ResolveError::Build(Box::new(source_dist.clone()), chain, (*err).clone())
(DistErrorKind::Build, Dist::Source(source_dist.clone()))
}
Dist::Source(source_dist @ SourceDist::Directory(_)) => {
ResolveError::Build(Box::new(source_dist.clone()), chain, (*err).clone())
(DistErrorKind::Build, Dist::Source(source_dist.clone()))
}
Dist::Built(built_dist) => {
ResolveError::Download(Box::new(built_dist.clone()), chain, (*err).clone())
(DistErrorKind::Download, Dist::Built(built_dist.clone()))
}
Dist::Source(source_dist) => {
if source_dist.is_local() {
ResolveError::Build(
Box::new(source_dist.clone()),
chain,
(*err).clone(),
)
(DistErrorKind::Build, Dist::Source(source_dist.clone()))
} else {
ResolveError::DownloadAndBuild(
Box::new(source_dist.clone()),
chain,
(*err).clone(),
(
DistErrorKind::DownloadAndBuild,
Dist::Source(source_dist.clone()),
)
}
}
});
};
return Err(ResolveError::Dist(
kind,
Box::new(dist),
chain,
(*err).clone(),
));
}
};
@ -1397,45 +1398,36 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
MetadataResponse::Error(dist, err) => {
let chain = DerivationChainBuilder::from_state(id, version, pubgrub)
.unwrap_or_default();
return Err(match &**dist {
Dist::Built(built_dist @ BuiltDist::Path(_)) => ResolveError::Read(
Box::new(built_dist.clone()),
chain,
(*err).clone(),
),
Dist::Source(source_dist @ SourceDist::Path(_)) => ResolveError::Build(
Box::new(source_dist.clone()),
chain,
(*err).clone(),
),
Dist::Source(source_dist @ SourceDist::Directory(_)) => {
ResolveError::Build(
Box::new(source_dist.clone()),
chain,
(*err).clone(),
)
let (kind, dist) = match &**dist {
Dist::Built(built_dist @ BuiltDist::Path(_)) => {
(DistErrorKind::Read, Dist::Built(built_dist.clone()))
}
Dist::Source(source_dist @ SourceDist::Path(_)) => {
(DistErrorKind::Build, Dist::Source(source_dist.clone()))
}
Dist::Source(source_dist @ SourceDist::Directory(_)) => {
(DistErrorKind::Build, Dist::Source(source_dist.clone()))
}
Dist::Built(built_dist) => {
(DistErrorKind::Download, Dist::Built(built_dist.clone()))
}
Dist::Built(built_dist) => ResolveError::Download(
Box::new(built_dist.clone()),
chain,
(*err).clone(),
),
Dist::Source(source_dist) => {
if source_dist.is_local() {
ResolveError::Build(
Box::new(source_dist.clone()),
chain,
(*err).clone(),
)
(DistErrorKind::Build, Dist::Source(source_dist.clone()))
} else {
ResolveError::DownloadAndBuild(
Box::new(source_dist.clone()),
chain,
(*err).clone(),
(
DistErrorKind::DownloadAndBuild,
Dist::Source(source_dist.clone()),
)
}
}
});
};
return Err(ResolveError::Dist(
kind,
Box::new(dist),
chain,
(*err).clone(),
));
}
};

View file

@ -1,20 +1,17 @@
use std::str::FromStr;
use std::sync::LazyLock;
use std::sync::{Arc, LazyLock};
use owo_colors::OwoColorize;
use rustc_hash::FxHashMap;
use version_ranges::Ranges;
use uv_distribution_types::{BuiltDist, DerivationChain, DerivationStep, Name, SourceDist};
use uv_distribution_types::{DerivationChain, DerivationStep, Dist, DistErrorKind, Name};
use uv_normalize::PackageName;
use uv_pep440::Version;
use uv_resolver::SentinelRange;
use crate::commands::pip;
type Error = Box<dyn std::error::Error + Send + Sync>;
/// Static map of common package name typos or misconfigurations to their correct package names.
static SUGGESTIONS: LazyLock<FxHashMap<PackageName, PackageName>> = LazyLock::new(|| {
let suggestions: Vec<(String, String)> =
serde_json::from_str(include_str!("suggestions.json")).unwrap();
@ -75,72 +72,31 @@ impl OperationDiagnostic {
}
None
}
pip::operations::Error::Resolve(uv_resolver::ResolveError::DownloadAndBuild(
pip::operations::Error::Resolve(uv_resolver::ResolveError::Dist(
kind,
dist,
chain,
err,
)) => {
download_and_build(dist, &chain, Box::new(err));
dist_error(kind, dist, &chain, err);
None
}
pip::operations::Error::Resolve(uv_resolver::ResolveError::Download(
pip::operations::Error::Requirements(uv_requirements::Error::Dist(
kind,
dist,
chain,
err,
)) => {
download(dist, &chain, Box::new(err));
dist_error(kind, dist, &chain, Arc::new(err));
None
}
pip::operations::Error::Resolve(uv_resolver::ResolveError::Build(dist, chain, err)) => {
build(dist, &chain, Box::new(err));
None
}
pip::operations::Error::Requirements(uv_requirements::Error::DownloadAndBuild(
pip::operations::Error::Prepare(uv_installer::PrepareError::Dist(
kind,
dist,
chain,
err,
)) => {
download_and_build(dist, &chain, Box::new(err));
None
}
pip::operations::Error::Requirements(uv_requirements::Error::Download(
dist,
chain,
err,
)) => {
download(dist, &chain, Box::new(err));
None
}
pip::operations::Error::Requirements(uv_requirements::Error::Build(
dist,
chain,
err,
)) => {
build(dist, &chain, Box::new(err));
None
}
pip::operations::Error::Prepare(uv_installer::PrepareError::DownloadAndBuild(
dist,
chain,
err,
)) => {
download_and_build(dist, &chain, Box::new(err));
None
}
pip::operations::Error::Prepare(uv_installer::PrepareError::Download(
dist,
chain,
err,
)) => {
download(dist, &chain, Box::new(err));
None
}
pip::operations::Error::Prepare(uv_installer::PrepareError::Build(
dist,
chain,
err,
)) => {
build(dist, &chain, Box::new(err));
dist_error(kind, dist, &chain, Arc::new(err));
None
}
pip::operations::Error::Requirements(err) => {
@ -158,26 +114,31 @@ impl OperationDiagnostic {
}
}
/// Render a remote source distribution build failure with a help message.
pub(crate) fn download_and_build(sdist: Box<SourceDist>, chain: &DerivationChain, cause: Error) {
/// Render a distribution failure (read, download or build) with a help message.
pub(crate) fn dist_error(
kind: DistErrorKind,
dist: Box<Dist>,
chain: &DerivationChain,
cause: Arc<uv_distribution::Error>,
) {
#[derive(Debug, miette::Diagnostic, thiserror::Error)]
#[error("Failed to download and build `{sdist}`")]
#[error("{kind} `{dist}`")]
#[diagnostic()]
struct Diagnostic {
sdist: Box<SourceDist>,
kind: DistErrorKind,
dist: Box<Dist>,
#[source]
cause: Error,
cause: Arc<uv_distribution::Error>,
#[help]
help: Option<String>,
}
let report = miette::Report::new(Diagnostic {
help: SUGGESTIONS
.get(sdist.name())
let help = SUGGESTIONS
.get(dist.name())
.map(|suggestion| {
format!(
"`{}` is often confused for `{}` Did you mean to install `{}` instead?",
sdist.name().cyan(),
dist.name().cyan(),
suggestion.cyan(),
suggestion.cyan(),
)
@ -186,85 +147,14 @@ pub(crate) fn download_and_build(sdist: Box<SourceDist>, chain: &DerivationChain
if chain.is_empty() {
None
} else {
Some(format_chain(sdist.name(), sdist.version(), chain))
Some(format_chain(dist.name(), dist.version(), chain))
}
}),
sdist,
cause,
});
anstream::eprint!("{report:?}");
}
/// Render a remote binary distribution download failure with a help message.
pub(crate) fn download(wheel: Box<BuiltDist>, chain: &DerivationChain, cause: Error) {
#[derive(Debug, miette::Diagnostic, thiserror::Error)]
#[error("Failed to download `{wheel}`")]
#[diagnostic()]
struct Diagnostic {
wheel: Box<BuiltDist>,
#[source]
cause: Error,
#[help]
help: Option<String>,
}
let report = miette::Report::new(Diagnostic {
help: SUGGESTIONS
.get(wheel.name())
.map(|suggestion| {
format!(
"`{}` is often confused for `{}` Did you mean to install `{}` instead?",
wheel.name().cyan(),
suggestion.cyan(),
suggestion.cyan(),
)
})
.or_else(|| {
if chain.is_empty() {
None
} else {
Some(format_chain(wheel.name(), Some(wheel.version()), chain))
}
}),
wheel,
cause,
});
anstream::eprint!("{report:?}");
}
/// Render a local source distribution build failure with a help message.
pub(crate) fn build(sdist: Box<SourceDist>, chain: &DerivationChain, cause: Error) {
#[derive(Debug, miette::Diagnostic, thiserror::Error)]
#[error("Failed to build `{sdist}`")]
#[diagnostic()]
struct Diagnostic {
sdist: Box<SourceDist>,
#[source]
cause: Error,
#[help]
help: Option<String>,
}
let report = miette::Report::new(Diagnostic {
help: SUGGESTIONS
.get(sdist.name())
.map(|suggestion| {
format!(
"`{}` is often confused for `{}` Did you mean to install `{}` instead?",
sdist.name().cyan(),
suggestion.cyan(),
suggestion.cyan(),
)
})
.or_else(|| {
if chain.is_empty() {
None
} else {
Some(format_chain(sdist.name(), sdist.version(), chain))
}
}),
sdist,
kind,
dist,
cause,
help,
});
anstream::eprint!("{report:?}");
}

View file

@ -469,28 +469,12 @@ pub(crate) async fn install(
.map_err(Error::from)
.map_err(|err| match err {
// Attach resolution context to the error.
Error::Prepare(uv_installer::PrepareError::Download(dist, chain, err)) => {
Error::Prepare(uv_installer::PrepareError::Dist(kind, dist, chain, err)) => {
debug_assert!(chain.is_empty());
let chain =
DerivationChainBuilder::from_resolution(resolution, (&*dist).into())
.unwrap_or_default();
Error::Prepare(uv_installer::PrepareError::Download(dist, chain, err))
}
Error::Prepare(uv_installer::PrepareError::Build(dist, chain, err)) => {
debug_assert!(chain.is_empty());
let chain =
DerivationChainBuilder::from_resolution(resolution, (&*dist).into())
.unwrap_or_default();
Error::Prepare(uv_installer::PrepareError::Build(dist, chain, err))
}
Error::Prepare(uv_installer::PrepareError::DownloadAndBuild(dist, chain, err)) => {
debug_assert!(chain.is_empty());
let chain =
DerivationChainBuilder::from_resolution(resolution, (&*dist).into())
.unwrap_or_default();
Error::Prepare(uv_installer::PrepareError::DownloadAndBuild(
dist, chain, err,
))
Error::Prepare(uv_installer::PrepareError::Dist(kind, dist, chain, err))
}
_ => err,
})?;