diff --git a/Cargo.lock b/Cargo.lock index 140d6c85e..a31fbbb81 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2541,6 +2541,7 @@ dependencies = [ "pypi-types", "rayon", "tempfile", + "thiserror", "tokio", "tokio-util", "tracing", diff --git a/crates/install-wheel-rs/src/lib.rs b/crates/install-wheel-rs/src/lib.rs index e24bbbc2e..6545b6b6f 100644 --- a/crates/install-wheel-rs/src/lib.rs +++ b/crates/install-wheel-rs/src/lib.rs @@ -65,6 +65,10 @@ pub enum Error { Pep440, #[error("Invalid direct_url.json")] DirectUrlJson(#[from] serde_json::Error), + #[error("No .dist-info directory found")] + MissingDistInfo, + #[error("Multiple .dist-info directories found: {0}")] + MultipleDistInfo(String), } impl Error { @@ -84,7 +88,7 @@ impl Error { pub fn find_dist_info<'a, T: Copy>( filename: &WheelFilename, files: impl Iterator, -) -> Result<(T, &'a str), String> { +) -> Result<(T, &'a str), Error> { let metadatas: Vec<_> = files .filter_map(|(payload, path)| { let (dist_info_dir, file) = path.split_once('/')?; @@ -102,17 +106,16 @@ pub fn find_dist_info<'a, T: Copy>( .collect(); let (payload, dist_info_dir) = match metadatas[..] { [] => { - return Err("no .dist-info directory".to_string()); + return Err(Error::MissingDistInfo); } [(payload, path)] => (payload, path), _ => { - return Err(format!( - "multiple .dist-info directories: {}", + return Err(Error::MultipleDistInfo( metadatas .into_iter() .map(|(_, dist_info_dir)| dist_info_dir.to_string()) .collect::>() - .join(", ") + .join(", "), )); } }; diff --git a/crates/install-wheel-rs/src/wheel.rs b/crates/install-wheel-rs/src/wheel.rs index 3e73e2507..b6019e791 100644 --- a/crates/install-wheel-rs/src/wheel.rs +++ b/crates/install-wheel-rs/src/wheel.rs @@ -930,8 +930,7 @@ pub fn install_wheel( ZipArchive::new(reader).map_err(|err| Error::from_zip_error("(index)".to_string(), err))?; debug!(name = name.as_ref(), "Getting wheel metadata"); - let dist_info_prefix = find_dist_info(filename, archive.file_names().map(|name| (name, name))) - .map_err(Error::InvalidWheel)? + let dist_info_prefix = find_dist_info(filename, archive.file_names().map(|name| (name, name)))? .1 .to_string(); let (name, _version) = read_metadata(&dist_info_prefix, &mut archive)?; diff --git a/crates/puffin-client/src/client.rs b/crates/puffin-client/src/client.rs index fa368926d..cf56e74a6 100644 --- a/crates/puffin-client/src/client.rs +++ b/crates/puffin-client/src/client.rs @@ -301,8 +301,7 @@ impl RegistryClient { .iter() .enumerate() .filter_map(|(idx, e)| Some((idx, e.entry().filename().as_str().ok()?))), - ) - .map_err(|err| Error::InvalidDistInfo(filename.clone(), err))?; + )?; // Read the contents of the METADATA file let mut contents = Vec::new(); diff --git a/crates/puffin-client/src/error.rs b/crates/puffin-client/src/error.rs index 0c32f1a01..c62fb4487 100644 --- a/crates/puffin-client/src/error.rs +++ b/crates/puffin-client/src/error.rs @@ -13,6 +13,9 @@ pub enum Error { #[error(transparent)] UrlParseError(#[from] url::ParseError), + #[error(transparent)] + InstallWheel(#[from] install_wheel_rs::Error), + #[error("{0} isn't available locally, but making network requests to registries was banned.")] NoIndex(String), diff --git a/crates/puffin-client/src/remote_metadata.rs b/crates/puffin-client/src/remote_metadata.rs index 2afd68e18..5c85e4377 100644 --- a/crates/puffin-client/src/remote_metadata.rs +++ b/crates/puffin-client/src/remote_metadata.rs @@ -76,8 +76,7 @@ pub(crate) async fn wheel_metadata_from_remote_zip( .iter() .enumerate() .filter_map(|(idx, e)| Some(((idx, e), e.entry().filename().as_str().ok()?))), - ) - .map_err(|err| Error::InvalidDistInfo(filename.clone(), err))?; + )?; let offset = metadata_entry.header_offset(); let size = metadata_entry.entry().compressed_size() diff --git a/crates/puffin-distribution/Cargo.toml b/crates/puffin-distribution/Cargo.toml index 992d94774..22e734399 100644 --- a/crates/puffin-distribution/Cargo.toml +++ b/crates/puffin-distribution/Cargo.toml @@ -24,6 +24,7 @@ bytesize = { workspace = true } fs-err = { workspace = true } rayon = { workspace = true } tempfile = { workspace = true } +thiserror = { workspace = true } tokio = { workspace = true } tokio-util = { workspace = true, features = ["compat"] } tracing = { workspace = true } diff --git a/crates/puffin-distribution/src/download.rs b/crates/puffin-distribution/src/download.rs index 09920afbc..a6b64b73f 100644 --- a/crates/puffin-distribution/src/download.rs +++ b/crates/puffin-distribution/src/download.rs @@ -1,7 +1,7 @@ use std::path::PathBuf; use std::str::FromStr; -use anyhow::{format_err, Context, Result}; +use anyhow::{Context, Result}; use tempfile::TempDir; use zip::ZipArchive; @@ -10,6 +10,8 @@ use distribution_types::{Dist, RemoteSource}; use install_wheel_rs::find_dist_info; use pypi_types::Metadata21; +use crate::error::Error; + /// A downloaded wheel that's stored in-memory. #[derive(Debug)] pub struct InMemoryWheel { @@ -86,12 +88,14 @@ impl std::fmt::Display for SourceDistDownload { impl InMemoryWheel { /// Read the [`Metadata21`] from a wheel. - pub fn read_dist_info(&self) -> Result { + pub fn read_dist_info(&self) -> Result { let mut archive = ZipArchive::new(std::io::Cursor::new(&self.buffer))?; - let filename = self.filename()?; + let filename = self + .filename() + .map_err(|err| Error::FilenameParse(self.dist.to_string(), err))?; let dist_info_dir = find_dist_info(&filename, archive.file_names().map(|name| (name, name))) - .map_err(|err| format_err!("Invalid wheel {filename}: {err}"))? + .map_err(|err| Error::DistInfo(self.dist.to_string(), err))? .1; let dist_info = std::io::read_to_string(archive.by_name(&format!("{dist_info_dir}/METADATA"))?)?; @@ -101,12 +105,14 @@ impl InMemoryWheel { impl DiskWheel { /// Read the [`Metadata21`] from a wheel. - pub fn read_dist_info(&self) -> Result { + pub fn read_dist_info(&self) -> Result { let mut archive = ZipArchive::new(fs_err::File::open(&self.path)?)?; - let filename = self.filename()?; + let filename = self + .filename() + .map_err(|err| Error::FilenameParse(self.dist.to_string(), err))?; let dist_info_dir = find_dist_info(&filename, archive.file_names().map(|name| (name, name))) - .map_err(|err| format_err!("Invalid wheel {filename}: {err}"))? + .map_err(|err| Error::DistInfo(self.dist.to_string(), err))? .1; let dist_info = std::io::read_to_string(archive.by_name(&format!("{dist_info_dir}/METADATA"))?)?; @@ -116,7 +122,7 @@ impl DiskWheel { impl WheelDownload { /// Read the [`Metadata21`] from a wheel. - pub fn read_dist_info(&self) -> Result { + pub fn read_dist_info(&self) -> Result { match self { WheelDownload::InMemory(wheel) => wheel.read_dist_info(), WheelDownload::Disk(wheel) => wheel.read_dist_info(), diff --git a/crates/puffin-distribution/src/error.rs b/crates/puffin-distribution/src/error.rs new file mode 100644 index 000000000..91eeb53c7 --- /dev/null +++ b/crates/puffin-distribution/src/error.rs @@ -0,0 +1,13 @@ +#[derive(thiserror::Error, Debug)] +pub enum Error { + #[error(transparent)] + IO(#[from] std::io::Error), + #[error(transparent)] + PypiTypes(#[from] pypi_types::Error), + #[error(transparent)] + Zip(#[from] zip::result::ZipError), + #[error("Unable to read .dist-info directory for: {0}")] + DistInfo(String, #[source] install_wheel_rs::Error), + #[error("Unable to parse wheel filename for: {0}")] + FilenameParse(String, #[source] anyhow::Error), +} diff --git a/crates/puffin-distribution/src/fetcher.rs b/crates/puffin-distribution/src/fetcher.rs index 2079a1099..c38909608 100644 --- a/crates/puffin-distribution/src/fetcher.rs +++ b/crates/puffin-distribution/src/fetcher.rs @@ -18,6 +18,7 @@ use puffin_git::{GitSource, GitUrl}; use puffin_traits::BuildContext; use pypi_types::Metadata21; +use crate::error::Error; use crate::reporter::Facade; use crate::{DiskWheel, Download, InMemoryWheel, Reporter, SourceDistDownload, WheelDownload}; @@ -52,7 +53,7 @@ impl<'a> Fetcher<'a> { } /// Return the [`Metadata21`] for a distribution, if it exists in the cache. - pub fn find_metadata(&self, dist: &Dist, tags: &Tags) -> Result> { + pub fn find_metadata(&self, dist: &Dist, tags: &Tags) -> Result, Error> { self.find_in_cache(dist, tags) .map(|wheel| wheel.read_dist_info()) .transpose() @@ -77,10 +78,14 @@ impl<'a> Fetcher<'a> { // Fetch the distribution, then read the metadata (for built distributions), or build // the distribution and _then_ read the metadata (for source distributions). dist => match self.fetch_dist(dist, client).await? { - Download::Wheel(wheel) => wheel.read_dist_info(), + Download::Wheel(wheel) => { + let metadata = wheel.read_dist_info()?; + Ok(metadata) + } Download::SourceDist(sdist) => { let wheel = self.build_sdist(sdist, build_context).await?; - wheel.read_dist_info() + let metadata = wheel.read_dist_info()?; + Ok(metadata) } }, } diff --git a/crates/puffin-distribution/src/lib.rs b/crates/puffin-distribution/src/lib.rs index 95456738d..2f293a972 100644 --- a/crates/puffin-distribution/src/lib.rs +++ b/crates/puffin-distribution/src/lib.rs @@ -4,6 +4,7 @@ pub use crate::reporter::Reporter; pub use crate::unzip::Unzip; mod download; +mod error; mod fetcher; mod reporter; mod unzip;