Improve error messages and make cache failures non fatal (#333)

crates/puffin-build/src/lib.rs

@@ -169,11 +169,15 @@ impl SourceDistributionBuilder {
             let resolved_requirements = build_context
                 .resolve(&build_system.requires)
                 .await
-                .map_err(|err| Error::RequirementsInstall("build-system.requires", err))?;
+                .map_err(|err| {
+                    Error::RequirementsInstall("build-system.requires (resolve)", err)
+                })?;
             build_context
                 .install(&resolved_requirements, &venv)
                 .await
-                .map_err(|err| Error::RequirementsInstall("build-system.requires", err))?;
+                .map_err(|err| {
+                    Error::RequirementsInstall("build-system.requires (install)", err)
+                })?;
             build_system.requires.clone()
         } else {
             // TODO(konstin): Resolve those once globally and cache per puffin invocation
@@ -185,11 +189,11 @@ impl SourceDistributionBuilder {
             let resolved_requirements = build_context
                 .resolve(&requirements)
                 .await
-                .map_err(|err| Error::RequirementsInstall("setup.py build", err))?;
+                .map_err(|err| Error::RequirementsInstall("setup.py build (resolve)", err))?;
             build_context
                 .install(&resolved_requirements, &venv)
                 .await
-                .map_err(|err| Error::RequirementsInstall("setup.py build", err))?;
+                .map_err(|err| Error::RequirementsInstall("setup.py build (install)", err))?;
             requirements
         };
 
@@ -458,12 +462,12 @@ async fn create_pep517_build_environment(
         let resolved_requirements = build_context
             .resolve(&requirements)
             .await
-            .map_err(|err| Error::RequirementsInstall("build-system.requires", err))?;
+            .map_err(|err| Error::RequirementsInstall("build-system.requires (resolve)", err))?;
 
         build_context
             .install(&resolved_requirements, venv)
             .await
-            .map_err(|err| Error::RequirementsInstall("build-system.requires", err))?;
+            .map_err(|err| Error::RequirementsInstall("build-system.requires (install)", err))?;
     }
     Ok(())
 }

crates/puffin-installer/src/plan.rs

@@ -1,6 +1,6 @@
 use std::path::Path;
 
-use anyhow::Result;
+use anyhow::{Context, Result};
 use tracing::debug;
 
 use pep508_rs::{Requirement, VersionOrUrl};
@@ -34,11 +34,12 @@ impl PartitionedRequirements {
         venv: &Virtualenv,
     ) -> Result<Self> {
         // Index all the already-installed packages in site-packages.
-        let mut site_packages = SitePackages::try_from_executable(venv)?;
+        let mut site_packages =
+            SitePackages::try_from_executable(venv).context("Failed to list installed packages")?;
 
         // Index all the already-downloaded wheels in the cache.
-        let registry_index = RegistryIndex::try_from_directory(cache)?;
+        let registry_index = RegistryIndex::try_from_directory(cache);
-        let url_index = UrlIndex::try_from_directory(cache)?;
+        let url_index = UrlIndex::try_from_directory(cache);
 
         let mut local = vec![];
         let mut remote = vec![];

crates/puffin-installer/src/registry_index.rs

@@ -1,7 +1,8 @@
 use std::collections::HashMap;
 use std::path::Path;
 
-use anyhow::Result;
+use fs_err as fs;
+use tracing::warn;
 
 use puffin_distribution::CachedDistribution;
 use puffin_normalize::PackageName;
@@ -14,24 +15,48 @@ pub struct RegistryIndex(HashMap<PackageName, CachedDistribution>);
 
 impl RegistryIndex {
     /// Build an index of cached distributions from a directory.
-    pub fn try_from_directory(path: &Path) -> Result<Self> {
+    pub fn try_from_directory(path: &Path) -> Self {
         let mut index = HashMap::new();
 
         let cache = WheelCache::new(path);
         let Ok(dir) = cache.read_dir(CacheShard::Registry) else {
-            return Ok(Self(index));
+            return Self(index);
         };
 
         for entry in dir {
-            let entry = entry?;
+            let (path, file_type) =
-            if entry.file_type()?.is_dir() {
+                match entry.and_then(|entry| Ok((entry.path(), entry.file_type()?))) {
-                if let Some(dist_info) = CachedDistribution::try_from_path(&entry.path())? {
+                    Ok((path, file_type)) => (path, file_type),
-                    index.insert(dist_info.name().clone(), dist_info);
+                    Err(err) => {
+                        warn!(
+                            "Failed to read entry of cache at {}: {}",
+                            path.display(),
+                            err
+                        );
+                        continue;
+                    }
+                };
+            if file_type.is_dir() {
+                match CachedDistribution::try_from_path(&path) {
+                    Ok(None) => {}
+                    Ok(Some(dist_info)) => {
+                        index.insert(dist_info.name().clone(), dist_info);
+                    }
+                    Err(err) => {
+                        warn!("Invalid cache entry at {}, removing. {err}", path.display());
+                        let result = fs::remove_dir_all(&path);
+                        if let Err(err) = result {
+                            warn!(
+                                "Failed to remove invalid cache entry at {}: {err}",
+                                path.display()
+                            );
+                        }
+                    }
                 }
             }
         }
 
-        Ok(Self(index))
+        Self(index)
     }
 
     /// Returns a distribution from the index, if it exists.

crates/puffin-installer/src/url_index.rs

@@ -1,7 +1,7 @@
 use std::path::{Path, PathBuf};
 
-use anyhow::Result;
 use fxhash::FxHashMap;
+use tracing::warn;
 use url::Url;
 
 use puffin_distribution::{CachedDistribution, RemoteDistributionRef};
@@ -16,17 +16,27 @@ pub(crate) struct UrlIndex(FxHashMap<String, PathBuf>);
 
 impl UrlIndex {
     /// Build an index of cached distributions from a directory.
-    pub(crate) fn try_from_directory(path: &Path) -> Result<Self> {
+    pub(crate) fn try_from_directory(path: &Path) -> Self {
         let mut index = FxHashMap::default();
 
         let cache = WheelCache::new(path);
         let Ok(dir) = cache.read_dir(CacheShard::Url) else {
-            return Ok(Self(index));
+            return Self(index);
         };
 
         for entry in dir {
-            let entry = entry?;
+            let (file_type, entry) = match entry.and_then(|entry| Ok((entry.file_type()?, entry))) {
-            if entry.file_type()?.is_dir() {
+                Ok((path, file_type)) => (path, file_type),
+                Err(err) => {
+                    warn!(
+                        "Failed to read entry of cache at {}: {}",
+                        path.display(),
+                        err
+                    );
+                    continue;
+                }
+            };
+            if file_type.is_dir() {
                 let file_name = entry.file_name();
                 let Some(filename) = file_name.to_str() else {
                     continue;
@@ -35,7 +45,7 @@ impl UrlIndex {
             }
         }
 
-        Ok(Self(index))
+        Self(index)
     }
 
     /// Returns a distribution from the index, if it exists.