fix(node): better detection for when to surface node resolution errors (#24653)

9 files changed, 225 insertions, 245 deletions

diff --git a/cli/module_loader.rs b/cli/module_loader.rs
index 4254375e5..2e047d36d 100644
--- a/cli/module_loader.rs
+++ b/cli/module_loader.rs
@@ -446,15 +446,14 @@ impl<TGraphContainer: ModuleGraphContainer>
     specifier: &str,
     referrer: &ModuleSpecifier,
   ) -> Result<ModuleSpecifier, AnyError> {
-    if let Some(result) = self.shared.node_resolver.resolve_if_in_npm_package(
-      specifier,
-      referrer,
-      NodeResolutionMode::Execution,
-    ) {
-      return match result? {
-        Some(res) => Ok(res.into_url()),
-        None => Err(generic_error("not found")),
-      };
+    if self.shared.node_resolver.in_npm_package(referrer) {
+      return Ok(
+        self
+          .shared
+          .node_resolver
+          .resolve(specifier, referrer, NodeResolutionMode::Execution)?
+          .into_url(),
+      );
     }
 
     let graph = self.graph_container.graph();
diff --git a/cli/npm/byonm.rs b/cli/npm/byonm.rs
index f776b79c1..d10bb6b2a 100644
--- a/cli/npm/byonm.rs
+++ b/cli/npm/byonm.rs
@@ -12,7 +12,8 @@ use deno_core::serde_json;
 use deno_package_json::PackageJsonDepValue;
 use deno_runtime::deno_fs::FileSystem;
 use deno_runtime::deno_node::errors::PackageFolderResolveError;
-use deno_runtime::deno_node::errors::PackageFolderResolveErrorKind;
+use deno_runtime::deno_node::errors::PackageFolderResolveIoError;
+use deno_runtime::deno_node::errors::PackageNotFoundError;
 use deno_runtime::deno_node::load_pkg_json;
 use deno_runtime::deno_node::NodePermissions;
 use deno_runtime::deno_node::NpmResolver;
@@ -198,7 +199,7 @@ impl NpmResolver for ByonmCliNpmResolver {
       }
 
       Err(
-        PackageFolderResolveErrorKind::NotFoundPackage {
+        PackageNotFoundError {
           package_name: name.to_string(),
           referrer: referrer.clone(),
           referrer_extra: None,
@@ -209,7 +210,7 @@ impl NpmResolver for ByonmCliNpmResolver {
 
     let path = inner(&*self.fs, name, referrer)?;
     self.fs.realpath_sync(&path).map_err(|err| {
-      PackageFolderResolveErrorKind::Io {
+      PackageFolderResolveIoError {
         package_name: name.to_string(),
         referrer: referrer.clone(),
         source: err.into_io_error(),
diff --git a/cli/npm/managed/mod.rs b/cli/npm/managed/mod.rs
index a18ad4d7f..602733cab 100644
--- a/cli/npm/managed/mod.rs
+++ b/cli/npm/managed/mod.rs
@@ -21,7 +21,7 @@ use deno_npm::NpmResolutionPackage;
 use deno_npm::NpmSystemInfo;
 use deno_runtime::deno_fs::FileSystem;
 use deno_runtime::deno_node::errors::PackageFolderResolveError;
-use deno_runtime::deno_node::errors::PackageFolderResolveErrorKind;
+use deno_runtime::deno_node::errors::PackageFolderResolveIoError;
 use deno_runtime::deno_node::NodePermissions;
 use deno_runtime::deno_node::NpmResolver;
 use deno_semver::package::PackageNv;
@@ -549,7 +549,7 @@ impl NpmResolver for ManagedCliNpmResolver {
       .resolve_package_folder_from_package(name, referrer)?;
     let path =
       canonicalize_path_maybe_not_exists_with_fs(&path, self.fs.as_ref())
-        .map_err(|err| PackageFolderResolveErrorKind::Io {
+        .map_err(|err| PackageFolderResolveIoError {
           package_name: name.to_string(),
           referrer: referrer.clone(),
           source: err,
diff --git a/cli/npm/managed/resolvers/global.rs b/cli/npm/managed/resolvers/global.rs
index d16fe7cd0..e7a57fc23 100644
--- a/cli/npm/managed/resolvers/global.rs
+++ b/cli/npm/managed/resolvers/global.rs
@@ -15,7 +15,8 @@ use deno_npm::NpmPackageId;
 use deno_npm::NpmSystemInfo;
 use deno_runtime::deno_fs::FileSystem;
 use deno_runtime::deno_node::errors::PackageFolderResolveError;
-use deno_runtime::deno_node::errors::PackageFolderResolveErrorKind;
+use deno_runtime::deno_node::errors::PackageNotFoundError;
+use deno_runtime::deno_node::errors::ReferrerNotFoundError;
 use deno_runtime::deno_node::NodePermissions;
 
 use super::super::cache::NpmCache;
@@ -84,7 +85,7 @@ impl NpmPackageFsResolver for GlobalNpmPackageResolver {
       .resolve_package_folder_id_from_specifier(referrer)
     else {
       return Err(
-        PackageFolderResolveErrorKind::NotFoundReferrer {
+        ReferrerNotFoundError {
           referrer: referrer.clone(),
           referrer_extra: None,
         }
@@ -98,7 +99,7 @@ impl NpmPackageFsResolver for GlobalNpmPackageResolver {
       Ok(pkg) => match self.maybe_package_folder(&pkg.id) {
         Some(folder) => Ok(folder),
         None => Err(
-          PackageFolderResolveErrorKind::NotFoundPackage {
+          PackageNotFoundError {
             package_name: name.to_string(),
             referrer: referrer.clone(),
             referrer_extra: Some(format!(
@@ -112,7 +113,7 @@ impl NpmPackageFsResolver for GlobalNpmPackageResolver {
       },
       Err(err) => match *err {
         PackageNotFoundFromReferrerError::Referrer(cache_folder_id) => Err(
-          PackageFolderResolveErrorKind::NotFoundReferrer {
+          ReferrerNotFoundError {
             referrer: referrer.clone(),
             referrer_extra: Some(cache_folder_id.to_string()),
           }
@@ -122,7 +123,7 @@ impl NpmPackageFsResolver for GlobalNpmPackageResolver {
           name,
           referrer: cache_folder_id_referrer,
         } => Err(
-          PackageFolderResolveErrorKind::NotFoundPackage {
+          PackageNotFoundError {
             package_name: name,
             referrer: referrer.clone(),
             referrer_extra: Some(cache_folder_id_referrer.to_string()),
diff --git a/cli/npm/managed/resolvers/local.rs b/cli/npm/managed/resolvers/local.rs
index 90a17b157..cda78548b 100644
--- a/cli/npm/managed/resolvers/local.rs
+++ b/cli/npm/managed/resolvers/local.rs
@@ -33,7 +33,9 @@ use deno_npm::NpmResolutionPackage;
 use deno_npm::NpmSystemInfo;
 use deno_runtime::deno_fs;
 use deno_runtime::deno_node::errors::PackageFolderResolveError;
-use deno_runtime::deno_node::errors::PackageFolderResolveErrorKind;
+use deno_runtime::deno_node::errors::PackageFolderResolveIoError;
+use deno_runtime::deno_node::errors::PackageNotFoundError;
+use deno_runtime::deno_node::errors::ReferrerNotFoundError;
 use deno_runtime::deno_node::NodePermissions;
 use deno_semver::package::PackageNv;
 use serde::Deserialize;
@@ -185,14 +187,14 @@ impl NpmPackageFsResolver for LocalNpmPackageResolver {
   ) -> Result<PathBuf, PackageFolderResolveError> {
     let maybe_local_path = self
       .resolve_folder_for_specifier(referrer)
-      .map_err(|err| PackageFolderResolveErrorKind::Io {
+      .map_err(|err| PackageFolderResolveIoError {
         package_name: name.to_string(),
         referrer: referrer.clone(),
         source: err,
       })?;
     let Some(local_path) = maybe_local_path else {
       return Err(
-        PackageFolderResolveErrorKind::NotFoundReferrer {
+        ReferrerNotFoundError {
           referrer: referrer.clone(),
           referrer_extra: None,
         }
@@ -220,7 +222,7 @@ impl NpmPackageFsResolver for LocalNpmPackageResolver {
     }
 
     Err(
-      PackageFolderResolveErrorKind::NotFoundPackage {
+      PackageNotFoundError {
         package_name: name.to_string(),
         referrer: referrer.clone(),
         referrer_extra: None,
diff --git a/cli/resolver.rs b/cli/resolver.rs
index c332878a2..c1bb8a0a5 100644
--- a/cli/resolver.rs
+++ b/cli/resolver.rs
@@ -25,15 +25,17 @@ use deno_runtime::deno_fs;
 use deno_runtime::deno_fs::FileSystem;
 use deno_runtime::deno_node::errors::ClosestPkgJsonError;
 use deno_runtime::deno_node::errors::NodeResolveError;
-use deno_runtime::deno_node::errors::ResolvePkgSubpathFromDenoModuleError;
+use deno_runtime::deno_node::errors::NodeResolveErrorKind;
+use deno_runtime::deno_node::errors::PackageFolderResolveErrorKind;
+use deno_runtime::deno_node::errors::PackageFolderResolveIoError;
+use deno_runtime::deno_node::errors::PackageNotFoundError;
+use deno_runtime::deno_node::errors::PackageResolveErrorKind;
 use deno_runtime::deno_node::errors::UrlToNodeResolutionError;
 use deno_runtime::deno_node::is_builtin_node_module;
-use deno_runtime::deno_node::parse_npm_pkg_name;
 use deno_runtime::deno_node::NodeModuleKind;
 use deno_runtime::deno_node::NodeResolution;
 use deno_runtime::deno_node::NodeResolutionMode;
 use deno_runtime::deno_node::NodeResolver;
-use deno_runtime::deno_node::NpmResolver as DenoNodeNpmResolver;
 use deno_runtime::deno_node::PackageJson;
 use deno_runtime::fs_util::specifier_to_file_path;
 use deno_semver::npm::NpmPackageReqReference;
@@ -47,7 +49,6 @@ use crate::args::JsxImportSourceConfig;
 use crate::args::DENO_DISABLE_PEDANTIC_NODE_WARNINGS;
 use crate::colors;
 use crate::node::CliNodeCodeTranslator;
-use crate::npm::ByonmCliNpmResolver;
 use crate::npm::CliNpmResolver;
 use crate::npm::InnerCliNpmResolverRef;
 use crate::util::sync::AtomicFlag;
@@ -102,17 +103,77 @@ impl CliNodeResolver {
     self.node_resolver.get_closest_package_json(referrer)
   }
 
-  pub fn resolve_if_in_npm_package(
+  pub fn resolve_if_for_npm_pkg(
     &self,
     specifier: &str,
     referrer: &ModuleSpecifier,
     mode: NodeResolutionMode,
-  ) -> Option<Result<Option<NodeResolution>, NodeResolveError>> {
-    if self.in_npm_package(referrer) {
-      // we're in an npm package, so use node resolution
-      Some(self.resolve(specifier, referrer, mode))
-    } else {
-      None
+  ) -> Result<Option<NodeResolution>, AnyError> {
+    let resolution_result = self.resolve(specifier, referrer, mode);
+    match resolution_result {
+      Ok(res) => Ok(Some(res)),
+      Err(err) => {
+        let err = err.into_kind();
+        match err {
+          NodeResolveErrorKind::RelativeJoin(_)
+          | NodeResolveErrorKind::PackageImportsResolve(_)
+          | NodeResolveErrorKind::UnsupportedEsmUrlScheme(_)
+          | NodeResolveErrorKind::DataUrlReferrer(_)
+          | NodeResolveErrorKind::TypesNotFound(_)
+          | NodeResolveErrorKind::FinalizeResolution(_)
+          | NodeResolveErrorKind::UrlToNodeResolution(_) => Err(err.into()),
+          NodeResolveErrorKind::PackageResolve(err) => {
+            let err = err.into_kind();
+            match err {
+              PackageResolveErrorKind::ClosestPkgJson(_)
+              | PackageResolveErrorKind::InvalidModuleSpecifier(_)
+              | PackageResolveErrorKind::ExportsResolve(_)
+              | PackageResolveErrorKind::SubpathResolve(_) => Err(err.into()),
+              PackageResolveErrorKind::PackageFolderResolve(err) => {
+                match err.as_kind() {
+                  PackageFolderResolveErrorKind::Io(
+                    PackageFolderResolveIoError { package_name, .. },
+                  )
+                  | PackageFolderResolveErrorKind::PackageNotFound(
+                    PackageNotFoundError { package_name, .. },
+                  ) => {
+                    if self.in_npm_package(referrer) {
+                      return Err(err.into());
+                    }
+                    if let Some(byonm_npm_resolver) =
+                      self.npm_resolver.as_byonm()
+                    {
+                      if byonm_npm_resolver
+                        .find_ancestor_package_json_with_dep(
+                          package_name,
+                          referrer,
+                        )
+                        .is_some()
+                      {
+                        return Err(anyhow!(
+                        concat!(
+                        "Could not resolve \"{}\", but found it in a package.json. ",
+                        "Deno expects the node_modules/ directory to be up to date. ",
+                        "Did you forget to run `npm install`?"
+                      ),
+                        specifier
+                      ));
+                      }
+                    }
+                    Ok(None)
+                  }
+                  PackageFolderResolveErrorKind::ReferrerNotFound(_) => {
+                    if self.in_npm_package(referrer) {
+                      return Err(err.into());
+                    }
+                    Ok(None)
+                  }
+                }
+              }
+            }
+          }
+        }
+      }
     }
   }
 
@@ -121,18 +182,18 @@ impl CliNodeResolver {
     specifier: &str,
     referrer: &ModuleSpecifier,
     mode: NodeResolutionMode,
-  ) -> Result<Option<NodeResolution>, NodeResolveError> {
+  ) -> Result<NodeResolution, NodeResolveError> {
     let referrer_kind = if self.cjs_resolutions.contains(referrer) {
       NodeModuleKind::Cjs
     } else {
       NodeModuleKind::Esm
     };
 
-    let maybe_res =
+    let res =
       self
         .node_resolver
         .resolve(specifier, referrer, referrer_kind, mode)?;
-    Ok(self.handle_node_resolution(maybe_res))
+    Ok(self.handle_node_resolution(res))
   }
 
   pub fn resolve_req_reference(
@@ -159,16 +220,15 @@ impl CliNodeResolver {
     let package_folder = self
       .npm_resolver
       .resolve_pkg_folder_from_deno_module_req(req, referrer)?;
-    let maybe_resolution = self
-      .maybe_resolve_package_sub_path_from_deno_module(
-        &package_folder,
-        sub_path,
-        Some(referrer),
-        mode,
-      )?;
-    match maybe_resolution {
-      Some(resolution) => Ok(resolution),
-      None => {
+    let resolution_result = self.resolve_package_sub_path_from_deno_module(
+      &package_folder,
+      sub_path,
+      Some(referrer),
+      mode,
+    );
+    match resolution_result {
+      Ok(resolution) => Ok(resolution),
+      Err(err) => {
         if self.npm_resolver.as_byonm().is_some() {
           let package_json_path = package_folder.join("package.json");
           if !self.fs.exists_sync(&package_json_path) {
@@ -178,12 +238,7 @@ impl CliNodeResolver {
             ));
           }
         }
-        Err(anyhow!(
-          "Failed resolving '{}{}' in '{}'.",
-          req,
-          sub_path.map(|s| format!("/{}", s)).unwrap_or_default(),
-          package_folder.display()
-        ))
+        Err(err)
       }
     }
   }
@@ -195,32 +250,7 @@ impl CliNodeResolver {
     maybe_referrer: Option<&ModuleSpecifier>,
     mode: NodeResolutionMode,
   ) -> Result<NodeResolution, AnyError> {
-    self
-      .maybe_resolve_package_sub_path_from_deno_module(
-        package_folder,
-        sub_path,
-        maybe_referrer,
-        mode,
-      )?
-      .ok_or_else(|| {
-        anyhow!(
-          "Failed resolving '{}' in '{}'.",
-          sub_path
-            .map(|s| format!("/{}", s))
-            .unwrap_or_else(|| ".".to_string()),
-          package_folder.display(),
-        )
-      })
-  }
-
-  pub fn maybe_resolve_package_sub_path_from_deno_module(
-    &self,
-    package_folder: &Path,
-    sub_path: Option<&str>,
-    maybe_referrer: Option<&ModuleSpecifier>,
-    mode: NodeResolutionMode,
-  ) -> Result<Option<NodeResolution>, ResolvePkgSubpathFromDenoModuleError> {
-    let maybe_res = self
+    let res = self
       .node_resolver
       .resolve_package_subpath_from_deno_module(
         package_folder,
@@ -228,7 +258,7 @@ impl CliNodeResolver {
         maybe_referrer,
         mode,
       )?;
-    Ok(self.handle_node_resolution(maybe_res))
+    Ok(self.handle_node_resolution(res))
   }
 
   pub fn handle_if_in_node_modules(
@@ -267,13 +297,13 @@ impl CliNodeResolver {
 
   fn handle_node_resolution(
     &self,
-    maybe_resolution: Option<NodeResolution>,
-  ) -> Option<NodeResolution> {
-    if let Some(NodeResolution::CommonJs(specifier)) = &maybe_resolution {
+    resolution: NodeResolution,
+  ) -> NodeResolution {
+    if let NodeResolution::CommonJs(specifier) = &resolution {
       // remember that this was a common js resolution
       self.cjs_resolutions.insert(specifier.clone());
     }
-    maybe_resolution
+    resolution
   }
 }
 
@@ -458,39 +488,6 @@ impl CliGraphResolver {
       bare_node_builtins_enabled: self.bare_node_builtins_enabled,
     }
   }
-
-  // todo(dsherret): update this and the surrounding code to handle the structured errors from NodeResolver
-  fn check_surface_byonm_node_error(
-    &self,
-    specifier: &str,
-    referrer: &ModuleSpecifier,
-    original_err: AnyError,
-    resolver: &ByonmCliNpmResolver,
-  ) -> Result<(), AnyError> {
-    if let Ok((pkg_name, _, _)) = parse_npm_pkg_name(specifier, referrer) {
-      match resolver.resolve_package_folder_from_package(&pkg_name, referrer) {
-        Ok(_) => {
-          return Err(original_err);
-        }
-        Err(_) => {
-          if resolver
-            .find_ancestor_package_json_with_dep(&pkg_name, referrer)
-            .is_some()
-          {
-            return Err(anyhow!(
-              concat!(
-                "Could not resolve \"{}\", but found it in a package.json. ",
-                "Deno expects the node_modules/ directory to be up to date. ",
-                "Did you forget to run `npm install`?"
-              ),
-              specifier
-            ));
-          }
-        }
-      }
-    }
-    Ok(())
-  }
 }
 
 impl Resolver for CliGraphResolver {
@@ -523,6 +520,18 @@ impl Resolver for CliGraphResolver {
     }
 
     let referrer = &referrer_range.specifier;
+
+    // Use node resolution if we're in an npm package
+    if let Some(node_resolver) = self.node_resolver.as_ref() {
+      if referrer.scheme() == "file" && node_resolver.in_npm_package(referrer) {
+        return node_resolver
+          .resolve(specifier, referrer, to_node_mode(mode))
+          .map(|res| res.into_url())
+          .map_err(|e| ResolveError::Other(e.into()));
+      }
+    }
+
+    // Attempt to resolve with the workspace resolver
     let result: Result<_, ResolveError> = self
       .workspace_resolver
       .resolve(specifier, referrer)
@@ -535,7 +544,19 @@ impl Resolver for CliGraphResolver {
     let result = match result {
       Ok(resolution) => match resolution {
         MappedResolution::Normal(specifier)
-        | MappedResolution::ImportMap(specifier) => Ok(specifier),
+        | MappedResolution::ImportMap(specifier) => {
+          // do sloppy imports resolution if enabled
+          if let Some(sloppy_imports_resolver) = &self.sloppy_imports_resolver {
+            Ok(sloppy_imports_resolve(
+              sloppy_imports_resolver,
+              specifier,
+              referrer_range,
+              mode,
+            ))
+          } else {
+            Ok(specifier)
+          }
+        }
         MappedResolution::WorkspaceNpmPackage {
           target_pkg_json: pkg_json,
           sub_path,
@@ -552,8 +573,6 @@ impl Resolver for CliGraphResolver {
           )
           .map_err(ResolveError::Other)
           .map(|res| res.into_url()),
-        // todo(dsherret): for byonm it should do resolution solely based on
-        // the referrer and not the package.json
         MappedResolution::PackageJson {
           dep_result,
           alias,
@@ -604,127 +623,75 @@ impl Resolver for CliGraphResolver {
       Err(err) => Err(err),
     };
 
-    // check if it's an npm specifier that resolves to a workspace member
-    if let Some(node_resolver) = &self.node_resolver {
+    // When the user is vendoring, don't allow them to import directly from the vendor/ directory
+    // as it might cause them confusion or duplicate dependencies. Additionally, this folder has
+    // special treatment in the language server so it will definitely cause issues/confusion there
+    // if they do this.
+    if let Some(vendor_specifier) = &self.maybe_vendor_specifier {
       if let Ok(specifier) = &result {
-        if let Ok(req_ref) = NpmPackageReqReference::from_specifier(specifier) {
+        if specifier.as_str().starts_with(vendor_specifier.as_str()) {
+          return Err(ResolveError::Other(anyhow!("Importing from the vendor directory is not permitted. Use a remote specifier instead or disable vendoring.")));
+        }
+      }
+    }
+
+    let Some(node_resolver) = &self.node_resolver else {
+      return result;
+    };
+
+    let is_byonm = self
+      .npm_resolver
+      .as_ref()
+      .is_some_and(|r| r.as_byonm().is_some());
+    match result {
+      Ok(specifier) => {
+        if let Ok(npm_req_ref) =
+          NpmPackageReqReference::from_specifier(&specifier)
+        {
+          // check if the npm specifier resolves to a workspace member
           if let Some(pkg_folder) = self
             .workspace_resolver
-            .resolve_workspace_pkg_json_folder_for_npm_specifier(req_ref.req())
+            .resolve_workspace_pkg_json_folder_for_npm_specifier(
+              npm_req_ref.req(),
+            )
           {
             return Ok(
               node_resolver
                 .resolve_package_sub_path_from_deno_module(
                   pkg_folder,
-                  req_ref.sub_path(),
+                  npm_req_ref.sub_path(),
                   Some(referrer),
                   to_node_mode(mode),
                 )?
                 .into_url(),
             );
           }
-        }
-      }
-    }
-
-    // do sloppy imports resolution if enabled
-    let result =
-      if let Some(sloppy_imports_resolver) = &self.sloppy_imports_resolver {
-        result.map(|specifier| {
-          sloppy_imports_resolve(
-            sloppy_imports_resolver,
-            specifier,
-            referrer_range,
-            mode,
-          )
-        })
-      } else {
-        result
-      };
-
-    // When the user is vendoring, don't allow them to import directly from the vendor/ directory
-    // as it might cause them confusion or duplicate dependencies. Additionally, this folder has
-    // special treatment in the language server so it will definitely cause issues/confusion there
-    // if they do this.
-    if let Some(vendor_specifier) = &self.maybe_vendor_specifier {
-      if let Ok(specifier) = &result {
-        if specifier.as_str().starts_with(vendor_specifier.as_str()) {
-          return Err(ResolveError::Other(anyhow!("Importing from the vendor directory is not permitted. Use a remote specifier instead or disable vendoring.")));
-        }
-      }
-    }
 
-    if let Some(resolver) =
-      self.npm_resolver.as_ref().and_then(|r| r.as_byonm())
-    {
-      match &result {
-        Ok(specifier) => {
-          if let Ok(npm_req_ref) =
-            NpmPackageReqReference::from_specifier(specifier)
-          {
-            let node_resolver = self.node_resolver.as_ref().unwrap();
+          // do npm resolution for byonm
+          if is_byonm {
             return node_resolver
               .resolve_req_reference(&npm_req_ref, referrer, to_node_mode(mode))
               .map(|res| res.into_url())
               .map_err(|err| err.into());
           }
         }
-        Err(_) => {
-          if referrer.scheme() == "file" {
-            if let Some(node_resolver) = &self.node_resolver {
-              let node_result =
-                node_resolver.resolve(specifier, referrer, to_node_mode(mode));
-              match node_result {
-                Ok(Some(res)) => {
-                  return Ok(res.into_url());
-                }
-                Ok(None) => {
-                  self
-                    .check_surface_byonm_node_error(
-                      specifier,
-                      referrer,
-                      anyhow!("Cannot find \"{}\"", specifier),
-                      resolver,
-                    )
-                    .map_err(ResolveError::Other)?;
-                }
-                Err(err) => {
-                  self
-                    .check_surface_byonm_node_error(
-                      specifier,
-                      referrer,
-                      err.into(),
-                      resolver,
-                    )
-                    .map_err(ResolveError::Other)?;
-                }
-              }
-            }
+
+        Ok(node_resolver.handle_if_in_node_modules(specifier)?)
+      }
+      Err(err) => {
+        // If byonm, check if the bare specifier resolves to an npm package
+        if is_byonm && referrer.scheme() == "file" {
+          let maybe_resolution = node_resolver
+            .resolve_if_for_npm_pkg(specifier, referrer, to_node_mode(mode))
+            .map_err(ResolveError::Other)?;
+          if let Some(res) = maybe_resolution {
+            return Ok(res.into_url());
           }
         }
-      }
-    }
 
-    if referrer.scheme() == "file" {
-      if let Some(node_resolver) = &self.node_resolver {
-        let node_result = node_resolver.resolve_if_in_npm_package(
-          specifier,
-          referrer,
-          to_node_mode(mode),
-        );
-        if let Some(Ok(Some(res))) = node_result {
-          return Ok(res.into_url());
-        }
+        Err(err)
       }
     }
-
-    let specifier = result?;
-    match &self.node_resolver {
-      Some(node_resolver) => node_resolver
-        .handle_if_in_node_modules(specifier)
-        .map_err(|e| e.into()),
-      None => Ok(specifier),
-    }
   }
 }
 
diff --git a/cli/standalone/mod.rs b/cli/standalone/mod.rs
index 74586c1be..561c99c98 100644
--- a/cli/standalone/mod.rs
+++ b/cli/standalone/mod.rs
@@ -151,15 +151,14 @@ impl ModuleLoader for EmbeddedModuleLoader {
       })?
     };
 
-    if let Some(result) = self.shared.node_resolver.resolve_if_in_npm_package(
-      specifier,
-      &referrer,
-      NodeResolutionMode::Execution,
-    ) {
-      return match result? {
-        Some(res) => Ok(res.into_url()),
-        None => Err(generic_error("not found")),
-      };
+    if self.shared.node_resolver.in_npm_package(&referrer) {
+      return Ok(
+        self
+          .shared
+          .node_resolver
+          .resolve(specifier, &referrer, NodeResolutionMode::Execution)?
+          .into_url(),
+      );
     }
 
     let mapped_resolution =
@@ -250,14 +249,12 @@ impl ModuleLoader for EmbeddedModuleLoader {
       Err(err)
         if err.is_unmapped_bare_specifier() && referrer.scheme() == "file" =>
       {
-        // todo(dsherret): return a better error from node resolution so that
-        // we can more easily tell whether to surface it or not
-        let node_result = self.shared.node_resolver.resolve(
+        let maybe_res = self.shared.node_resolver.resolve_if_for_npm_pkg(
           specifier,
           &referrer,
           NodeResolutionMode::Execution,
-        );
-        if let Ok(Some(res)) = node_result {
+        )?;
+        if let Some(res) = maybe_res {
           return Ok(res.into_url());
         }
         Err(err.into())
diff --git a/cli/tsc/mod.rs b/cli/tsc/mod.rs
index f6440266e..424b5c3d3 100644
--- a/cli/tsc/mod.rs
+++ b/cli/tsc/mod.rs
@@ -30,6 +30,8 @@ use deno_graph::GraphKind;
 use deno_graph::Module;
 use deno_graph::ModuleGraph;
 use deno_graph::ResolutionResolved;
+use deno_runtime::deno_node::errors::NodeJsErrorCode;
+use deno_runtime::deno_node::errors::NodeJsErrorCoded;
 use deno_runtime::deno_node::NodeModuleKind;
 use deno_runtime::deno_node::NodeResolution;
 use deno_runtime::deno_node::NodeResolutionMode;
@@ -756,13 +758,21 @@ fn resolve_graph_specifier_types(
           .as_managed()
           .unwrap() // should never be byonm because it won't create Module::Npm
           .resolve_pkg_folder_from_deno_module(module.nv_reference.nv())?;
-        let maybe_resolution =
+        let res_result =
           npm.node_resolver.resolve_package_subpath_from_deno_module(
             &package_folder,
             module.nv_reference.sub_path(),
             Some(referrer),
             NodeResolutionMode::Types,
-          )?;
+          );
+        let maybe_resolution = match res_result {
+          Ok(res) => Some(res),
+          Err(err) => match err.code() {
+            NodeJsErrorCode::ERR_TYPES_NOT_FOUND
+            | NodeJsErrorCode::ERR_MODULE_NOT_FOUND => None,
+            _ => return Err(err.into()),
+          },
+        };
         Ok(Some(NodeResolution::into_specifier_and_media_type(
           maybe_resolution,
         )))
@@ -805,8 +815,7 @@ fn resolve_non_graph_specifier_types(
           referrer_kind,
           NodeResolutionMode::Types,
         )
-        .ok()
-        .flatten(),
+        .ok(),
     )))
   } else if let Ok(npm_req_ref) = NpmPackageReqReference::from_str(specifier) {
     debug_assert_eq!(referrer_kind, NodeModuleKind::Esm);
@@ -817,13 +826,20 @@ fn resolve_non_graph_specifier_types(
     let package_folder = npm
       .npm_resolver
       .resolve_pkg_folder_from_deno_module_req(npm_req_ref.req(), referrer)?;
-    let maybe_resolution = node_resolver
-      .resolve_package_subpath_from_deno_module(
-        &package_folder,
-        npm_req_ref.sub_path(),
-        Some(referrer),
-        NodeResolutionMode::Types,
-      )?;
+    let res_result = node_resolver.resolve_package_subpath_from_deno_module(
+      &package_folder,
+      npm_req_ref.sub_path(),
+      Some(referrer),
+      NodeResolutionMode::Types,
+    );
+    let maybe_resolution = match res_result {
+      Ok(res) => Some(res),
+      Err(err) => match err.code() {
+        NodeJsErrorCode::ERR_TYPES_NOT_FOUND
+        | NodeJsErrorCode::ERR_MODULE_NOT_FOUND => None,
+        _ => return Err(err.into()),
+      },
+    };
     Ok(Some(NodeResolution::into_specifier_and_media_type(
       maybe_resolution,
     )))
diff --git a/cli/worker.rs b/cli/worker.rs
index a639dae41..0d7e61c50 100644
--- a/cli/worker.rs
+++ b/cli/worker.rs
@@ -687,7 +687,7 @@ impl CliMainWorkerFactory {
       return Ok(None);
     }
 
-    let Some(resolution) = self
+    let resolution = self
       .shared
       .node_resolver
       .resolve_package_subpath_from_deno_module(
@@ -695,10 +695,7 @@ impl CliMainWorkerFactory {
         sub_path,
         /* referrer */ None,
         NodeResolutionMode::Execution,
-      )?
-    else {
-      return Ok(None);
-    };
+      )?;
     match &resolution {
       NodeResolution::BuiltIn(_) => Ok(None),
       NodeResolution::CommonJs(specifier) | NodeResolution::Esm(specifier) => {