refactor: remove CliNpmRegistryApi (#27222)

Extracts more code out of the CLI.

cli/lsp/diagnostics.rs

@@ -1262,7 +1262,7 @@ impl DenoDiagnostic {
       Self::NoAttributeType => (lsp::DiagnosticSeverity::ERROR, "The module is a JSON module and not being imported with an import attribute. Consider adding `with { type: \"json\" }` to the import statement.".to_string(), None),
       Self::NoCache(specifier) => (lsp::DiagnosticSeverity::ERROR, format!("Uncached or missing remote URL: {specifier}"), Some(json!({ "specifier": specifier }))),
       Self::NotInstalledJsr(pkg_req, specifier) => (lsp::DiagnosticSeverity::ERROR, format!("JSR package \"{pkg_req}\" is not installed or doesn't exist."), Some(json!({ "specifier": specifier }))),
-      Self::NotInstalledNpm(pkg_req, specifier) => (lsp::DiagnosticSeverity::ERROR, format!("NPM package \"{pkg_req}\" is not installed or doesn't exist."), Some(json!({ "specifier": specifier }))),
+      Self::NotInstalledNpm(pkg_req, specifier) => (lsp::DiagnosticSeverity::ERROR, format!("npm package \"{pkg_req}\" is not installed or doesn't exist."), Some(json!({ "specifier": specifier }))),
       Self::NoLocal(specifier) => {
         let maybe_sloppy_resolution = CliSloppyImportsResolver::new(
           SloppyImportsCachedFs::new(Arc::new(deno_fs::RealFs))

cli/npm/managed/mod.rs

@@ -44,7 +44,6 @@ use crate::util::fs::canonicalize_path_maybe_not_exists_with_fs;
 use crate::util::progress_bar::ProgressBar;
 use crate::util::sync::AtomicFlag;
 
-use self::registry::CliNpmRegistryApi;
 use self::resolution::NpmResolution;
 use self::resolvers::create_npm_fs_resolver;
 use self::resolvers::NpmPackageFsResolver;
@@ -57,7 +56,6 @@ use super::CliNpmTarballCache;
 use super::InnerCliNpmResolverRef;
 use super::ResolvePkgFolderFromDenoReqError;
 
-mod registry;
 mod resolution;
 mod resolvers;
 
@@ -120,13 +118,13 @@ pub async fn create_managed_npm_resolver(
 ) -> Result<Arc<dyn CliNpmResolver>, AnyError> {
   let npm_cache_env = create_cache_env(&options);
   let npm_cache = create_cache(npm_cache_env.clone(), &options);
-  let npm_api = create_api(npm_cache.clone(), npm_cache_env.clone(), &options);
+  let api = create_api(npm_cache.clone(), npm_cache_env.clone(), &options);
-  let snapshot = resolve_snapshot(&npm_api, options.snapshot).await?;
+  let snapshot = resolve_snapshot(&api, options.snapshot).await?;
   Ok(create_inner(
     npm_cache_env,
     options.fs,
     options.maybe_lockfile,
-    npm_api,
+    api,
     npm_cache,
     options.npmrc,
     options.npm_install_deps_provider,
@@ -143,7 +141,7 @@ fn create_inner(
   env: Arc<CliNpmCacheEnv>,
   fs: Arc<dyn deno_runtime::deno_fs::FileSystem>,
   maybe_lockfile: Option<Arc<CliLockfile>>,
-  npm_api: Arc<CliNpmRegistryApi>,
+  registry_info_provider: Arc<CliNpmRegistryInfoProvider>,
   npm_cache: Arc<CliNpmCache>,
   npm_rc: Arc<ResolvedNpmRc>,
   npm_install_deps_provider: Arc<NpmInstallDepsProvider>,
@@ -154,7 +152,7 @@ fn create_inner(
   lifecycle_scripts: LifecycleScriptsConfig,
 ) -> Arc<dyn CliNpmResolver> {
   let resolution = Arc::new(NpmResolution::from_serialized(
-    npm_api.clone(),
+    registry_info_provider.clone(),
     snapshot,
     maybe_lockfile.clone(),
   ));
@@ -178,7 +176,7 @@ fn create_inner(
     fs,
     fs_resolver,
     maybe_lockfile,
-    npm_api,
+    registry_info_provider,
     npm_cache,
     npm_install_deps_provider,
     resolution,
@@ -215,25 +213,25 @@ fn create_api(
   cache: Arc<CliNpmCache>,
   env: Arc<CliNpmCacheEnv>,
   options: &CliManagedNpmResolverCreateOptions,
-) -> Arc<CliNpmRegistryApi> {
+) -> Arc<CliNpmRegistryInfoProvider> {
-  Arc::new(CliNpmRegistryApi::new(
-    cache.clone(),
   Arc::new(CliNpmRegistryInfoProvider::new(
     cache,
     env,
     options.npmrc.clone(),
-    )),
   ))
 }
 
 async fn resolve_snapshot(
-  api: &CliNpmRegistryApi,
+  registry_info_provider: &Arc<CliNpmRegistryInfoProvider>,
   snapshot: CliNpmResolverManagedSnapshotOption,
 ) -> Result<Option<ValidSerializedNpmResolutionSnapshot>, AnyError> {
   match snapshot {
     CliNpmResolverManagedSnapshotOption::ResolveFromLockfile(lockfile) => {
       if !lockfile.overwrite() {
-        let snapshot = snapshot_from_lockfile(lockfile.clone(), api)
+        let snapshot = snapshot_from_lockfile(
+          lockfile.clone(),
+          &registry_info_provider.as_npm_registry_api(),
+        )
         .await
         .with_context(|| {
           format!("failed reading lockfile '{}'", lockfile.filename.display())
@@ -304,7 +302,7 @@ pub struct ManagedCliNpmResolver {
   fs: Arc<dyn FileSystem>,
   fs_resolver: Arc<dyn NpmPackageFsResolver>,
   maybe_lockfile: Option<Arc<CliLockfile>>,
-  npm_api: Arc<CliNpmRegistryApi>,
+  registry_info_provider: Arc<CliNpmRegistryInfoProvider>,
   npm_cache: Arc<CliNpmCache>,
   npm_install_deps_provider: Arc<NpmInstallDepsProvider>,
   resolution: Arc<NpmResolution>,
@@ -329,7 +327,7 @@ impl ManagedCliNpmResolver {
     fs: Arc<dyn FileSystem>,
     fs_resolver: Arc<dyn NpmPackageFsResolver>,
     maybe_lockfile: Option<Arc<CliLockfile>>,
-    npm_api: Arc<CliNpmRegistryApi>,
+    registry_info_provider: Arc<CliNpmRegistryInfoProvider>,
     npm_cache: Arc<CliNpmCache>,
     npm_install_deps_provider: Arc<NpmInstallDepsProvider>,
     resolution: Arc<NpmResolution>,
@@ -342,7 +340,7 @@ impl ManagedCliNpmResolver {
       fs,
       fs_resolver,
       maybe_lockfile,
-      npm_api,
+      registry_info_provider,
       npm_cache,
       npm_install_deps_provider,
       text_only_progress_bar,
@@ -588,7 +586,7 @@ impl ManagedCliNpmResolver {
   ) -> Result<Arc<NpmPackageInfo>, AnyError> {
     // this will internally cache the package information
     self
-      .npm_api
+      .registry_info_provider
       .package_info(package_name)
       .await
       .map_err(|err| err.into())
@@ -684,7 +682,7 @@ impl CliNpmResolver for ManagedCliNpmResolver {
   fn clone_snapshotted(&self) -> Arc<dyn CliNpmResolver> {
     // create a new snapshotted npm resolution and resolver
     let npm_resolution = Arc::new(NpmResolution::new(
-      self.npm_api.clone(),
+      self.registry_info_provider.clone(),
       self.resolution.snapshot(),
       self.maybe_lockfile.clone(),
     ));
@@ -703,7 +701,7 @@ impl CliNpmResolver for ManagedCliNpmResolver {
         self.lifecycle_scripts.clone(),
       ),
       self.maybe_lockfile.clone(),
-      self.npm_api.clone(),
+      self.registry_info_provider.clone(),
       self.npm_cache.clone(),
       self.npm_install_deps_provider.clone(),
       npm_resolution,

cli/npm/managed/registry.rs

@@ -1,201 +0,0 @@
-// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
-
-use std::collections::HashMap;
-use std::collections::HashSet;
-use std::sync::Arc;
-
-use async_trait::async_trait;
-use deno_core::anyhow::anyhow;
-use deno_core::error::AnyError;
-use deno_core::futures::future::BoxFuture;
-use deno_core::futures::future::Shared;
-use deno_core::futures::FutureExt;
-use deno_core::parking_lot::Mutex;
-use deno_npm::registry::NpmPackageInfo;
-use deno_npm::registry::NpmRegistryApi;
-use deno_npm::registry::NpmRegistryPackageInfoLoadError;
-use deno_npm_cache::NpmCacheSetting;
-
-use crate::npm::CliNpmCache;
-use crate::npm::CliNpmRegistryInfoProvider;
-use crate::util::sync::AtomicFlag;
-
-// todo(#27198): Remove this and move functionality down into
-// RegistryInfoProvider, which already does most of this.
-#[derive(Debug)]
-pub struct CliNpmRegistryApi(Option<Arc<CliNpmRegistryApiInner>>);
-
-impl CliNpmRegistryApi {
-  pub fn new(
-    cache: Arc<CliNpmCache>,
-    registry_info_provider: Arc<CliNpmRegistryInfoProvider>,
-  ) -> Self {
-    Self(Some(Arc::new(CliNpmRegistryApiInner {
-      cache,
-      force_reload_flag: Default::default(),
-      mem_cache: Default::default(),
-      previously_reloaded_packages: Default::default(),
-      registry_info_provider,
-    })))
-  }
-
-  /// Clears the internal memory cache.
-  pub fn clear_memory_cache(&self) {
-    self.inner().clear_memory_cache();
-  }
-
-  fn inner(&self) -> &Arc<CliNpmRegistryApiInner> {
-    // this panicking indicates a bug in the code where this
-    // wasn't initialized
-    self.0.as_ref().unwrap()
-  }
-}
-
-#[async_trait(?Send)]
-impl NpmRegistryApi for CliNpmRegistryApi {
-  async fn package_info(
-    &self,
-    name: &str,
-  ) -> Result<Arc<NpmPackageInfo>, NpmRegistryPackageInfoLoadError> {
-    match self.inner().maybe_package_info(name).await {
-      Ok(Some(info)) => Ok(info),
-      Ok(None) => Err(NpmRegistryPackageInfoLoadError::PackageNotExists {
-        package_name: name.to_string(),
-      }),
-      Err(err) => {
-        Err(NpmRegistryPackageInfoLoadError::LoadError(Arc::new(err)))
-      }
-    }
-  }
-
-  fn mark_force_reload(&self) -> bool {
-    self.inner().mark_force_reload()
-  }
-}
-
-type CacheItemPendingResult =
-  Result<Option<Arc<NpmPackageInfo>>, Arc<AnyError>>;
-
-#[derive(Debug)]
-enum CacheItem {
-  Pending(Shared<BoxFuture<'static, CacheItemPendingResult>>),
-  Resolved(Option<Arc<NpmPackageInfo>>),
-}
-
-#[derive(Debug)]
-struct CliNpmRegistryApiInner {
-  cache: Arc<CliNpmCache>,
-  force_reload_flag: AtomicFlag,
-  mem_cache: Mutex<HashMap<String, CacheItem>>,
-  previously_reloaded_packages: Mutex<HashSet<String>>,
-  registry_info_provider: Arc<CliNpmRegistryInfoProvider>,
-}
-
-impl CliNpmRegistryApiInner {
-  pub async fn maybe_package_info(
-    self: &Arc<Self>,
-    name: &str,
-  ) -> Result<Option<Arc<NpmPackageInfo>>, AnyError> {
-    let (created, future) = {
-      let mut mem_cache = self.mem_cache.lock();
-      match mem_cache.get(name) {
-        Some(CacheItem::Resolved(maybe_info)) => {
-          return Ok(maybe_info.clone());
-        }
-        Some(CacheItem::Pending(future)) => (false, future.clone()),
-        None => {
-          let future = {
-            let api = self.clone();
-            let name = name.to_string();
-            async move {
-              if (api.cache.cache_setting().should_use_for_npm_package(&name) && !api.force_reload_flag.is_raised())
-                // if this has been previously reloaded, then try loading from the
-                // file system cache
-                || !api.previously_reloaded_packages.lock().insert(name.to_string())
-              {
-                // attempt to load from the file cache
-                if let Some(info) = api.load_file_cached_package_info(&name).await {
-                  let result = Some(Arc::new(info));
-                  return Ok(result);
-                }
-              }
-              api.registry_info_provider
-                .load_package_info(&name)
-                .await
-                .map_err(Arc::new)
-            }
-            .boxed()
-            .shared()
-          };
-          mem_cache
-            .insert(name.to_string(), CacheItem::Pending(future.clone()));
-          (true, future)
-        }
-      }
-    };
-
-    if created {
-      match future.await {
-        Ok(maybe_info) => {
-          // replace the cache item to say it's resolved now
-          self
-            .mem_cache
-            .lock()
-            .insert(name.to_string(), CacheItem::Resolved(maybe_info.clone()));
-          Ok(maybe_info)
-        }
-        Err(err) => {
-          // purge the item from the cache so it loads next time
-          self.mem_cache.lock().remove(name);
-          Err(anyhow!("{:#}", err))
-        }
-      }
-    } else {
-      Ok(future.await.map_err(|err| anyhow!("{:#}", err))?)
-    }
-  }
-
-  fn mark_force_reload(&self) -> bool {
-    // never force reload the registry information if reloading
-    // is disabled or if we're already reloading
-    if matches!(
-      self.cache.cache_setting(),
-      NpmCacheSetting::Only | NpmCacheSetting::ReloadAll
-    ) {
-      return false;
-    }
-    if self.force_reload_flag.raise() {
-      self.clear_memory_cache();
-      true
-    } else {
-      false
-    }
-  }
-
-  async fn load_file_cached_package_info(
-    &self,
-    name: &str,
-  ) -> Option<NpmPackageInfo> {
-    let result = deno_core::unsync::spawn_blocking({
-      let cache = self.cache.clone();
-      let name = name.to_string();
-      move || cache.load_package_info(&name)
-    })
-    .await
-    .unwrap();
-    match result {
-      Ok(value) => value,
-      Err(err) => {
-        if cfg!(debug_assertions) {
-          panic!("error loading cached npm package info for {name}: {err:#}");
-        } else {
-          None
-        }
-      }
-    }
-  }
-
-  fn clear_memory_cache(&self) {
-    self.mem_cache.lock().clear();
-  }
-}

cli/npm/managed/resolution.rs

@@ -27,10 +27,9 @@ use deno_semver::package::PackageReq;
 use deno_semver::VersionReq;
 
 use crate::args::CliLockfile;
+use crate::npm::CliNpmRegistryInfoProvider;
 use crate::util::sync::SyncReadAsyncWriteLock;
 
-use super::CliNpmRegistryApi;
-
 pub struct AddPkgReqsResult {
   /// Results from adding the individual packages.
   ///
@@ -47,7 +46,7 @@ pub struct AddPkgReqsResult {
 ///
 /// This does not interact with the file system.
 pub struct NpmResolution {
-  api: Arc<CliNpmRegistryApi>,
+  registry_info_provider: Arc<CliNpmRegistryInfoProvider>,
   snapshot: SyncReadAsyncWriteLock<NpmResolutionSnapshot>,
   maybe_lockfile: Option<Arc<CliLockfile>>,
 }
@@ -63,22 +62,22 @@ impl std::fmt::Debug for NpmResolution {
 
 impl NpmResolution {
   pub fn from_serialized(
-    api: Arc<CliNpmRegistryApi>,
+    registry_info_provider: Arc<CliNpmRegistryInfoProvider>,
     initial_snapshot: Option<ValidSerializedNpmResolutionSnapshot>,
     maybe_lockfile: Option<Arc<CliLockfile>>,
   ) -> Self {
     let snapshot =
       NpmResolutionSnapshot::new(initial_snapshot.unwrap_or_default());
-    Self::new(api, snapshot, maybe_lockfile)
+    Self::new(registry_info_provider, snapshot, maybe_lockfile)
   }
 
   pub fn new(
-    api: Arc<CliNpmRegistryApi>,
+    registry_info_provider: Arc<CliNpmRegistryInfoProvider>,
     initial_snapshot: NpmResolutionSnapshot,
     maybe_lockfile: Option<Arc<CliLockfile>>,
   ) -> Self {
     Self {
-      api,
+      registry_info_provider,
       snapshot: SyncReadAsyncWriteLock::new(initial_snapshot),
       maybe_lockfile,
     }
@@ -91,7 +90,7 @@ impl NpmResolution {
     // only allow one thread in here at a time
     let snapshot_lock = self.snapshot.acquire().await;
     let result = add_package_reqs_to_snapshot(
-      &self.api,
+      &self.registry_info_provider,
       package_reqs,
       self.maybe_lockfile.clone(),
       || snapshot_lock.read().clone(),
@@ -119,7 +118,7 @@ impl NpmResolution {
 
     let reqs_set = package_reqs.iter().collect::<HashSet<_>>();
     let snapshot = add_package_reqs_to_snapshot(
-      &self.api,
+      &self.registry_info_provider,
       package_reqs,
       self.maybe_lockfile.clone(),
       || {
@@ -259,7 +258,7 @@ impl NpmResolution {
 }
 
 async fn add_package_reqs_to_snapshot(
-  api: &CliNpmRegistryApi,
+  registry_info_provider: &Arc<CliNpmRegistryInfoProvider>,
   package_reqs: &[PackageReq],
   maybe_lockfile: Option<Arc<CliLockfile>>,
   get_new_snapshot: impl Fn() -> NpmResolutionSnapshot,
@@ -282,26 +281,28 @@ async fn add_package_reqs_to_snapshot(
     /* this string is used in tests */
     "Running npm resolution."
   );
+  let npm_registry_api = registry_info_provider.as_npm_registry_api();
   let result = snapshot
-    .add_pkg_reqs(api, get_add_pkg_reqs_options(package_reqs))
+    .add_pkg_reqs(&npm_registry_api, get_add_pkg_reqs_options(package_reqs))
     .await;
-  api.clear_memory_cache();
   let result = match &result.dep_graph_result {
-    Err(NpmResolutionError::Resolution(err)) if api.mark_force_reload() => {
+    Err(NpmResolutionError::Resolution(err))
+      if npm_registry_api.mark_force_reload() =>
+    {
       log::debug!("{err:#}");
       log::debug!("npm resolution failed. Trying again...");
 
-      // try again
+      // try again with forced reloading
       let snapshot = get_new_snapshot();
-      let result = snapshot
+      snapshot
-        .add_pkg_reqs(api, get_add_pkg_reqs_options(package_reqs))
+        .add_pkg_reqs(&npm_registry_api, get_add_pkg_reqs_options(package_reqs))
-        .await;
+        .await
-      api.clear_memory_cache();
-      result
     }
     _ => result,
   };
 
+  registry_info_provider.clear_memory_cache();
+
   if let Ok(snapshot) = &result.dep_graph_result {
     if let Some(lockfile) = maybe_lockfile {
       populate_lockfile_from_snapshot(&lockfile, snapshot);

resolvers/npm_cache/lib.rs

@@ -42,7 +42,7 @@ pub struct DownloadError {
 
 impl std::error::Error for DownloadError {
   fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
-    Some(self.error.as_ref())
+    self.error.source()
   }
 }
 

resolvers/npm_cache/registry_info.rs

@@ -1,14 +1,19 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 
 use std::collections::HashMap;
+use std::collections::HashSet;
 use std::sync::Arc;
 
 use anyhow::anyhow;
 use anyhow::bail;
 use anyhow::Context;
 use anyhow::Error as AnyError;
+use async_trait::async_trait;
 use deno_npm::npm_rc::ResolvedNpmRc;
 use deno_npm::registry::NpmPackageInfo;
+use deno_npm::registry::NpmRegistryApi;
+use deno_npm::registry::NpmRegistryPackageInfoLoadError;
+use deno_unsync::sync::AtomicFlag;
 use deno_unsync::sync::MultiRuntimeAsyncValueCreator;
 use futures::future::LocalBoxFuture;
 use futures::FutureExt;
@@ -43,8 +48,49 @@ enum MemoryCacheItem {
   MemoryCached(Result<Option<Arc<NpmPackageInfo>>, Arc<AnyError>>),
 }
 
-// todo(#27198): refactor to store this only in the http cache and also
+#[derive(Debug, Default)]
-// consolidate with CliNpmRegistryApi.
+struct MemoryCache {
+  clear_id: usize,
+  items: HashMap<String, MemoryCacheItem>,
+}
+
+impl MemoryCache {
+  #[inline(always)]
+  pub fn clear(&mut self) {
+    self.clear_id += 1;
+    self.items.clear();
+  }
+
+  #[inline(always)]
+  pub fn get(&self, key: &str) -> Option<&MemoryCacheItem> {
+    self.items.get(key)
+  }
+
+  #[inline(always)]
+  pub fn insert(&mut self, key: String, value: MemoryCacheItem) {
+    self.items.insert(key, value);
+  }
+
+  #[inline(always)]
+  pub fn try_insert(
+    &mut self,
+    clear_id: usize,
+    key: &str,
+    value: MemoryCacheItem,
+  ) -> bool {
+    if clear_id != self.clear_id {
+      return false;
+    }
+    // if the clear_id is the same then the item should exist
+    debug_assert!(self.items.contains_key(key));
+    if let Some(item) = self.items.get_mut(key) {
+      *item = value;
+    }
+    true
+  }
+}
+
+// todo(#27198): refactor to store this only in the http cache
 
 /// Downloads packuments from the npm registry.
 ///
@@ -55,7 +101,9 @@ pub struct RegistryInfoProvider<TEnv: NpmCacheEnv> {
   cache: Arc<NpmCache<TEnv>>,
   env: Arc<TEnv>,
   npmrc: Arc<ResolvedNpmRc>,
-  memory_cache: Mutex<HashMap<String, MemoryCacheItem>>,
+  force_reload_flag: AtomicFlag,
+  memory_cache: Mutex<MemoryCache>,
+  previously_loaded_packages: Mutex<HashSet<String>>,
 }
 
 impl<TEnv: NpmCacheEnv> RegistryInfoProvider<TEnv> {
@@ -68,17 +116,60 @@ impl<TEnv: NpmCacheEnv> RegistryInfoProvider<TEnv> {
       cache,
       env,
       npmrc,
+      force_reload_flag: AtomicFlag::lowered(),
       memory_cache: Default::default(),
+      previously_loaded_packages: Default::default(),
     }
   }
 
-  pub async fn load_package_info(
+  /// Clears the internal memory cache.
+  pub fn clear_memory_cache(&self) {
+    self.memory_cache.lock().clear();
+  }
+
+  fn mark_force_reload(&self) -> bool {
+    // never force reload the registry information if reloading
+    // is disabled or if we're already reloading
+    if matches!(
+      self.cache.cache_setting(),
+      NpmCacheSetting::Only | NpmCacheSetting::ReloadAll
+    ) {
+      return false;
+    }
+    if self.force_reload_flag.raise() {
+      self.clear_memory_cache();
+      true
+    } else {
+      false
+    }
+  }
+
+  pub fn as_npm_registry_api(self: &Arc<Self>) -> NpmRegistryApiAdapter<TEnv> {
+    NpmRegistryApiAdapter(self.clone())
+  }
+
+  pub async fn package_info(
+    self: &Arc<Self>,
+    name: &str,
+  ) -> Result<Arc<NpmPackageInfo>, NpmRegistryPackageInfoLoadError> {
+    match self.maybe_package_info(name).await {
+      Ok(Some(info)) => Ok(info),
+      Ok(None) => Err(NpmRegistryPackageInfoLoadError::PackageNotExists {
+        package_name: name.to_string(),
+      }),
+      Err(err) => {
+        Err(NpmRegistryPackageInfoLoadError::LoadError(Arc::new(err)))
+      }
+    }
+  }
+
+  pub async fn maybe_package_info(
     self: &Arc<Self>,
     name: &str,
   ) -> Result<Option<Arc<NpmPackageInfo>>, AnyError> {
     self.load_package_info_inner(name).await.with_context(|| {
       format!(
-        "Error getting response at {} for package \"{}\"",
+        "Failed loading {} for package \"{}\"",
         get_package_url(&self.npmrc, name),
         name
       )
@@ -89,18 +180,9 @@ impl<TEnv: NpmCacheEnv> RegistryInfoProvider<TEnv> {
     self: &Arc<Self>,
     name: &str,
   ) -> Result<Option<Arc<NpmPackageInfo>>, AnyError> {
-    if *self.cache.cache_setting() == NpmCacheSetting::Only {
+    let (cache_item, clear_id) = {
-      return Err(deno_core::error::custom_error(
-        "NotCached",
-        format!(
-          "An npm specifier not found in cache: \"{name}\", --cached-only is specified."
-        )
-      ));
-    }
-
-    let cache_item = {
       let mut mem_cache = self.memory_cache.lock();
-      if let Some(cache_item) = mem_cache.get(name) {
+      let cache_item = if let Some(cache_item) = mem_cache.get(name) {
         cache_item.clone()
       } else {
         let value_creator = MultiRuntimeAsyncValueCreator::new({
@@ -111,7 +193,8 @@ impl<TEnv: NpmCacheEnv> RegistryInfoProvider<TEnv> {
         let cache_item = MemoryCacheItem::Pending(Arc::new(value_creator));
         mem_cache.insert(name.to_string(), cache_item.clone());
         cache_item
-      }
+      };
+      (cache_item, mem_cache.clear_id)
     };
 
     match cache_item {
@@ -130,25 +213,37 @@ impl<TEnv: NpmCacheEnv> RegistryInfoProvider<TEnv> {
           Ok(FutureResult::SavedFsCache(info)) => {
             // return back the future and mark this package as having
             // been saved in the cache for next time it's requested
-            *self.memory_cache.lock().get_mut(name).unwrap() =
+            self.memory_cache.lock().try_insert(
-              MemoryCacheItem::FsCached;
+              clear_id,
+              name,
+              MemoryCacheItem::FsCached,
+            );
             Ok(Some(info))
           }
           Ok(FutureResult::ErroredFsCache(info)) => {
             // since saving to the fs cache failed, keep the package information in memory
-            *self.memory_cache.lock().get_mut(name).unwrap() =
+            self.memory_cache.lock().try_insert(
-              MemoryCacheItem::MemoryCached(Ok(Some(info.clone())));
+              clear_id,
+              name,
+              MemoryCacheItem::MemoryCached(Ok(Some(info.clone()))),
+            );
             Ok(Some(info))
           }
           Ok(FutureResult::PackageNotExists) => {
-            *self.memory_cache.lock().get_mut(name).unwrap() =
+            self.memory_cache.lock().try_insert(
-              MemoryCacheItem::MemoryCached(Ok(None));
+              clear_id,
+              name,
+              MemoryCacheItem::MemoryCached(Ok(None)),
+            );
             Ok(None)
           }
           Err(err) => {
-            let return_err = anyhow!("{}", err);
+            let return_err = anyhow!("{:#}", err);
-            *self.memory_cache.lock().get_mut(name).unwrap() =
+            self.memory_cache.lock().try_insert(
-              MemoryCacheItem::MemoryCached(Err(err));
+              clear_id,
+              name,
+              MemoryCacheItem::MemoryCached(Err(err)),
+            );
             Err(return_err)
           }
         }
@@ -196,6 +291,29 @@ impl<TEnv: NpmCacheEnv> RegistryInfoProvider<TEnv> {
       };
     let name = name.to_string();
     async move {
+      if (downloader.cache.cache_setting().should_use_for_npm_package(&name) && !downloader.force_reload_flag.is_raised())
+        // if this has been previously reloaded, then try loading from the
+        // file system cache
+        || downloader.previously_loaded_packages.lock().contains(&name)
+      {
+        // attempt to load from the file cache
+        if let Some(info) = downloader.cache.load_package_info(&name)? {
+          let result = Arc::new(info);
+          return Ok(FutureResult::SavedFsCache(result));
+        }
+      }
+
+      if *downloader.cache.cache_setting() == NpmCacheSetting::Only {
+        return Err(deno_core::error::custom_error(
+          "NotCached",
+          format!(
+            "npm package not found in cache: \"{name}\", --cached-only is specified."
+          )
+        ));
+      }
+
+      downloader.previously_loaded_packages.lock().insert(name.to_string());
+
       let maybe_bytes = downloader
         .env
         .download_with_retries_on_any_tokio_runtime(
@@ -234,6 +352,24 @@ impl<TEnv: NpmCacheEnv> RegistryInfoProvider<TEnv> {
   }
 }
 
+pub struct NpmRegistryApiAdapter<TEnv: NpmCacheEnv>(
+  Arc<RegistryInfoProvider<TEnv>>,
+);
+
+#[async_trait(?Send)]
+impl<TEnv: NpmCacheEnv> NpmRegistryApi for NpmRegistryApiAdapter<TEnv> {
+  async fn package_info(
+    &self,
+    name: &str,
+  ) -> Result<Arc<NpmPackageInfo>, NpmRegistryPackageInfoLoadError> {
+    self.0.package_info(name).await
+  }
+
+  fn mark_force_reload(&self) -> bool {
+    self.0.mark_force_reload()
+  }
+}
+
 // todo(#27198): make this private and only use RegistryInfoProvider in the rest of
 // the code
 pub fn get_package_url(npmrc: &ResolvedNpmRc, name: &str) -> Url {

resolvers/npm_cache/tarball.rs

@@ -65,13 +65,13 @@ impl<TEnv: NpmCacheEnv> TarballCache<TEnv> {
 
   pub async fn ensure_package(
     self: &Arc<Self>,
-    package: &PackageNv,
+    package_nv: &PackageNv,
     dist: &NpmPackageVersionDistInfo,
   ) -> Result<(), AnyError> {
     self
-      .ensure_package_inner(package, dist)
+      .ensure_package_inner(package_nv, dist)
       .await
-      .with_context(|| format!("Failed caching npm package '{}'.", package))
+      .with_context(|| format!("Failed caching npm package '{}'.", package_nv))
   }
 
   async fn ensure_package_inner(
@@ -100,7 +100,7 @@ impl<TEnv: NpmCacheEnv> TarballCache<TEnv> {
 
     match cache_item {
       MemoryCacheItem::Cached => Ok(()),
-      MemoryCacheItem::Errored(err) => Err(anyhow!("{}", err)),
+      MemoryCacheItem::Errored(err) => Err(anyhow!("{:#}", err)),
       MemoryCacheItem::Pending(creator) => {
         let result = creator.get().await;
         match result {
@@ -110,7 +110,7 @@ impl<TEnv: NpmCacheEnv> TarballCache<TEnv> {
             Ok(())
           }
           Err(err) => {
-            let result_err = anyhow!("{}", err);
+            let result_err = anyhow!("{:#}", err);
             *self.memory_cache.lock().get_mut(package_nv).unwrap() =
               MemoryCacheItem::Errored(err);
             Err(result_err)
@@ -138,7 +138,7 @@ impl<TEnv: NpmCacheEnv> TarballCache<TEnv> {
         return Err(deno_core::error::custom_error(
           "NotCached",
           format!(
-            "An npm specifier not found in cache: \"{}\", --cached-only is specified.",
+            "npm package not found in cache: \"{}\", --cached-only is specified.",
             &package_nv.name
           )
         )

tests/integration/lsp_tests.rs

@@ -6009,7 +6009,7 @@ fn lsp_code_actions_deno_cache_npm() {
         "severity": 1,
         "code": "not-installed-npm",
         "source": "deno",
-        "message": "NPM package \"chalk\" is not installed or doesn't exist.",
+        "message": "npm package \"chalk\" is not installed or doesn't exist.",
         "data": { "specifier": "npm:chalk" }
       }],
       "version": 1
@@ -6036,7 +6036,7 @@ fn lsp_code_actions_deno_cache_npm() {
           "severity": 1,
           "code": "not-installed-npm",
           "source": "deno",
-          "message": "NPM package \"chalk\" is not installed or doesn't exist.",
+          "message": "npm package \"chalk\" is not installed or doesn't exist.",
           "data": { "specifier": "npm:chalk" }
         }],
         "only": ["quickfix"]
@@ -6056,7 +6056,7 @@ fn lsp_code_actions_deno_cache_npm() {
         "severity": 1,
         "code": "not-installed-npm",
         "source": "deno",
-        "message": "NPM package \"chalk\" is not installed or doesn't exist.",
+        "message": "npm package \"chalk\" is not installed or doesn't exist.",
         "data": { "specifier": "npm:chalk" }
       }],
       "command": {
@@ -6111,7 +6111,7 @@ fn lsp_code_actions_deno_cache_all() {
           "severity": 1,
           "code": "not-installed-npm",
           "source": "deno",
-          "message": "NPM package \"chalk\" is not installed or doesn't exist.",
+          "message": "npm package \"chalk\" is not installed or doesn't exist.",
           "data": { "specifier": "npm:chalk" },
         },
       ],
@@ -6199,7 +6199,7 @@ fn lsp_code_actions_deno_cache_all() {
             "severity": 1,
             "code": "not-installed-npm",
             "source": "deno",
-            "message": "NPM package \"chalk\" is not installed or doesn't exist.",
+            "message": "npm package \"chalk\" is not installed or doesn't exist.",
             "data": { "specifier": "npm:chalk" },
           },
         ],
@@ -9860,7 +9860,7 @@ fn lsp_completions_node_builtin() {
         "severity": 1,
         "code": "not-installed-npm",
         "source": "deno",
-        "message": "NPM package \"@types/node\" is not installed or doesn't exist."
+        "message": "npm package \"@types/node\" is not installed or doesn't exist."
       }
     ])
   );

tests/integration/npm_tests.rs

@@ -102,7 +102,7 @@ fn cached_only_after_first_run() {
   let stdout = String::from_utf8_lossy(&output.stdout);
   assert_contains!(
     stderr,
-    "An npm specifier not found in cache: \"ansi-styles\", --cached-only is specified."
+    "npm package not found in cache: \"ansi-styles\", --cached-only is specified."
   );
   assert!(stdout.is_empty());
   assert!(!output.status.success());

tests/specs/npm/cached_only/cached_only/main.out

@@ -1,2 +1,3 @@
-error: Error getting response at http://localhost:4260/chalk for package "chalk": An npm specifier not found in cache: "chalk", --cached-only is specified.
+error: Failed loading http://localhost:4260/chalk for package "chalk"
+    0: npm package not found in cache: "chalk", --cached-only is specified.
     at file:///[WILDCARD]/main.ts:1:19

tests/specs/npm/npmrc_bad_registry_config/main.out

@@ -1,3 +1,5 @@
 Download http://localhost:4261/@denotest%2fbasic
-error: Error getting response at http://localhost:4261/@denotest%2fbasic for package "@denotest/basic": Bad response: 401
+error: Failed loading http://localhost:4261/@denotest%2fbasic for package "@denotest/basic"
-[WILDCARD]
+
+Caused by:
+    Bad response: 401

tests/specs/npm/npmrc_bad_token/main.out

@@ -1,3 +1,5 @@
 Download http://localhost:4261/@denotest%2fbasic
-error: Error getting response at http://localhost:4261/@denotest%2fbasic for package "@denotest/basic": Bad response: 401
+error: Failed loading http://localhost:4261/@denotest%2fbasic for package "@denotest/basic"
-[WILDCARD]
+
+Caused by:
+    Bad response: 401

tests/specs/npm/npmrc_password_no_username/install.out

@@ -1,3 +1,4 @@
-[UNORDERED_START]
+error: Failed loading http://localhost:4261/@denotest%2fbasic for package "@denotest/basic"
-error: Error getting response at http://localhost:4261/@denotest%2fbasic for package "@denotest/basic": Both the username and password must be provided for basic auth
+
-[UNORDERED_END]
+Caused by:
+    Both the username and password must be provided for basic auth

tests/specs/npm/npmrc_username_no_password/install.out

@@ -1,3 +1,4 @@
-[UNORDERED_START]
+error: Failed loading http://localhost:4261/@denotest%2fbasic for package "@denotest/basic"
-error: Error getting response at http://localhost:4261/@denotest%2fbasic for package "@denotest/basic": Both the username and password must be provided for basic auth
+
-[UNORDERED_END]
+Caused by:
+    Both the username and password must be provided for basic auth

tests/testdata/npm/cached_only/main.out

@@ -1,2 +1,2 @@
-error: Error getting response at http://localhost:4260/chalk for package "chalk": An npm specifier not found in cache: "chalk", --cached-only is specified.
+error: Failed loading http://localhost:4260/chalk for package "chalk": npm package not found in cache: "chalk", --cached-only is specified.
     at file:///[WILDCARD]/testdata/npm/cached_only/main.ts:1:19