Surface errors for offline `--find-links` URLs (#1271)

## Summary

Ensures that if the user passes `--no-index` with `--find-links`, and
we're unable to access the HTML page, we show an appropriate hint.

crates/puffin-client/src/flat_index.rs

@@ -32,7 +32,48 @@ pub enum FlatIndexError {
     FindLinksUrl(Url, #[source] Error),
 }
 
-type FlatIndexEntry = (DistFilename, File, IndexUrl);
+#[derive(Debug, Default, Clone)]
+pub struct FlatIndexEntries {
+    /// The list of `--find-links` entries.
+    entries: Vec<(DistFilename, File, IndexUrl)>,
+    /// Whether any `--find-links` entries could not be resolved due to a lack of network
+    /// connectivity.
+    offline: bool,
+}
+
+impl FlatIndexEntries {
+    /// Create a [`FlatIndexEntries`] from a list of `--find-links` entries.
+    fn from_entries(entries: Vec<(DistFilename, File, IndexUrl)>) -> Self {
+        Self {
+            entries,
+            offline: false,
+        }
+    }
+
+    /// Create a [`FlatIndexEntries`] to represent an offline `--find-links` entry.
+    fn offline() -> Self {
+        Self {
+            entries: Vec::new(),
+            offline: true,
+        }
+    }
+
+    /// Extend this list of `--find-links` entries with another list.
+    fn extend(&mut self, other: Self) {
+        self.entries.extend(other.entries);
+        self.offline |= other.offline;
+    }
+
+    /// Return the number of `--find-links` entries.
+    fn len(&self) -> usize {
+        self.entries.len()
+    }
+
+    /// Return `true` if there are no `--find-links` entries.
+    fn is_empty(&self) -> bool {
+        self.entries.is_empty()
+    }
+}
 
 /// A client for reading distributions from `--find-links` entries (either local directories or
 /// remote HTML indexes).
@@ -53,7 +94,7 @@ impl<'a> FlatIndexClient<'a> {
     pub async fn fetch(
         &self,
         indexes: impl Iterator<Item = &FlatIndexLocation>,
-    ) -> Result<Vec<FlatIndexEntry>, FlatIndexError> {
+    ) -> Result<FlatIndexEntries, FlatIndexError> {
         let mut fetches = futures::stream::iter(indexes)
             .map(|index| async move {
                 let entries = match index {
@@ -74,11 +115,11 @@ impl<'a> FlatIndexClient<'a> {
                         index
                     );
                 }
-                Ok::<Vec<FlatIndexEntry>, FlatIndexError>(entries)
+                Ok::<FlatIndexEntries, FlatIndexError>(entries)
             })
             .buffered(16);
 
-        let mut results = Vec::new();
+        let mut results = FlatIndexEntries::default();
         while let Some(entries) = fetches.next().await.transpose()? {
             results.extend(entries);
         }
@@ -86,7 +127,7 @@ impl<'a> FlatIndexClient<'a> {
     }
 
     /// Read a flat remote index from a `--find-links` URL.
-    async fn read_from_url(&self, url: &Url) -> Result<Vec<FlatIndexEntry>, Error> {
+    async fn read_from_url(&self, url: &Url) -> Result<FlatIndexEntries, Error> {
         let cache_entry = self.cache.entry(
             CacheBucket::FlatIndex,
             "html",
@@ -142,15 +183,9 @@ impl<'a> FlatIndexClient<'a> {
                 parse_simple_response,
             )
             .await;
-        let files = match response {
+        match response {
-            Ok(files) => files,
+            Ok(files) => {
-            Err(CachedClientError::Client(err)) if matches!(err.kind(), ErrorKind::Offline(_)) => {
+                let files = files
-                warn!("Remote `--find-links` entry was not available in the cache: {url}");
-                vec![]
-            }
-            Err(err) => return Err(err.into()),
-        };
-        Ok(files
                     .into_iter()
                     .filter_map(|file| {
                         Some((
@@ -159,11 +194,18 @@ impl<'a> FlatIndexClient<'a> {
                             IndexUrl::Url(url.clone()),
                         ))
                     })
-            .collect())
+                    .collect();
+                Ok(FlatIndexEntries::from_entries(files))
+            }
+            Err(CachedClientError::Client(err)) if matches!(err.kind(), ErrorKind::Offline(_)) => {
+                Ok(FlatIndexEntries::offline())
+            }
+            Err(err) => Err(err.into()),
+        }
     }
 
     /// Read a flat remote index from a `--find-links` directory.
-    fn read_from_directory(path: &PathBuf) -> Result<Vec<FlatIndexEntry>, std::io::Error> {
+    fn read_from_directory(path: &PathBuf) -> Result<FlatIndexEntries, std::io::Error> {
         // Absolute paths are required for the URL conversion.
         let path = fs_err::canonicalize(path)?;
 
@@ -203,28 +245,36 @@ impl<'a> FlatIndexClient<'a> {
             };
             dists.push((filename, file, IndexUrl::Pypi));
         }
-        Ok(dists)
+        Ok(FlatIndexEntries::from_entries(dists))
     }
 }
 
 /// A set of [`PrioritizedDistribution`] from a `--find-links` entry, indexed by [`PackageName`]
 /// and [`Version`].
 #[derive(Debug, Clone, Default)]
-pub struct FlatIndex(FxHashMap<PackageName, FlatDistributions>);
+pub struct FlatIndex {
+    /// The list of [`FlatDistributions`] from the `--find-links` entries, indexed by package name.
+    index: FxHashMap<PackageName, FlatDistributions>,
+    /// Whether any `--find-links` entries could not be resolved due to a lack of network
+    /// connectivity.
+    offline: bool,
+}
 
 impl FlatIndex {
     /// Collect all files from a `--find-links` target into a [`FlatIndex`].
     #[instrument(skip_all)]
-    pub fn from_entries(entries: Vec<FlatIndexEntry>, tags: &Tags) -> Self {
+    pub fn from_entries(entries: FlatIndexEntries, tags: &Tags) -> Self {
-        let mut flat_index = FxHashMap::default();
-
         // Collect compatible distributions.
-        for (filename, file, index) in entries {
+        let mut index = FxHashMap::default();
-            let distributions = flat_index.entry(filename.name().clone()).or_default();
+        for (filename, file, url) in entries.entries {
-            Self::add_file(distributions, file, filename, tags, index);
+            let distributions = index.entry(filename.name().clone()).or_default();
+            Self::add_file(distributions, file, filename, tags, url);
         }
 
-        Self(flat_index)
+        // Collect offline entries.
+        let offline = entries.offline;
+
+        Self { index, offline }
     }
 
     fn add_file(
@@ -277,7 +327,12 @@ impl FlatIndex {
 
     /// Get the [`FlatDistributions`] for the given package name.
     pub fn get(&self, package_name: &PackageName) -> Option<&FlatDistributions> {
-        self.0.get(package_name)
+        self.index.get(package_name)
+    }
+
+    /// Returns `true` if there are any offline `--find-links` entries.
+    pub fn offline(&self) -> bool {
+        self.offline
     }
 }
 
@@ -297,8 +352,8 @@ impl FlatDistributions {
 }
 
 impl IntoIterator for FlatDistributions {
-    type IntoIter = std::collections::btree_map::IntoIter<Version, PrioritizedDistribution>;
     type Item = (Version, PrioritizedDistribution);
+    type IntoIter = std::collections::btree_map::IntoIter<Version, PrioritizedDistribution>;
 
     fn into_iter(self) -> Self::IntoIter {
         self.0.into_iter()

crates/puffin-resolver/src/resolver/provider.rs

@@ -158,6 +158,8 @@ impl<'a, Context: BuildContext + Send + Sync> ResolverProvider
                 puffin_client::ErrorKind::NoIndex(_) => {
                     if let Some(flat_index) = self.flat_index.get(package_name).cloned() {
                         Ok(VersionsResponse::Found(VersionMap::from(flat_index)))
+                    } else if self.flat_index.offline() {
+                        Ok(VersionsResponse::Offline)
                     } else {
                         Ok(VersionsResponse::NoIndex)
                     }

crates/puffin/tests/pip_compile.rs

@@ -3235,8 +3235,6 @@ fn offline_find_links() -> Result<()> {
     );
 
     // Resolve with `--offline`, `--find-links`, and `--no-index`.
-    // TODO(charlie): This should indicate that the network was disabled, but we don't "know" that
-    // the `--find-links` lookup failed.
     puffin_snapshot!(context.compile()
             .arg("requirements.in")
             .arg("--find-links")
@@ -3249,8 +3247,10 @@ fn offline_find_links() -> Result<()> {
 
     ----- stderr -----
       × No solution found when resolving dependencies:
-      ╰─▶ Because tqdm was not found in the provided package locations and you
+      ╰─▶ Because tqdm was not found in the cache and you require tqdm, we can
-          require tqdm, we can conclude that the requirements are unsatisfiable.
+          conclude that the requirements are unsatisfiable.
+
+          hint: Packages were unavailable because the network was disabled
     "###
     );