better error message for missing module (#3402)

cli/file_fetcher.rs

@@ -134,6 +134,7 @@ impl SourceFileFetcher {
   pub fn fetch_source_file_async(
     self: &Self,
     specifier: &ModuleSpecifier,
+    maybe_referrer: Option<ModuleSpecifier>,
   ) -> Pin<Box<SourceFileFuture>> {
     let module_url = specifier.as_url().to_owned();
     debug!("fetch_source_file. specifier {} ", &module_url);
@@ -156,12 +157,16 @@ impl SourceFileFetcher {
       .then(move |result| {
         let mut out = match result.map_err(|err| {
           if err.kind() == ErrorKind::NotFound {
-            // For NotFound, change the message to something better.
-            DenoError::new(
-              ErrorKind::NotFound,
-              format!("Cannot resolve module \"{}\"", module_url.to_string()),
-            )
-            .into()
+            let msg = if let Some(referrer) = maybe_referrer {
+              format!(
+                "Cannot resolve module \"{}\" from \"{}\"",
+                module_url.to_string(),
+                referrer
+              )
+            } else {
+              format!("Cannot resolve module \"{}\"", module_url.to_string())
+            };
+            DenoError::new(ErrorKind::NotFound, msg).into()
           } else {
             err
           }
@@ -1011,10 +1016,12 @@ mod tests {
     );
 
     // first download
-    tokio_util::run(fetcher.fetch_source_file_async(&specifier).then(|r| {
-      assert!(r.is_ok());
-      futures::future::ok(())
-    }));
+    tokio_util::run(fetcher.fetch_source_file_async(&specifier, None).then(
+      |r| {
+        assert!(r.is_ok());
+        futures::future::ok(())
+      },
+    ));
 
     let result = fs::File::open(&headers_file_name);
     assert!(result.is_ok());
@@ -1026,10 +1033,12 @@ mod tests {
     // download file again, it should use already fetched file even though `use_disk_cache` is set to
     // false, this can be verified using source header file creation timestamp (should be
     // the same as after first download)
-    tokio_util::run(fetcher.fetch_source_file_async(&specifier).then(|r| {
-      assert!(r.is_ok());
-      futures::future::ok(())
-    }));
+    tokio_util::run(fetcher.fetch_source_file_async(&specifier, None).then(
+      |r| {
+        assert!(r.is_ok());
+        futures::future::ok(())
+      },
+    ));
 
     let result = fs::File::open(&headers_file_name);
     assert!(result.is_ok());
@@ -1443,20 +1452,24 @@ mod tests {
     // Test failure case.
     let specifier =
       ModuleSpecifier::resolve_url(file_url!("/baddir/hello.ts")).unwrap();
-    tokio_util::run(fetcher.fetch_source_file_async(&specifier).then(|r| {
-      assert!(r.is_err());
-      futures::future::ok(())
-    }));
+    tokio_util::run(fetcher.fetch_source_file_async(&specifier, None).then(
+      |r| {
+        assert!(r.is_err());
+        futures::future::ok(())
+      },
+    ));
 
     let p = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR"))
       .join("js/main.ts")
       .to_owned();
     let specifier =
       ModuleSpecifier::resolve_url_or_path(p.to_str().unwrap()).unwrap();
-    tokio_util::run(fetcher.fetch_source_file_async(&specifier).then(|r| {
-      assert!(r.is_ok());
-      futures::future::ok(())
-    }));
+    tokio_util::run(fetcher.fetch_source_file_async(&specifier, None).then(
+      |r| {
+        assert!(r.is_ok());
+        futures::future::ok(())
+      },
+    ));
   }
 
   #[test]
@@ -1467,20 +1480,24 @@ mod tests {
     // Test failure case.
     let specifier =
       ModuleSpecifier::resolve_url(file_url!("/baddir/hello.ts")).unwrap();
-    tokio_util::run(fetcher.fetch_source_file_async(&specifier).then(|r| {
-      assert!(r.is_err());
-      futures::future::ok(())
-    }));
+    tokio_util::run(fetcher.fetch_source_file_async(&specifier, None).then(
+      |r| {
+        assert!(r.is_err());
+        futures::future::ok(())
+      },
+    ));
 
     let p = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR"))
       .join("js/main.ts")
       .to_owned();
     let specifier =
       ModuleSpecifier::resolve_url_or_path(p.to_str().unwrap()).unwrap();
-    tokio_util::run(fetcher.fetch_source_file_async(&specifier).then(|r| {
-      assert!(r.is_ok());
-      futures::future::ok(())
-    }));
+    tokio_util::run(fetcher.fetch_source_file_async(&specifier, None).then(
+      |r| {
+        assert!(r.is_ok());
+        futures::future::ok(())
+      },
+    ));
   }
 
   #[test]

cli/global_state.rs

@@ -124,13 +124,14 @@ impl ThreadSafeGlobalState {
   pub fn fetch_compiled_module(
     self: &Self,
     module_specifier: &ModuleSpecifier,
+    maybe_referrer: Option<ModuleSpecifier>,
   ) -> impl Future<Output = Result<CompiledModule, ErrBox>> {
     let state1 = self.clone();
     let state2 = self.clone();
 
     self
       .file_fetcher
-      .fetch_source_file_async(&module_specifier)
+      .fetch_source_file_async(&module_specifier, maybe_referrer)
       .and_then(move |out| match out.media_type {
         msg::MediaType::Unknown => state1.js_compiler.compile_async(&out),
         msg::MediaType::Json => state1.json_compiler.compile_async(&out),

cli/js/compiler.ts

@@ -171,7 +171,8 @@ class SourceFile {
 
   /** Cache the source file to be able to be retrieved by `moduleSpecifier` and
    * `containingFile`. */
-  cache(moduleSpecifier: string, containingFile: string): void {
+  cache(moduleSpecifier: string, containingFile?: string): void {
+    containingFile = containingFile || "";
     let innerCache = SourceFile._specifierCache.get(containingFile);
     if (!innerCache) {
       innerCache = new Map();
@@ -269,7 +270,7 @@ function fetchAsset(name: string): string {
 /** Ops to Rust to resolve and fetch modules meta data. */
 function fetchSourceFiles(
   specifiers: string[],
-  referrer: string
+  referrer?: string
 ): Promise<SourceFileJson[]> {
   util.log("compiler::fetchSourceFiles", { specifiers, referrer });
   return sendAsync(dispatch.OP_FETCH_SOURCE_FILES, {
@@ -286,7 +287,7 @@ function fetchSourceFiles(
  * that should be actually resolved. */
 async function processImports(
   specifiers: Array<[string, string]>,
-  referrer = ""
+  referrer?: string
 ): Promise<SourceFileJson[]> {
   if (!specifiers.length) {
     return [];

cli/lib.rs

@@ -176,7 +176,7 @@ async fn print_file_info(worker: Worker, module_specifier: ModuleSpecifier) {
 
   let maybe_source_file = global_state_
     .file_fetcher
-    .fetch_source_file_async(&module_specifier)
+    .fetch_source_file_async(&module_specifier, None)
     .await;
   if let Err(err) = maybe_source_file {
     println!("{}", err);
@@ -197,7 +197,7 @@ async fn print_file_info(worker: Worker, module_specifier: ModuleSpecifier) {
 
   let maybe_compiled = global_state_
     .clone()
-    .fetch_compiled_module(&module_specifier)
+    .fetch_compiled_module(&module_specifier, None)
     .await;
   if let Err(e) = maybe_compiled {
     debug!("compiler error exiting!");

cli/ops/compiler.rs

@@ -51,7 +51,7 @@ fn op_cache(
 #[derive(Deserialize)]
 struct FetchSourceFilesArgs {
   specifiers: Vec<String>,
-  referrer: String,
+  referrer: Option<String>,
 }
 
 fn op_fetch_source_files(
@@ -65,14 +65,23 @@ fn op_fetch_source_files(
   // to this. Need a test to demonstrate the hole.
   let is_dyn_import = false;
 
+  let (referrer, ref_specifier) = if let Some(referrer) = args.referrer {
+    let specifier = ModuleSpecifier::resolve_url(&referrer)
+      .expect("Referrer is not a valid specifier");
+    (referrer, Some(specifier))
+  } else {
+    // main script import
+    (".".to_string(), None)
+  };
+
   let mut futures = vec![];
   for specifier in &args.specifiers {
     let resolved_specifier =
-      state.resolve(specifier, &args.referrer, false, is_dyn_import)?;
+      state.resolve(specifier, &referrer, false, is_dyn_import)?;
     let fut = state
       .global_state
       .file_fetcher
-      .fetch_source_file_async(&resolved_specifier);
+      .fetch_source_file_async(&resolved_specifier, ref_specifier.clone());
     futures.push(fut);
   }
 

cli/state.rs

@@ -178,12 +178,13 @@ impl Loader for ThreadSafeState {
   fn load(
     &self,
     module_specifier: &ModuleSpecifier,
+    maybe_referrer: Option<ModuleSpecifier>,
   ) -> Pin<Box<deno::SourceCodeInfoFuture>> {
     self.metrics.resolve_count.fetch_add(1, Ordering::SeqCst);
     let module_url_specified = module_specifier.to_string();
     let fut = self
       .global_state
-      .fetch_compiled_module(module_specifier)
+      .fetch_compiled_module(module_specifier, maybe_referrer)
       .map_ok(|compiled_module| deno::SourceCodeInfo {
         // Real module name, might be different from initial specifier
         // due to redirections.

cli/tests/error_004_missing_module.ts.out

-[WILDCARD]error: Uncaught NotFound: Cannot resolve module "[WILDCARD]/bad-module.ts"
+[WILDCARD]error: Uncaught NotFound: Cannot resolve module "[WILDCARD]/bad-module.ts" from "[WILDCARD]/error_004_missing_module.ts"
 [WILDCARD]dispatch_json.ts:[WILDCARD]
     at DenoError ([WILDCARD]errors.ts:[WILDCARD])
     at unwrapResponse ([WILDCARD]dispatch_json.ts:[WILDCARD])

cli/tests/error_005_missing_dynamic_import.ts.out

-[WILDCARD]error: Uncaught NotFound: Cannot resolve module "[WILDCARD]/bad-module.ts"
+[WILDCARD]error: Uncaught NotFound: Cannot resolve module "[WILDCARD]/bad-module.ts" from "[WILDCARD]/error_005_missing_dynamic_import.ts"
 [WILDCARD]dispatch_json.ts:[WILDCARD]
     at DenoError ([WILDCARD]errors.ts:[WILDCARD])
     at unwrapResponse ([WILDCARD]dispatch_json.ts:[WILDCARD])

cli/tests/error_006_import_ext_failure.ts.out

-[WILDCARD]error: Uncaught NotFound: Cannot resolve module "[WILDCARD]/non-existent"
+[WILDCARD]error: Uncaught NotFound: Cannot resolve module "[WILDCARD]/non-existent" from "[WILDCARD]/error_006_import_ext_failure.ts"
 [WILDCARD]dispatch_json.ts:[WILDCARD]
     at DenoError ([WILDCARD]errors.ts:[WILDCARD])
     at unwrapResponse ([WILDCARD]dispatch_json.ts:[WILDCARD])

core/modules.rs

@@ -48,6 +48,7 @@ pub trait Loader: Send + Sync {
   fn load(
     &self,
     module_specifier: &ModuleSpecifier,
+    maybe_referrer: Option<ModuleSpecifier>,
   ) -> Pin<Box<SourceCodeInfoFuture>>;
 }
 
@@ -154,7 +155,7 @@ impl<L: Loader + Unpin> RecursiveLoad<L> {
     // integrated into one thing.
     self
       .pending
-      .push(self.loader.load(&module_specifier).boxed());
+      .push(self.loader.load(&module_specifier, None).boxed());
     self.state = State::LoadingRoot;
 
     Ok(())
@@ -166,6 +167,8 @@ impl<L: Loader + Unpin> RecursiveLoad<L> {
     referrer: &str,
     parent_id: deno_mod,
   ) -> Result<(), ErrBox> {
+    let referrer_specifier = ModuleSpecifier::resolve_url(referrer)
+      .expect("Referrer should be a valid specifier");
     let module_specifier = self.loader.resolve(
       specifier,
       referrer,
@@ -181,9 +184,10 @@ impl<L: Loader + Unpin> RecursiveLoad<L> {
     if !modules.is_registered(module_name)
       && !self.is_pending.contains(&module_specifier)
     {
-      self
-        .pending
-        .push(self.loader.load(&module_specifier).boxed());
+      let fut = self
+        .loader
+        .load(&module_specifier, Some(referrer_specifier.clone()));
+      self.pending.push(fut.boxed());
       self.is_pending.insert(module_specifier);
     }
 
@@ -739,6 +743,7 @@ mod tests {
     fn load(
       &self,
       module_specifier: &ModuleSpecifier,
+      _maybe_referrer: Option<ModuleSpecifier>,
     ) -> Pin<Box<SourceCodeInfoFuture>> {
       let mut loads = self.loads.lock().unwrap();
       loads.push(module_specifier.to_string());