fix: lazily surface errors in package.json deps parsing (#17974)

Closes #17941

1 files changed, 122 insertions, 64 deletions

diff --git a/cli/args/package_json.rs b/cli/args/package_json.rs
index 301d1b8ba..4f44137de 100644
--- a/cli/args/package_json.rs
+++ b/cli/args/package_json.rs
@@ -5,30 +5,52 @@ use std::collections::HashMap;
 use std::path::Path;
 use std::path::PathBuf;
 
-use deno_core::anyhow::anyhow;
 use deno_core::anyhow::bail;
 use deno_core::error::AnyError;
 use deno_graph::npm::NpmPackageReq;
+use deno_graph::semver::NpmVersionReqSpecifierParseError;
 use deno_graph::semver::VersionReq;
 use deno_runtime::deno_node::PackageJson;
+use thiserror::Error;
+
+#[derive(Debug, Clone, Error, PartialEq, Eq, Hash)]
+#[error("Could not find @ symbol in npm url '{value}'")]
+pub struct PackageJsonDepNpmSchemeValueParseError {
+  pub value: String,
+}
 
 /// Gets the name and raw version constraint taking into account npm
 /// package aliases.
 pub fn parse_dep_entry_name_and_raw_version<'a>(
   key: &'a str,
   value: &'a str,
-) -> Result<(&'a str, &'a str), AnyError> {
+) -> Result<(&'a str, &'a str), PackageJsonDepNpmSchemeValueParseError> {
   if let Some(package_and_version) = value.strip_prefix("npm:") {
     if let Some((name, version)) = package_and_version.rsplit_once('@') {
       Ok((name, version))
     } else {
-      bail!("could not find @ symbol in npm url '{}'", value);
+      Err(PackageJsonDepNpmSchemeValueParseError {
+        value: value.to_string(),
+      })
     }
   } else {
     Ok((key, value))
   }
 }
 
+#[derive(Debug, Error, Clone, Hash)]
+pub enum PackageJsonDepValueParseError {
+  #[error(transparent)]
+  SchemeValue(#[from] PackageJsonDepNpmSchemeValueParseError),
+  #[error(transparent)]
+  Specifier(#[from] NpmVersionReqSpecifierParseError),
+  #[error("Not implemented scheme: {scheme}")]
+  Unsupported { scheme: String },
+}
+
+pub type PackageJsonDeps =
+  BTreeMap<String, Result<NpmPackageReq, PackageJsonDepValueParseError>>;
+
 /// Gets an application level package.json's npm package requirements.
 ///
 /// Note that this function is not general purpose. It is specifically for
@@ -37,48 +59,43 @@ pub fn parse_dep_entry_name_and_raw_version<'a>(
 /// entries to npm specifiers which can then be used in the resolver.
 pub fn get_local_package_json_version_reqs(
   package_json: &PackageJson,
-) -> Result<BTreeMap<String, NpmPackageReq>, AnyError> {
+) -> PackageJsonDeps {
+  fn parse_entry(
+    key: &str,
+    value: &str,
+  ) -> Result<NpmPackageReq, PackageJsonDepValueParseError> {
+    if value.starts_with("workspace:")
+      || value.starts_with("file:")
+      || value.starts_with("git:")
+      || value.starts_with("http:")
+      || value.starts_with("https:")
+    {
+      return Err(PackageJsonDepValueParseError::Unsupported {
+        scheme: key.split(':').next().unwrap().to_string(),
+      });
+    }
+    let (name, version_req) = parse_dep_entry_name_and_raw_version(key, value)
+      .map_err(PackageJsonDepValueParseError::SchemeValue)?;
+
+    let result = VersionReq::parse_from_specifier(version_req);
+    match result {
+      Ok(version_req) => Ok(NpmPackageReq {
+        name: name.to_string(),
+        version_req: Some(version_req),
+      }),
+      Err(err) => Err(PackageJsonDepValueParseError::Specifier(err)),
+    }
+  }
+
   fn insert_deps(
     deps: Option<&HashMap<String, String>>,
-    result: &mut BTreeMap<String, NpmPackageReq>,
-  ) -> Result<(), AnyError> {
+    result: &mut PackageJsonDeps,
+  ) {
     if let Some(deps) = deps {
       for (key, value) in deps {
-        if value.starts_with("workspace:")
-          || value.starts_with("file:")
-          || value.starts_with("git:")
-          || value.starts_with("http:")
-          || value.starts_with("https:")
-        {
-          // skip these specifiers for now
-          continue;
-        }
-        let (name, version_req) =
-          parse_dep_entry_name_and_raw_version(key, value)?;
-
-        let version_req = {
-          let result = VersionReq::parse_from_specifier(version_req);
-          match result {
-            Ok(version_req) => version_req,
-            Err(e) => {
-              let err = anyhow!("{:#}", e).context(concat!(
-                "Parsing version constraints in the application-level ",
-                "package.json is more strict at the moment"
-              ));
-              return Err(err);
-            }
-          }
-        };
-        result.insert(
-          key.to_string(),
-          NpmPackageReq {
-            name: name.to_string(),
-            version_req: Some(version_req),
-          },
-        );
+        result.insert(key.to_string(), parse_entry(key, value));
       }
     }
-    Ok(())
   }
 
   let deps = package_json.dependencies.as_ref();
@@ -87,10 +104,10 @@ pub fn get_local_package_json_version_reqs(
 
   // insert the dev dependencies first so the dependencies will
   // take priority and overwrite any collisions
-  insert_deps(dev_deps, &mut result)?;
-  insert_deps(deps, &mut result)?;
+  insert_deps(dev_deps, &mut result);
+  insert_deps(deps, &mut result);
 
-  Ok(result)
+  result
 }
 
 /// Attempts to discover the package.json file, maybe stopping when it
@@ -147,7 +164,7 @@ mod test {
       (
         "test",
         "npm:package",
-        Err("could not find @ symbol in npm url 'npm:package'"),
+        Err("Could not find @ symbol in npm url 'npm:package'"),
       ),
     ];
     for (key, value, expected_result) in cases {
@@ -159,6 +176,23 @@ mod test {
     }
   }
 
+  fn get_local_package_json_version_reqs_for_tests(
+    package_json: &PackageJson,
+  ) -> BTreeMap<String, Result<NpmPackageReq, String>> {
+    get_local_package_json_version_reqs(package_json)
+      .into_iter()
+      .map(|(k, v)| {
+        (
+          k,
+          match v {
+            Ok(v) => Ok(v),
+            Err(err) => Err(err.to_string()),
+          },
+        )
+      })
+      .collect::<BTreeMap<_, _>>()
+  }
+
   #[test]
   fn test_get_local_package_json_version_reqs() {
     let mut package_json = PackageJson::empty(PathBuf::from("/package.json"));
@@ -171,21 +205,21 @@ mod test {
       // should be ignored
       ("other".to_string(), "^3.2".to_string()),
     ]));
-    let result = get_local_package_json_version_reqs(&package_json).unwrap();
+    let deps = get_local_package_json_version_reqs_for_tests(&package_json);
     assert_eq!(
-      result,
+      deps,
       BTreeMap::from([
         (
           "test".to_string(),
-          NpmPackageReq::from_str("test@^1.2").unwrap()
+          Ok(NpmPackageReq::from_str("test@^1.2").unwrap())
         ),
         (
           "other".to_string(),
-          NpmPackageReq::from_str("package@~1.3").unwrap()
+          Ok(NpmPackageReq::from_str("package@~1.3").unwrap())
         ),
         (
           "package_b".to_string(),
-          NpmPackageReq::from_str("package_b@~2.2").unwrap()
+          Ok(NpmPackageReq::from_str("package_b@~2.2").unwrap())
         )
       ])
     );
@@ -198,18 +232,20 @@ mod test {
       "test".to_string(),
       "1.x - 1.3".to_string(),
     )]));
