feat(outdated): warn about packages skipped due to registry errors (#34974)

Closes #32179

Author: bartlomieju

cli/jsr.rs

@@ -15,6 +15,7 @@ use deno_semver::package::PackageReq;
 
 use crate::args::jsr_url;
 use crate::file_fetcher::CliFileFetcher;
+use crate::npm::PackageInfoLoadError;
 
 /// This is similar to a subset of `JsrCacheResolver` which fetches rather than
 /// just reads the cache. Keep in sync!
@@ -24,7 +25,8 @@ pub struct JsrFetchResolver {
   /// The `module_graph` field of the version infos should be forcibly absent.
   /// It can be large and we don't want to store it.
   info_by_nv: DashMap<PackageNv, Option<Arc<JsrPackageVersionInfo>>>,
-  info_by_name: DashMap<String, Option<Arc<JsrPackageInfo>>>,
+  info_by_name:
+    DashMap<String, Result<Arc<JsrPackageInfo>, Arc<PackageInfoLoadError>>>,
   file_fetcher: Arc<CliFileFetcher>,
   jsr_version_resolver: Arc<JsrVersionResolver>,
 }
@@ -116,34 +118,57 @@ impl JsrFetchResolver {
       .ok()?;
     let info = serde_json::from_slice::<JsrPackageInfo>(&file.source).ok()?;
     let info = Arc::new(info);
-    self
-      .info_by_name
-      .insert(name.to_string(), Some(info.clone()));
+    self.info_by_name.insert(name.to_string(), Ok(info.clone()));
     Some(info)
   }
 
   fn meta_url(&self, name: &str) -> Option<deno_core::url::Url> {
     jsr_url().join(&format!("{}/meta.json", name)).ok()
   }
 
-  // todo(dsherret): this should return error messages and only `None` when the package
-  // doesn't exist
   pub async fn package_info(&self, name: &str) -> Option<Arc<JsrPackageInfo>> {
+    self.package_info_with_reason(name).await.ok()
+  }
+
+  /// Like [`Self::package_info`], but preserves the reason the fetch failed
+  /// so callers can surface it instead of silently treating the package as
+  /// having no available versions.
+  pub async fn package_info_with_reason(
+    &self,
+    name: &str,
+  ) -> Result<Arc<JsrPackageInfo>, Arc<PackageInfoLoadError>> {
     if let Some(info) = self.info_by_name.get(name) {
       return info.value().clone();
     }
-    let fetch_package_info = || async {
-      let meta_url = self.meta_url(name)?;
-      let file = self
-        .file_fetcher
-        .fetch_bypass_permissions(&meta_url)
+    let result =
+      self
+        .fetch_package_info(name)
         .await
-        .ok()?;
-      serde_json::from_slice::<JsrPackageInfo>(&file.source).ok()
-    };
-    let info = fetch_package_info().await.map(Arc::new);
-    self.info_by_name.insert(name.to_string(), info.clone());
-    info
+        .map(Arc::new)
+        .map_err(|reason| {
+          Arc::new(PackageInfoLoadError {
+            registry_url: jsr_url().to_string(),
+            reason,
+          })
+        });
+    self.info_by_name.insert(name.to_string(), result.clone());
+    result
+  }
+
+  async fn fetch_package_info(
+    &self,
+    name: &str,
+  ) -> Result<JsrPackageInfo, String> {
+    let meta_url = self
+      .meta_url(name)
+      .ok_or_else(|| format!("invalid package name: {name}"))?;
+    let file = self
+      .file_fetcher
+      .fetch_bypass_permissions(&meta_url)
+      .await
+      .map_err(|e| format!("{e:#}"))?;
+    serde_json::from_slice::<JsrPackageInfo>(&file.source)
+      .map_err(|e| format!("failed to parse package metadata: {e}"))
   }
 
   pub async fn package_version_info(

cli/npm.rs

@@ -240,10 +240,22 @@ fn decompress_gzip(compressed: Vec<u8>) -> Result<Vec<u8>, JsErrorBox> {
   Ok(decompressed)
 }
 
+/// Why fetching package metadata for a single package failed. Used to surface
+/// actionable diagnostics (e.g. in `deno outdated`) instead of silently
+/// dropping packages that live on unreachable or unauthorized registries.
+#[derive(Clone, Debug)]
+pub struct PackageInfoLoadError {
+  /// The registry URL the metadata was being fetched from.
+  pub registry_url: String,
+  /// A human readable description of what went wrong.
+  pub reason: String,
+}
+
 #[derive(Debug)]
 pub struct NpmFetchResolver {
   nv_by_req: DashMap<PackageReq, Option<PackageNv>>,
-  info_by_name: DashMap<String, Option<Arc<NpmPackageInfo>>>,
+  info_by_name:
+    DashMap<String, Result<Arc<NpmPackageInfo>, Arc<PackageInfoLoadError>>>,
   file_fetcher: Arc<CliFileFetcher>,
   npmrc: Arc<ResolvedNpmRc>,
   version_resolver: Arc<NpmVersionResolver>,
@@ -293,18 +305,44 @@ impl NpmFetchResolver {
   }
 
   pub async fn package_info(&self, name: &str) -> Option<Arc<NpmPackageInfo>> {
+    self.package_info_with_reason(name).await.ok()
+  }
+
+  /// Like [`Self::package_info`], but preserves the reason the fetch failed
+  /// (e.g. an HTTP 401 from a private registry) so callers can surface it
+  /// instead of silently treating the package as having no available versions.
+  pub async fn package_info_with_reason(
+    &self,
+    name: &str,
+  ) -> Result<Arc<NpmPackageInfo>, Arc<PackageInfoLoadError>> {
     if let Some(info) = self.info_by_name.get(name) {
       return info.value().clone();
     }
-    // todo(#27198): use RegistryInfoProvider instead
-    let fetch_package_info = || async {
-      let info_url = deno_npm_cache::get_package_url(&self.npmrc, name);
-      let registry_config = self.npmrc.get_registry_config(name);
-      // TODO(bartlomieju): this should error out, not use `.ok()`.
-      let maybe_auth_header =
-        deno_npm_cache::maybe_auth_header_value_for_npm_registry(
-          registry_config,
-        )
+    let registry_url = self.npmrc.get_registry_url(name).to_string();
+    let result =
+      self
+        .fetch_package_info(name)
+        .await
+        .map(Arc::new)
+        .map_err(|reason| {
+          Arc::new(PackageInfoLoadError {
+            registry_url: registry_url.clone(),
+            reason,
+          })
+        });
+    self.info_by_name.insert(name.to_string(), result.clone());
+    result
+  }
+
+  // todo(#27198): use RegistryInfoProvider instead
+  async fn fetch_package_info(
+    &self,
+    name: &str,
+  ) -> Result<NpmPackageInfo, String> {
+    let info_url = deno_npm_cache::get_package_url(&self.npmrc, name);
+    let registry_config = self.npmrc.get_registry_config(name);
+    let maybe_auth_header =
+      deno_npm_cache::maybe_auth_header_value_for_npm_registry(registry_config)
         .map_err(AnyError::from)
         .and_then(|value| match value {
           Some(value) => Ok(Some((
@@ -313,17 +351,14 @@ impl NpmFetchResolver {
           ))),
           None => Ok(None),
         })
-        .ok()?;
-      let file = self
-        .file_fetcher
-        .fetch_bypass_permissions_with_maybe_auth(&info_url, maybe_auth_header)
-        .await
-        .ok()?;
-      serde_json::from_slice::<NpmPackageInfo>(&file.source).ok()
-    };
-    let info = fetch_package_info().await.map(Arc::new);
-    self.info_by_name.insert(name.to_string(), info.clone());
-    info
+        .map_err(|e| format!("{e:#}"))?;
+    let file = self
+      .file_fetcher
+      .fetch_bypass_permissions_with_maybe_auth(&info_url, maybe_auth_header)
+      .await
+      .map_err(|e| format!("{e:#}"))?;
+    serde_json::from_slice::<NpmPackageInfo>(&file.source)
+      .map_err(|e| format!("failed to parse package metadata: {e}"))
   }
 
   pub fn applicable_version_infos<'a>(

cli/tools/pm/deps.rs

@@ -50,6 +50,7 @@ use crate::module_loader::ModuleLoadPreparer;
 use crate::npm::CliNpmInstaller;
 use crate::npm::CliNpmResolver;
 use crate::npm::NpmFetchResolver;
+use crate::npm::PackageInfoLoadError;
 use crate::util::progress_bar::ProgressBar;
 use crate::util::sync::AtomicFlag;
 
@@ -453,6 +454,10 @@ where
 pub struct PackageLatestVersion {
   pub semver_compatible: Option<PackageNv>,
   pub latest: Option<PackageNv>,
+  /// Set when fetching the package's metadata failed (e.g. an unreachable or
+  /// unauthorized private registry). The package is dropped from the outdated
+  /// table, so this is used to warn the user instead of skipping silently.
+  pub fetch_error: Option<Arc<PackageInfoLoadError>>,
 }
 
 pub struct DepManager {
@@ -739,9 +744,13 @@ impl DepManager {
               .await
               .ok()
               .flatten();
-            let info =
-              self.npm_fetch_resolver.package_info(&semver_req.name).await;
-            let latest = info
+            let info_result = self
+              .npm_fetch_resolver
+              .package_info_with_reason(&semver_req.name)
+              .await;
+            let fetch_error = info_result.as_ref().err().cloned();
+            let latest = info_result
+              .ok()
               .and_then(|info| {
                 let version_resolver =
                   self.npm_version_resolver.get_for_package(&info);
@@ -781,6 +790,7 @@ impl DepManager {
             PackageLatestVersion {
               latest,
               semver_compatible,
+              fetch_error,
             }
           }
           .boxed_local(),
@@ -795,9 +805,13 @@ impl DepManager {
               .await
               .ok()
               .flatten();
-            let info =
-              self.jsr_fetch_resolver.package_info(&semver_req.name).await;
-            let latest = info
+            let info_result = self
+              .jsr_fetch_resolver
+              .package_info_with_reason(&semver_req.name)
+              .await;
+            let fetch_error = info_result.as_ref().err().cloned();
+            let latest = info_result
+              .ok()
               .and_then(|info| {
                 let version_resolver = self
                   .jsr_fetch_resolver
@@ -831,6 +845,7 @@ impl DepManager {
             PackageLatestVersion {
               latest,
               semver_compatible,
+              fetch_error,
             }
           }
           .boxed_local(),

cli/tools/pm/outdated/mod.rs

@@ -29,6 +29,14 @@ use crate::file_fetcher::CreateCliFileFetcherOptions;
 use crate::file_fetcher::create_cli_file_fetcher;
 use crate::jsr::JsrFetchResolver;
 use crate::npm::NpmFetchResolver;
+use crate::npm::PackageInfoLoadError;
+
+/// Packages whose metadata could not be fetched (e.g. unreachable or
+/// unauthorized private registries) are dropped from the update check. They are
+/// collected here, keyed and deduplicated by `(kind, name)`, so we can warn the
+/// user instead of skipping them silently.
+type SkippedPackages =
+  std::collections::BTreeMap<(DepKind, StackString), Arc<PackageInfoLoadError>>;
 
 #[derive(Debug, PartialEq, Eq, PartialOrd, Ord)]
 struct OutdatedPackage {
@@ -129,11 +137,14 @@ fn print_outdated(
 ) -> Result<(), AnyError> {
   let mut outdated = Vec::new();
   let mut seen = std::collections::BTreeSet::new();
+  let mut skipped = SkippedPackages::new();
   for (dep_id, resolved, latest_versions) in
     deps.deps_with_resolved_latest_versions()
   {
     let dep = deps.get_dep(dep_id);
 
+    collect_skipped(&mut skipped, dep.kind, &dep.req.name, &latest_versions);
+
     let Some(resolved) = resolved else { continue };
 
     let latest = {
@@ -171,12 +182,75 @@ fn print_outdated(
   if !outdated.is_empty() {
     outdated.sort();
     print_outdated_table(&outdated);
+  }
+
+  print_skipped_warning(&skipped);
+
+  if !outdated.is_empty() {
     print_suggestion(compatible);
   }
 
   Ok(())
 }
 
+/// Records a package whose metadata fetch failed, deduplicating by
+/// `(kind, name)` and keeping the first reason seen.
+fn collect_skipped(
+  skipped: &mut SkippedPackages,
+  kind: DepKind,
+  name: &StackString,
+  latest_versions: &PackageLatestVersion,
+) {
+  if let Some(error) = &latest_versions.fetch_error {
+    skipped
+      .entry((kind, name.clone()))
+      .or_insert_with(|| error.clone());
+  }
+}
+
+fn print_skipped_warning(skipped: &SkippedPackages) {
+  if skipped.is_empty() {
+    return;
+  }
+
+  log::warn!("");
+  log::warn!(
+    "{}",
+    color_print::cformat!(
+      "<yellow>Warning</>: Unable to check updates for {} package(s), their metadata could not be fetched (e.g. private registry auth or network issues).",
+      skipped.len(),
+    )
+  );
+
+  if log::log_enabled!(log::Level::Debug) {
+    for ((kind, name), error) in skipped {
+      log::warn!(
+        "{}",
+        color_print::cformat!(
+          "  <bold>{}:{}</> (registry: {})",
+          kind.scheme(),
+          name,
+          error.registry_url,
+        )
+      );
+      log::warn!("    Reason: {}", error.reason);
+    }
+    log::warn!(
+      "{}",
+      color_print::cformat!(
+        "<p(245)>Verify your registry credentials (.npmrc) and network connectivity.</>"
+      )
+    );
+  } else {
+    log::warn!(
+      "{}",
+      color_print::cformat!(
+        "<p(245)>Run with </><u>--log-level=debug</><p(245)> for details on which packages could not be checked.</>"
+      )
+    );
+  }
+}
+
 pub async fn outdated(
   flags: Arc<Flags>,
   update_flags: OutdatedFlags,
@@ -367,13 +441,15 @@ async fn update(
   let mut to_update = Vec::new();
 
   let mut can_update_to_latest = false;
+  let mut skipped = SkippedPackages::new();
 
   for (dep_id, resolved, latest_versions) in deps
     .deps_with_resolved_latest_versions()
     .into_iter()
     .collect::<Vec<_>>()
   {
     let dep = deps.get_dep(dep_id);
+    collect_skipped(&mut skipped, dep.kind, &dep.req.name, &latest_versions);
     let new_version_req = choose_new_version_req(
       dep,
       resolved.as_ref(),
@@ -552,6 +628,8 @@ async fn update(
     }
   }
 
+  print_skipped_warning(&skipped);
+
   Ok(())
 }
 

tests/specs/outdated/private_registry_warning/.npmrc

@@ -0,0 +1 @@
+@acme:registry=http://localhost:9/