refactor: update deno_core and use more concrete errors (#27620)

waiting for denoland/deno_core#1043 Fixes #27672
denoland · Jan 17, 2025 · 0540757 · 0540757
1 parent b55451b
commit 0540757
Show file tree

Hide file tree

Showing 22 changed files with 474 additions and 202 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -49,7 +49,7 @@ repository = "https://github.com/denoland/deno"
 
 [workspace.dependencies]
 deno_ast = { version = "=0.44.0", features = ["transpiling"] }
-deno_core = { version = "0.330.0" }
+deno_core = { version = "0.331.0" }
 
 deno_bench_util = { version = "0.180.0", path = "./bench_util" }
 deno_config = { version = "=0.45.0", features = ["workspace", "sync"] }
@@ -123,7 +123,7 @@ dashmap = "5.5.3"
 data-encoding = "2.3.3"
 data-url = "=0.3.1"
 deno_cache_dir = "=0.16.0"
-deno_error = "=0.5.3"
+deno_error = "=0.5.5"
 deno_package_json = { version = "0.4.0", default-features = false }
 deno_unsync = "0.4.2"
 dlopen2 = "0.6.1"

diff --git a/cli/args/lockfile.rs b/cli/args/lockfile.rs
@@ -61,11 +61,13 @@ impl<'a, T> std::ops::DerefMut for Guard<'a, T> {
 }
 
 #[derive(Debug, thiserror::Error, deno_error::JsError)]
-#[error("Failed writing lockfile")]
-#[class(inherit)]
-struct AtomicWriteFileWithRetriesError {
-  #[source]
-  source: std::io::Error,
+pub enum AtomicWriteFileWithRetriesError {
+  #[class(inherit)]
+  #[error(transparent)]
+  Changed(JsErrorBox),
+  #[class(inherit)]
+  #[error("Failed writing lockfile")]
+  Io(#[source] std::io::Error),
 }
 
 impl CliLockfile {
@@ -87,12 +89,16 @@ impl CliLockfile {
     self.lockfile.lock().overwrite
   }
 
-  pub fn write_if_changed(&self) -> Result<(), JsErrorBox> {
+  pub fn write_if_changed(
+    &self,
+  ) -> Result<(), AtomicWriteFileWithRetriesError> {
     if self.skip_write {
       return Ok(());
     }
 
-    self.error_if_changed()?;
+    self
+      .error_if_changed()
+      .map_err(AtomicWriteFileWithRetriesError::Changed)?;
     let mut lockfile = self.lockfile.lock();
     let Some(bytes) = lockfile.resolve_write_bytes() else {
       return Ok(()); // nothing to do
@@ -105,9 +111,7 @@ impl CliLockfile {
       &bytes,
       cache::CACHE_PERM,
     )
-    .map_err(|source| {
-      JsErrorBox::from_err(AtomicWriteFileWithRetriesError { source })
-    })?;
+    .map_err(AtomicWriteFileWithRetriesError::Io)?;
     lockfile.has_content_changed = false;
     Ok(())
   }

diff --git a/cli/args/mod.rs b/cli/args/mod.rs
@@ -82,6 +82,7 @@ use deno_terminal::colors;
 use dotenvy::from_filename;
 pub use flags::*;
 use import_map::resolve_import_map_value_from_specifier;
+pub use lockfile::AtomicWriteFileWithRetriesError;
 pub use lockfile::CliLockfile;
 pub use lockfile::CliLockfileReadFromPathOptions;
 use once_cell::sync::Lazy;

diff --git a/cli/emit.rs b/cli/emit.rs
@@ -112,9 +112,9 @@ impl Emitter {
     &self,
     specifier: &ModuleSpecifier,
     media_type: MediaType,
-    module_kind: deno_ast::ModuleKind,
+    module_kind: ModuleKind,
     source: &Arc<str>,
-  ) -> Result<String, AnyError> {
+  ) -> Result<String, EmitParsedSourceHelperError> {
     // Note: keep this in sync with the sync version below
     let helper = EmitParsedSourceHelper(self);
     match helper.pre_emit_parsed_source(specifier, module_kind, source) {

diff --git a/cli/main.rs b/cli/main.rs
@@ -201,7 +201,7 @@ async fn run_subcommand(flags: Arc<Flags>) -> Result<i32, AnyError> {
         match result {
           Ok(v) => Ok(v),
           Err(script_err) => {
-            if let Some(ResolvePkgFolderFromDenoReqError::Byonm(ByonmResolvePkgFolderFromDenoReqError::UnmatchedReq(_))) = util::result::any_and_jserrorbox_downcast_ref::<ResolvePkgFolderFromDenoReqError>(&script_err) {
+            if let Some(worker::CreateCustomWorkerError::ResolvePkgFolderFromDenoReq(ResolvePkgFolderFromDenoReqError::Byonm(ByonmResolvePkgFolderFromDenoReqError::UnmatchedReq(_)))) = util::result::any_and_jserrorbox_downcast_ref::<worker::CreateCustomWorkerError>(&script_err) {
               if flags.node_modules_dir.is_none() {
                 let mut flags = flags.deref().clone();
                 let watch = match &flags.subcommand {

diff --git a/cli/module_loader.rs b/cli/module_loader.rs
@@ -13,8 +13,6 @@ use std::sync::Arc;
 
 use deno_ast::MediaType;
 use deno_ast::ModuleKind;
-use deno_core::anyhow::anyhow;
-use deno_core::anyhow::Context;
 use deno_core::error::AnyError;
 use deno_core::error::ModuleLoaderError;
 use deno_core::futures::future::FutureExt;
@@ -45,6 +43,7 @@ use deno_lib::worker::ModuleLoaderFactory;
 use deno_resolver::npm::DenoInNpmPackageChecker;
 use deno_runtime::code_cache;
 use deno_runtime::deno_node::create_host_defined_options;
+use deno_runtime::deno_node::ops::require::UnableToGetCwdError;
 use deno_runtime::deno_node::NodeRequireLoader;
 use deno_runtime::deno_permissions::PermissionsContainer;
 use deno_semver::npm::NpmPackageReqReference;
@@ -99,6 +98,11 @@ pub enum PrepareModuleLoadError {
   Check(#[from] CheckError),
   #[class(inherit)]
   #[error(transparent)]
+  AtomicWriteFileWithRetries(
+    #[from] crate::args::AtomicWriteFileWithRetriesError,
+  ),
+  #[class(inherit)]
+  #[error(transparent)]
   Other(#[from] JsErrorBox),
 }
 
@@ -419,6 +423,55 @@ impl ModuleLoaderFactory for CliModuleLoaderFactory {
   }
 }
 
+#[derive(Debug, thiserror::Error, deno_error::JsError)]
+pub enum LoadCodeSourceError {
+  #[class(inherit)]
+  #[error(transparent)]
+  NpmModuleLoad(crate::resolver::NpmModuleLoadError),
+  #[class(inherit)]
+  #[error(transparent)]
+  LoadPreparedModule(#[from] LoadPreparedModuleError),
+  #[class(generic)]
+  #[error("Loading unprepared module: {}{}", .specifier, .maybe_referrer.as_ref().map(|r| format!(", imported from: {}", r)).unwrap_or_default())]
+  LoadUnpreparedModule {
+    specifier: ModuleSpecifier,
+    maybe_referrer: Option<ModuleSpecifier>,
+  },
+}
+
+#[derive(Debug, thiserror::Error, deno_error::JsError)]
+pub enum LoadPreparedModuleError {
+  #[class(inherit)]
+  #[error(transparent)]
+  NpmModuleLoad(#[from] crate::emit::EmitParsedSourceHelperError),
+  #[class(inherit)]
+  #[error(transparent)]
+  LoadMaybeCjs(#[from] LoadMaybeCjsError),
+  #[class(inherit)]
+  #[error(transparent)]
+  Other(#[from] JsErrorBox),
+}
+
+#[derive(Debug, thiserror::Error, deno_error::JsError)]
+pub enum LoadMaybeCjsError {
+  #[class(inherit)]
+  #[error(transparent)]
+  NpmModuleLoad(#[from] crate::emit::EmitParsedSourceHelperError),
+  #[class(inherit)]
+  #[error(transparent)]
+  TranslateCjsToEsm(#[from] node_resolver::analyze::TranslateCjsToEsmError),
+}
+
+#[derive(Debug, thiserror::Error, deno_error::JsError)]
+#[class(inherit)]
+#[error("Could not resolve '{reference}'")]
+pub struct CouldNotResolveError {
+  reference: deno_semver::npm::NpmPackageNvReference,
+  #[source]
+  #[inherit]
+  source: node_resolver::errors::PackageSubpathResolveError,
+}
+
 struct CliModuleLoaderInner<TGraphContainer: ModuleGraphContainer> {
   lib: TsTypeLib,
   is_worker: bool,
@@ -443,7 +496,10 @@ impl<TGraphContainer: ModuleGraphContainer>
     maybe_referrer: Option<&ModuleSpecifier>,
     requested_module_type: RequestedModuleType,
   ) -> Result<ModuleSource, ModuleLoaderError> {
-    let code_source = self.load_code_source(specifier, maybe_referrer).await?;
+    let code_source = self
+      .load_code_source(specifier, maybe_referrer)
+      .await
+      .map_err(JsErrorBox::from_err)?;
     let code = if self.shared.is_inspecting
       || code_source.media_type == MediaType::Wasm
     {
@@ -504,7 +560,7 @@ impl<TGraphContainer: ModuleGraphContainer>
     &self,
     specifier: &ModuleSpecifier,
     maybe_referrer: Option<&ModuleSpecifier>,
-  ) -> Result<ModuleCodeStringSource, AnyError> {
+  ) -> Result<ModuleCodeStringSource, LoadCodeSourceError> {
     if let Some(code_source) = self.load_prepared_module(specifier).await? {
       return Ok(code_source);
     }
@@ -513,14 +569,14 @@ impl<TGraphContainer: ModuleGraphContainer>
         .shared
         .npm_module_loader
         .load(specifier, maybe_referrer)
-        .await;
+        .await
+        .map_err(LoadCodeSourceError::NpmModuleLoad);
     }
 
-    let mut msg = format!("Loading unprepared module: {specifier}");
-    if let Some(referrer) = maybe_referrer {
-      msg = format!("{}, imported from: {}", msg, referrer.as_str());
-    }
-    Err(anyhow!(msg))
+    Err(LoadCodeSourceError::LoadUnpreparedModule {
+      specifier: specifier.clone(),
+      maybe_referrer: maybe_referrer.cloned(),
+    })
   }
 
   fn resolve_referrer(
@@ -543,7 +599,8 @@ impl<TGraphContainer: ModuleGraphContainer>
         .map_err(|e| e.into())
     } else {
       // this cwd check is slow, so try to avoid it
-      let cwd = std::env::current_dir().context("Unable to get CWD")?;
+      let cwd = std::env::current_dir()
+        .map_err(|e| JsErrorBox::from_err(UnableToGetCwdError(e)))?;
       deno_core::resolve_path(referrer, &cwd).map_err(|e| e.into())
     }
   }
@@ -622,8 +679,11 @@ impl<TGraphContainer: ModuleGraphContainer>
             ResolutionMode::Import,
             NodeResolutionKind::Execution,
           )
-          .with_context(|| {
-            format!("Could not resolve '{}'.", module.nv_reference)
+          .map_err(|source| {
+            JsErrorBox::from_err(CouldNotResolveError {
+              reference: module.nv_reference.clone(),
+              source,
+            })
           })?
       }
       Some(Module::Node(module)) => module.specifier.clone(),
@@ -644,7 +704,7 @@ impl<TGraphContainer: ModuleGraphContainer>
   async fn load_prepared_module(
     &self,
     specifier: &ModuleSpecifier,
-  ) -> Result<Option<ModuleCodeStringSource>, AnyError> {
+  ) -> Result<Option<ModuleCodeStringSource>, LoadPreparedModuleError> {
     // Note: keep this in sync with the sync version below
     let graph = self.graph_container.graph();
     match self.load_prepared_module_or_defer_emit(&graph, specifier)? {
@@ -676,7 +736,8 @@ impl<TGraphContainer: ModuleGraphContainer>
       }) => self
         .load_maybe_cjs(specifier, media_type, source)
         .await
-        .map(Some),
+        .map(Some)
+        .map_err(LoadPreparedModuleError::LoadMaybeCjs),
       None => Ok(None),
     }
   }
@@ -837,7 +898,7 @@ impl<TGraphContainer: ModuleGraphContainer>
     specifier: &ModuleSpecifier,
     media_type: MediaType,
     original_source: &Arc<str>,
-  ) -> Result<ModuleCodeStringSource, AnyError> {
+  ) -> Result<ModuleCodeStringSource, LoadMaybeCjsError> {
     let js_source = if media_type.is_emittable() {
       Cow::Owned(
         self

diff --git a/cli/node.rs b/cli/node.rs
@@ -5,13 +5,14 @@ use std::sync::Arc;
 
 use deno_ast::MediaType;
 use deno_ast::ModuleSpecifier;
-use deno_core::error::AnyError;
+use deno_error::JsErrorBox;
 use deno_graph::ParsedSourceStore;
 use deno_resolver::npm::DenoInNpmPackageChecker;
 use deno_runtime::deno_fs;
 use deno_runtime::deno_node::RealIsBuiltInNodeModuleChecker;
 use node_resolver::analyze::CjsAnalysis as ExtNodeCjsAnalysis;
 use node_resolver::analyze::CjsAnalysisExports;
+use node_resolver::analyze::CjsCodeAnalysisError;
 use node_resolver::analyze::CjsCodeAnalyzer;
 use node_resolver::analyze::NodeCodeTranslator;
 use serde::Deserialize;
@@ -75,7 +76,7 @@ impl CliCjsCodeAnalyzer {
     &self,
     specifier: &ModuleSpecifier,
     source: &str,
-  ) -> Result<CliCjsAnalysis, AnyError> {
+  ) -> Result<CliCjsAnalysis, CjsCodeAnalysisError> {
     let source_hash = CacheDBHash::from_hashable(source);
     if let Some(analysis) =
       self.cache.get_cjs_analysis(specifier.as_str(), source_hash)
@@ -102,9 +103,10 @@ impl CliCjsCodeAnalyzer {
       deno_core::unsync::spawn_blocking({
         let specifier = specifier.clone();
         let source: Arc<str> = source.into();
-        move || -> Result<_, AnyError> {
-          let parsed_source =
-            maybe_parsed_source.map(Ok).unwrap_or_else(|| {
+        move || -> Result<_, CjsCodeAnalysisError> {
+          let parsed_source = maybe_parsed_source
+            .map(Ok)
+            .unwrap_or_else(|| {
               deno_ast::parse_program(deno_ast::ParseParams {
                 specifier,
                 text: source,
@@ -113,7 +115,8 @@ impl CliCjsCodeAnalyzer {
                 scope_analysis: false,
                 maybe_syntax: None,
               })
-            })?;
+            })
+            .map_err(JsErrorBox::from_err)?;
           let is_script = parsed_source.compute_is_script();
           let is_cjs = cjs_tracker.is_cjs_with_known_is_script(
             parsed_source.specifier(),
@@ -151,7 +154,7 @@ impl CjsCodeAnalyzer for CliCjsCodeAnalyzer {
     &self,
     specifier: &ModuleSpecifier,
     source: Option<Cow<'a, str>>,
-  ) -> Result<ExtNodeCjsAnalysis<'a>, AnyError> {
+  ) -> Result<ExtNodeCjsAnalysis<'a>, CjsCodeAnalysisError> {
     let source = match source {
       Some(source) => source,
       None => {