fix: surface package.json location on dep parse failure (#26665)

Related: #26653
denoland · Nov 5, 2024 · 919a96d · 919a96d
1 parent 12c8b80
commit 919a96d
Show file tree

Hide file tree

Showing 8 changed files with 56 additions and 12 deletions.
diff --git a/cli/args/mod.rs b/cli/args/mod.rs
@@ -46,6 +46,7 @@ pub use flags::*;
 pub use lockfile::CliLockfile;
 pub use lockfile::CliLockfileReadFromPathOptions;
 pub use package_json::NpmInstallDepsProvider;
+pub use package_json::PackageJsonDepValueParseWithLocationError;
 
 use deno_ast::ModuleSpecifier;
 use deno_core::anyhow::bail;

diff --git a/cli/args/package_json.rs b/cli/args/package_json.rs
@@ -5,10 +5,12 @@ use std::sync::Arc;
 
 use deno_config::workspace::Workspace;
 use deno_core::serde_json;
+use deno_core::url::Url;
 use deno_package_json::PackageJsonDepValue;
 use deno_package_json::PackageJsonDepValueParseError;
 use deno_semver::npm::NpmPackageReqReference;
 use deno_semver::package::PackageReq;
+use thiserror::Error;
 
 #[derive(Debug)]
 pub struct InstallNpmRemotePkg {
@@ -23,11 +25,20 @@ pub struct InstallNpmWorkspacePkg {
   pub target_dir: PathBuf,
 }
 
+#[derive(Debug, Error, Clone)]
+#[error("Failed to install '{}'\n    at {}", alias, location)]
+pub struct PackageJsonDepValueParseWithLocationError {
+  pub location: Url,
+  pub alias: String,
+  #[source]
+  pub source: PackageJsonDepValueParseError,
+}
+
 #[derive(Debug, Default)]
 pub struct NpmInstallDepsProvider {
   remote_pkgs: Vec<InstallNpmRemotePkg>,
   workspace_pkgs: Vec<InstallNpmWorkspacePkg>,
-  pkg_json_dep_errors: Vec<PackageJsonDepValueParseError>,
+  pkg_json_dep_errors: Vec<PackageJsonDepValueParseWithLocationError>,
 }
 
 impl NpmInstallDepsProvider {
@@ -89,7 +100,13 @@ impl NpmInstallDepsProvider {
           let dep = match dep {
             Ok(dep) => dep,
             Err(err) => {
-              pkg_json_dep_errors.push(err);
+              pkg_json_dep_errors.push(
+                PackageJsonDepValueParseWithLocationError {
+                  location: pkg_json.specifier(),
+                  alias,
+                  source: err,
+                },
+              );
               continue;
             }
           };
@@ -150,7 +167,9 @@ impl NpmInstallDepsProvider {
     &self.workspace_pkgs
   }
 
-  pub fn pkg_json_dep_errors(&self) -> &[PackageJsonDepValueParseError] {
+  pub fn pkg_json_dep_errors(
+    &self,
+  ) -> &[PackageJsonDepValueParseWithLocationError] {
     &self.pkg_json_dep_errors
   }
 }
diff --git a/cli/npm/managed/mod.rs b/cli/npm/managed/mod.rs
@@ -38,6 +38,7 @@ use crate::args::LifecycleScriptsConfig;
 use crate::args::NpmInstallDepsProvider;
 use crate::args::NpmProcessState;
 use crate::args::NpmProcessStateKind;
+use crate::args::PackageJsonDepValueParseWithLocationError;
 use crate::cache::DenoCacheEnvFsAdapter;
 use crate::cache::FastInsecureHasher;
 use crate::http_util::HttpClientProvider;
@@ -480,19 +481,24 @@ impl ManagedCliNpmResolver {
     self.resolution.resolve_pkg_id_from_pkg_req(req)
   }
 
-  pub fn ensure_no_pkg_json_dep_errors(&self) -> Result<(), AnyError> {
+  pub fn ensure_no_pkg_json_dep_errors(
+    &self,
+  ) -> Result<(), Box<PackageJsonDepValueParseWithLocationError>> {
     for err in self.npm_install_deps_provider.pkg_json_dep_errors() {
-      match err {
+      match &err.source {
         deno_package_json::PackageJsonDepValueParseError::VersionReq(_) => {
-          return Err(
-            AnyError::from(err.clone())
-              .context("Failed to install from package.json"),
-          );
+          return Err(Box::new(err.clone()));
         }
         deno_package_json::PackageJsonDepValueParseError::Unsupported {
           ..
         } => {
-          log::warn!("{} {} in package.json", colors::yellow("Warning"), err)
+          // only warn for this one
+          log::warn!(
+            "{} {}\n    at {}",
+            colors::yellow("Warning"),
+            err.source,
+            err.location,
+          )
         }
       }
     }

diff --git a/tests/specs/install/invalid_scheme/__test__.jsonc b/tests/specs/install/invalid_scheme/__test__.jsonc
@@ -0,0 +1,5 @@
+{
+  "tempDir": true,
+  "args": "install",
+  "output": "install.out"
+}
diff --git a/tests/specs/install/invalid_scheme/install.out b/tests/specs/install/invalid_scheme/install.out
@@ -0,0 +1,5 @@
+Warning Not implemented scheme 'git'
+    at file:///[WILDLINE]/package.json
+Download http://localhost:4260/@denotest%2fadd
+Download http://localhost:4260/@denotest/add/1.0.0.tgz
+Initialize @denotest/[email protected]
diff --git a/tests/specs/install/invalid_scheme/package.json b/tests/specs/install/invalid_scheme/package.json
@@ -0,0 +1,6 @@
+{
+  "dependencies": {
+    "@denotest/add": "*",
+    "deno": "git:denoland/deno"
+  }
+}
diff --git a/tests/specs/run/package_json/invalid_value/add.out b/tests/specs/run/package_json/invalid_value/add.out
@@ -1,5 +1,6 @@
 Add npm:[email protected]
-error: Failed to install from package.json
+error: Failed to install '@denotest/cjs-default-export'
+    at file:///[WILDLINE]/package.json
 
 Caused by:
     0: Invalid version requirement

diff --git a/tests/specs/run/package_json/invalid_value/install.out b/tests/specs/run/package_json/invalid_value/install.out
@@ -1,4 +1,5 @@
-error: Failed to install from package.json
+error: Failed to install '@denotest/cjs-default-export'
+    at file:///[WILDLINE]/package.json
 
 Caused by:
     0: Invalid version requirement