-    let err = get_local_package_json_version_reqs(&package_json)
-      .err()
-      .unwrap();
+    let map = get_local_package_json_version_reqs_for_tests(&package_json);
     assert_eq!(
-      format!("{err:#}"),
-      concat!(
-        "Parsing version constraints in the application-level ",
-        "package.json is more strict at the moment: Invalid npm specifier ",
-        "version requirement. Unexpected character.\n",
-        "   - 1.3\n",
-        "  ~"
-      )
+      map,
+      BTreeMap::from([(
+        "test".to_string(),
+        Err(
+          concat!(
+            "Invalid npm specifier version requirement. Unexpected character.\n",
+            "   - 1.3\n",
+            "  ~"
+          )
+          .to_string()
+        )
+      )])
     );
   }
 
@@ -224,13 +260,35 @@ mod test {
       ("http".to_string(), "http://something".to_string()),
       ("https".to_string(), "https://something".to_string()),
     ]));
-    let result = get_local_package_json_version_reqs(&package_json).unwrap();
+    let result = get_local_package_json_version_reqs_for_tests(&package_json);
     assert_eq!(
       result,
-      BTreeMap::from([(
-        "test".to_string(),
-        NpmPackageReq::from_str("test@1").unwrap()
-      )])
+      BTreeMap::from([
+        (
+          "file".to_string(),
+          Err("Not implemented scheme: file".to_string()),
+        ),
+        (
+          "git".to_string(),
+          Err("Not implemented scheme: git".to_string()),
+        ),
+        (
+          "http".to_string(),
+          Err("Not implemented scheme: http".to_string()),
+        ),
+        (
+          "https".to_string(),
+          Err("Not implemented scheme: https".to_string()),
+        ),
+        (
+          "test".to_string(),
+          Ok(NpmPackageReq::from_str("test@1").unwrap())
+        ),
+        (
+          "work".to_string(),
+          Err("Not implemented scheme: work".to_string()),
+        )
+      ])
     );
   }
 }