refactor: Rewrite usages of V8 inspector to the new API (denoland#30743)

Based on denoland/deno_core#1193.

This commit rewrites 3 parts of the system to use a new "sync"
V8 inspector API exposed by `deno_core`:
- REPL
- coverage collection
- hot module replacement

Turns out the async abstraction over V8 inspector was unnecessary
and actually greatly complicated usage of the inspector.

Towards denoland#13572
Towards denoland#13206

Author: bartlomieju

Cargo.lock

@@ -62,7 +62,7 @@ repository = "https://github.com/denoland/deno"
 
 [workspace.dependencies]
 deno_ast = { version = "=0.50.0", features = ["transpiling"] }
-deno_core = { version = "0.357.0" }
+deno_core = { version = "0.358.0" }
 
 deno_cache_dir = "=0.25.0"
 deno_doc = "=0.183.0"

Cargo.toml

@@ -99,10 +99,10 @@ use crate::resolver::CliResolver;
 use crate::resolver::on_resolve_diagnostic;
 use crate::standalone::binary::DenoCompileBinaryWriter;
 use crate::sys::CliSys;
-use crate::tools::coverage::CoverageCollector;
+use crate::tools::coverage::CoverageCollectorState;
 use crate::tools::installer::BinNameResolver;
 use crate::tools::lint::LintRuleProvider;
-use crate::tools::run::hmr::HmrRunner;
+use crate::tools::run::hmr::HmrRunnerState;
 use crate::tsc::TypeCheckingCjsTracker;
 use crate::type_checker::TypeChecker;
 use crate::util::file_watcher::WatcherCommunicator;
@@ -1106,8 +1106,8 @@ impl CliFactory {
     let create_hmr_runner = if cli_options.has_hmr() {
       let watcher_communicator = self.watcher_communicator.clone().unwrap();
       let emitter = self.emitter()?.clone();
-      let fn_: crate::worker::CreateHmrRunnerCb = Box::new(move |session| {
-        HmrRunner::new(emitter.clone(), session, watcher_communicator.clone())
+      let fn_: crate::worker::CreateHmrRunnerCb = Box::new(move || {
+        HmrRunnerState::new(emitter.clone(), watcher_communicator.clone())
       });
       Some(fn_)
     } else {
@@ -1116,9 +1116,7 @@ impl CliFactory {
     let create_coverage_collector =
       if let Some(coverage_dir) = cli_options.coverage_dir() {
         let fn_: crate::worker::CreateCoverageCollectorCb =
-          Box::new(move |session| {
-            CoverageCollector::new(coverage_dir.clone(), session)
-          });
+          Box::new(move || CoverageCollectorState::new(coverage_dir.clone()));
         Some(fn_)
       } else {
         None

cli/factory.rs

@@ -816,8 +816,11 @@ impl LibMainWorker {
   }
 
   #[inline]
-  pub fn create_inspector_session(&mut self) -> LocalInspectorSession {
-    self.worker.create_inspector_session()
+  pub fn create_inspector_session(
+    &mut self,
+    cb: deno_core::InspectorSessionSend,
+  ) -> LocalInspectorSession {
+    self.worker.create_inspector_session(cb)
   }
 
   #[inline]

cli/lib/worker.rs

@@ -7,6 +7,7 @@ use std::io::Write;
 use std::path::Path;
 use std::path::PathBuf;
 use std::sync::Arc;
+use std::sync::atomic::AtomicI32;
 
 use deno_ast::MediaType;
 use deno_ast::ModuleKind;
@@ -15,17 +16,15 @@ use deno_config::glob::FileCollector;
 use deno_config::glob::FilePatterns;
 use deno_config::glob::PathOrPattern;
 use deno_config::glob::PathOrPatternSet;
-use deno_core::InspectorPostMessageError;
-use deno_core::InspectorPostMessageErrorKind;
 use deno_core::LocalInspectorSession;
 use deno_core::anyhow::Context;
 use deno_core::anyhow::anyhow;
 use deno_core::error::AnyError;
 use deno_core::error::CoreError;
+use deno_core::parking_lot::Mutex;
 use deno_core::serde_json;
 use deno_core::sourcemap::SourceMap;
 use deno_core::url::Url;
-use deno_error::JsErrorBox;
 use deno_resolver::npm::DenoInNpmPackageChecker;
 use node_resolver::InNpmPackageChecker;
 use regex::Regex;
@@ -55,36 +54,47 @@ pub mod reporter;
 mod util;
 use merge::ProcessCoverage;
 
-pub struct CoverageCollector {
-  pub dir: PathBuf,
-  session: LocalInspectorSession,
+static NEXT_MSG_ID: AtomicI32 = AtomicI32::new(0);
+
+fn next_msg_id() -> i32 {
+  NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed)
 }
 
-impl CoverageCollector {
-  pub fn new(dir: PathBuf, session: LocalInspectorSession) -> Self {
-    Self { dir, session }
-  }
+#[derive(Debug)]
+pub struct CoverageCollectorInner {
+  dir: PathBuf,
+  coverage_msg_id: Option<i32>,
+}
 
-  pub async fn start_collecting(
-    &mut self,
-  ) -> Result<(), InspectorPostMessageError> {
-    self.enable_debugger().await?;
-    self.enable_profiler().await?;
-    self
-      .start_precise_coverage(cdp::StartPreciseCoverageArgs {
-        call_count: true,
-        detailed: true,
-        allow_triggered_updates: false,
-      })
-      .await?;
+#[derive(Clone, Debug)]
+pub struct CoverageCollectorState(Arc<Mutex<CoverageCollectorInner>>);
 
-    Ok(())
+impl CoverageCollectorState {
+  pub fn new(dir: PathBuf) -> Self {
+    Self(Arc::new(Mutex::new(CoverageCollectorInner {
+      dir,
+      coverage_msg_id: None,
+    })))
   }
 
-  pub async fn stop_collecting(&mut self) -> Result<(), CoreError> {
-    fs::create_dir_all(&self.dir)?;
+  pub fn callback(&self, msg: deno_core::InspectorMsg) {
+    let deno_core::InspectorMsgKind::Message(msg_id) = msg.kind else {
+      return;
+    };
+    let maybe_coverage_msg_id = self.0.lock().coverage_msg_id.as_ref().cloned();
+
+    if let Some(coverage_msg_id) = maybe_coverage_msg_id
+      && coverage_msg_id == msg_id
+    {
+      let message: serde_json::Value =
+        serde_json::from_str(&msg.content).unwrap();
+      let coverages: cdp::TakePreciseCoverageResponse =
+        serde_json::from_value(message["result"].clone()).unwrap();
+      self.write_coverages(coverages.result);
+    }
+  }
 
-    let script_coverages = self.take_precise_coverage().await?.result;
+  fn write_coverages(&self, script_coverages: Vec<cdp::ScriptCoverage>) {
     for script_coverage in script_coverages {
       // Filter out internal and http/https JS files, eval'd scripts,
       // and scripts with invalid urls from being included in coverage reports
@@ -100,92 +110,86 @@ impl CoverageCollector {
       }
 
       let filename = format!("{}.json", Uuid::new_v4());
-      let filepath = self.dir.join(filename);
-
-      let mut out = BufWriter::new(File::create(&filepath)?);
-      let coverage = serde_json::to_string(&script_coverage)
-        .map_err(JsErrorBox::from_err)?;
+      let filepath = self.0.lock().dir.join(filename);
+
+      let file = match File::create(&filepath) {
+        Ok(f) => f,
+        Err(err) => {
+          log::error!(
+            "Failed to create coverage file at {:?}, reason: {:?}",
+            filepath,
+            err
+          );
+          continue;
+        }
+      };
+      let mut out = BufWriter::new(file);
+      let coverage = serde_json::to_string(&script_coverage).unwrap();
       let formatted_coverage =
         format_json(&filepath, &coverage, &Default::default())
           .ok()
           .flatten()
           .unwrap_or(coverage);
 
-      out.write_all(formatted_coverage.as_bytes())?;
-      out.flush()?;
+      if let Err(err) = out.write_all(formatted_coverage.as_bytes()) {
+        log::error!(
+          "Failed to write coverage file at {:?}, reason: {:?}",
+          filepath,
+          err
+        );
+        continue;
+      }
+      if let Err(err) = out.flush() {
+        log::error!(
+          "Failed to flush coverage file at {:?}, reason: {:?}",
+          filepath,
+          err
+        );
+        continue;
+      }
     }
-
-    self.disable_debugger().await?;
-    self.disable_profiler().await?;
-
-    Ok(())
-  }
-
-  async fn enable_debugger(&mut self) -> Result<(), InspectorPostMessageError> {
-    self
-      .session
-      .post_message::<()>("Debugger.enable", None)
-      .await?;
-    Ok(())
   }
+}
 
-  async fn enable_profiler(&mut self) -> Result<(), InspectorPostMessageError> {
-    self
-      .session
-      .post_message::<()>("Profiler.enable", None)
-      .await?;
-    Ok(())
-  }
+pub struct CoverageCollector {
+  pub state: CoverageCollectorState,
+  session: LocalInspectorSession,
+}
 
-  async fn disable_debugger(
-    &mut self,
-  ) -> Result<(), InspectorPostMessageError> {
-    self
-      .session
-      .post_message::<()>("Debugger.disable", None)
-      .await?;
-    Ok(())
+impl CoverageCollector {
+  pub fn new(
+    state: CoverageCollectorState,
+    session: LocalInspectorSession,
+  ) -> Self {
+    Self { state, session }
   }
 
-  async fn disable_profiler(
-    &mut self,
-  ) -> Result<(), InspectorPostMessageError> {
+  pub fn start_collecting(&mut self) {
     self
       .session
-      .post_message::<()>("Profiler.disable", None)
-      .await?;
-    Ok(())
-  }
-
-  async fn start_precise_coverage(
-    &mut self,
-    parameters: cdp::StartPreciseCoverageArgs,
-  ) -> Result<cdp::StartPreciseCoverageResponse, InspectorPostMessageError> {
-    let return_value = self
-      .session
-      .post_message("Profiler.startPreciseCoverage", Some(parameters))
-      .await?;
-
-    let return_object = serde_json::from_value(return_value).map_err(|e| {
-      InspectorPostMessageErrorKind::JsBox(JsErrorBox::from_err(e)).into_box()
-    })?;
-
-    Ok(return_object)
+      .post_message::<()>(next_msg_id(), "Profiler.enable", None);
+    self.session.post_message(
+      next_msg_id(),
+      "Profiler.startPreciseCoverage",
+      Some(cdp::StartPreciseCoverageArgs {
+        call_count: true,
+        detailed: true,
+        allow_triggered_updates: false,
+      }),
+    );
   }
 
-  async fn take_precise_coverage(
-    &mut self,
-  ) -> Result<cdp::TakePreciseCoverageResponse, InspectorPostMessageError> {
-    let return_value = self
-      .session
-      .post_message::<()>("Profiler.takePreciseCoverage", None)
-      .await?;
+  pub fn stop_collecting(&mut self) -> Result<(), CoreError> {
+    fs::create_dir_all(&self.state.0.lock().dir)?;
+    let msg_id = next_msg_id();
+    self.state.0.lock().coverage_msg_id.replace(msg_id);
 
-    let return_object = serde_json::from_value(return_value).map_err(|e| {
-      InspectorPostMessageErrorKind::JsBox(JsErrorBox::from_err(e)).into_box()
-    })?;
-
-    Ok(return_object)
+    self.session.post_message::<()>(
+      msg_id,
+      "Profiler.takePreciseCoverage",
+      None,
+    );
+    Ok(())
   }
 }