From 6bc39505b0ee4365bfba947fc00bb9656441aeeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Thu, 11 Sep 2025 01:50:03 +0200 Subject: [PATCH 01/20] coverage collector compiles! --- Cargo.lock | 6 -- Cargo.toml | 3 + cli/factory.rs | 6 +- cli/lib/worker.rs | 9 ++ cli/tools/coverage/mod.rs | 171 ++++++++++++++++++++------------------ cli/tools/test/mod.rs | 11 +-- cli/worker.rs | 39 ++++----- runtime/worker.rs | 20 +++++ 8 files changed, 140 insertions(+), 125 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 49016997f32e22..dccd776dfc97aa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1931,8 +1931,6 @@ dependencies = [ [[package]] name = "deno_core" version = "0.357.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "aa0cf9203866302d1321e45e3a15f2cd799fdcee469a9e64f314fdeac86ce357" dependencies = [ "anyhow", "az", @@ -2649,8 +2647,6 @@ dependencies = [ [[package]] name = "deno_ops" version = "0.233.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9c7689c66ee4172042c1da65119905f1d31948e32f75473861a9ff70e254603f" dependencies = [ "indexmap 2.9.0", "proc-macro-rules", @@ -8252,8 +8248,6 @@ dependencies = [ [[package]] name = "serde_v8" version = "0.266.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ca27ce1cc0f7bbe1d0274ce5efbfaf95cb8997503da85daf62076891b6d6e74e" dependencies = [ "deno_error", "num-bigint", diff --git a/Cargo.toml b/Cargo.toml index f753c51e1f12fd..889e2e445e2e82 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -537,3 +537,6 @@ opt-level = 3 opt-level = 3 [profile.release.package.deno_npm_cache] opt-level = 3 + +[patch.crates-io] +deno_core = { path = "../deno_core/core" } diff --git a/cli/factory.rs b/cli/factory.rs index 052d9e4697dd33..92b634a25564da 100644 --- a/cli/factory.rs +++ b/cli/factory.rs @@ -99,7 +99,7 @@ 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; @@ -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 diff --git a/cli/lib/worker.rs b/cli/lib/worker.rs index 5cef55cea108b8..822b3117f6f521 100644 --- a/cli/lib/worker.rs +++ b/cli/lib/worker.rs @@ -24,6 +24,7 @@ use deno_runtime::deno_core::CompiledWasmModuleStore; use deno_runtime::deno_core::Extension; use deno_runtime::deno_core::JsRuntime; use deno_runtime::deno_core::LocalInspectorSession; +use deno_runtime::deno_core::LocalSyncInspectorSession; use deno_runtime::deno_core::ModuleLoader; use deno_runtime::deno_core::SharedArrayBufferStore; use deno_runtime::deno_core::error::CoreError; @@ -820,6 +821,14 @@ impl LibMainWorker { self.worker.create_inspector_session() } + #[inline] + pub fn create_sync_inspector_session( + &mut self, + cb: deno_core::InspectorSessionSend, + ) -> LocalSyncInspectorSession { + self.worker.create_sync_inspector_session(cb) + } + #[inline] pub fn dispatch_load_event(&mut self) -> Result<(), Box> { self.worker.dispatch_load_event() diff --git a/cli/tools/coverage/mod.rs b/cli/tools/coverage/mod.rs index b858cc481e0112..1fc76da0f98fb7 100644 --- a/cli/tools/coverage/mod.rs +++ b/cli/tools/coverage/mod.rs @@ -15,13 +15,12 @@ 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::LocalSyncInspectorSession; 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; @@ -55,36 +54,34 @@ pub mod reporter; mod util; use merge::ProcessCoverage; -pub struct CoverageCollector { - pub dir: PathBuf, - session: LocalInspectorSession, +#[derive(Debug)] +pub struct CoverageCollectorInner { + dir: PathBuf, } -impl CoverageCollector { - pub fn new(dir: PathBuf, session: LocalInspectorSession) -> Self { - Self { dir, session } +#[derive(Clone, Debug)] +pub struct CoverageCollectorState(Arc>); + +impl CoverageCollectorState { + pub fn new(dir: PathBuf) -> Self { + Self(Arc::new(Mutex::new(CoverageCollectorInner { dir }))) } - 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?; + pub fn callback(&self, msg: deno_core::InspectorMsg) { + let deno_core::InspectorMsgKind::Message(_msg_id) = msg.kind else { + let message: serde_json::Value = + serde_json::from_str(&msg.content).unwrap(); + log::error!("CoverageCollector received a notification {:?}", message); + return; + }; - Ok(()) + // Check if we know this `msg_id` + let message: cdp::TakePreciseCoverageResponse = + serde_json::from_str(&msg.content).unwrap(); + self.write_coverages(message.result); } - pub async fn stop_collecting(&mut self) -> Result<(), CoreError> { - fs::create_dir_all(&self.dir)?; - - let script_coverages = self.take_precise_coverage().await?.result; + fn write_coverages(&self, script_coverages: Vec) { 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 +97,102 @@ impl CoverageCollector { } let filename = format!("{}.json", Uuid::new_v4()); - let filepath = self.dir.join(filename); + let filepath = self.0.lock().dir.join(filename); - let mut out = BufWriter::new(File::create(&filepath)?); + // TODO(bartlomieju): remove unwrap + let mut out = BufWriter::new(File::create(&filepath).unwrap()); + // TODO(bartlomieju): remove unwrap let coverage = serde_json::to_string(&script_coverage) - .map_err(JsErrorBox::from_err)?; + .map_err(JsErrorBox::from_err) + .unwrap(); let formatted_coverage = format_json(&filepath, &coverage, &Default::default()) .ok() .flatten() .unwrap_or(coverage); - out.write_all(formatted_coverage.as_bytes())?; - out.flush()?; + // TODO(bartlomieju): add logging + let _ = out.write_all(formatted_coverage.as_bytes()); + let _ = out.flush(); } + } +} - self.disable_debugger().await?; - self.disable_profiler().await?; +pub struct CoverageCollector { + pub state: CoverageCollectorState, + session: LocalSyncInspectorSession, +} - Ok(()) +impl CoverageCollector { + pub fn new( + state: CoverageCollectorState, + session: LocalSyncInspectorSession, + ) -> Self { + Self { state, session } } - async fn enable_debugger(&mut self) -> Result<(), InspectorPostMessageError> { - self - .session - .post_message::<()>("Debugger.enable", None) - .await?; - Ok(()) + pub fn start_collecting(&mut self) { + // TODO(barltomieju): is it actually necessary to call this one? + self.enable_debugger(); + + self.enable_profiler(); + self.start_precise_coverage(cdp::StartPreciseCoverageArgs { + call_count: true, + detailed: true, + allow_triggered_updates: false, + }); } - async fn enable_profiler(&mut self) -> Result<(), InspectorPostMessageError> { - self - .session - .post_message::<()>("Profiler.enable", None) - .await?; + pub fn stop_collecting(&mut self) -> Result<(), CoreError> { + fs::create_dir_all(&self.state.0.lock().dir)?; + self.take_precise_coverage(); + + // TODO(barltomieju): is it actually necessary to call these? + // self.disable_debugger(); + // self.disable_profiler(); + Ok(()) } - async fn disable_debugger( - &mut self, - ) -> Result<(), InspectorPostMessageError> { - self - .session - .post_message::<()>("Debugger.disable", None) - .await?; - Ok(()) + // TODO(barltomieju): is it actually necessary to call this? + fn enable_debugger(&mut self) { + self.session.post_message::<()>("Debugger.enable", None); } - async fn disable_profiler( - &mut self, - ) -> Result<(), InspectorPostMessageError> { - self - .session - .post_message::<()>("Profiler.disable", None) - .await?; - Ok(()) + fn enable_profiler(&mut self) { + self.session.post_message::<()>("Profiler.enable", None); } - async fn start_precise_coverage( + // TODO(barltomieju): is it actually necessary to call this? + // fn disable_debugger(&mut self) { + // self.session.post_message::<()>("Debugger.disable", None); + // } + + // TODO(barltomieju): is it actually necessary to call this? + // fn disable_profiler(&mut self) { + // self.session.post_message::<()>("Profiler.disable", None); + // } + + fn start_precise_coverage( &mut self, parameters: cdp::StartPreciseCoverageArgs, - ) -> Result { - let return_value = self + ) { + 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("Profiler.startPreciseCoverage", Some(parameters)); } - async fn take_precise_coverage( - &mut self, - ) -> Result { - let return_value = self + fn take_precise_coverage(&mut self) { + let _msg_id = self .session - .post_message::<()>("Profiler.takePreciseCoverage", None) - .await?; + .post_message::<()>("Profiler.takePreciseCoverage", None); - let return_object = serde_json::from_value(return_value).map_err(|e| { - InspectorPostMessageErrorKind::JsBox(JsErrorBox::from_err(e)).into_box() - })?; + // let return_object = serde_json::from_value(return_value).map_err(|e| { + // InspectorPostMessageErrorKind::JsBox(JsErrorBox::from_err(e)).into_box() + // })?; - Ok(return_object) + // Ok(return_object) + todo!() } } diff --git a/cli/tools/test/mod.rs b/cli/tools/test/mod.rs index 59cf04e1daa920..3d30d113629c28 100644 --- a/cli/tools/test/mod.rs +++ b/cli/tools/test/mod.rs @@ -36,7 +36,6 @@ use deno_core::error::AnyError; use deno_core::error::CoreError; use deno_core::error::CoreErrorKind; use deno_core::error::JsError; -use deno_core::futures::FutureExt; use deno_core::futures::StreamExt; use deno_core::futures::future; use deno_core::futures::stream; @@ -668,7 +667,7 @@ async fn configure_main_worker( None, ) .await?; - let coverage_collector = worker.maybe_setup_coverage_collector().await?; + let coverage_collector = worker.maybe_setup_coverage_collector()?; if options.trace_leaks { worker .execute_script_static( @@ -804,13 +803,7 @@ async fn test_specifier_inner( worker.run_up_to_duration(Duration::from_millis(0)).await?; if let Some(coverage_collector) = &mut coverage_collector { - worker - .js_runtime - .with_event_loop_future( - coverage_collector.stop_collecting().boxed_local(), - PollEventLoopOptions::default(), - ) - .await?; + coverage_collector.stop_collecting()?; } Ok(()) } diff --git a/cli/worker.rs b/cli/worker.rs index e467b4c1aba7ea..633dd7848908c9 100644 --- a/cli/worker.rs +++ b/cli/worker.rs @@ -30,6 +30,7 @@ use crate::npm::CliNpmInstaller; use crate::npm::CliNpmResolver; use crate::sys::CliSys; use crate::tools::coverage::CoverageCollector; +use crate::tools::coverage::CoverageCollectorState; use crate::tools::run::hmr::HmrRunner; use crate::util::file_watcher::WatcherCommunicator; use crate::util::file_watcher::WatcherRestartMode; @@ -38,9 +39,8 @@ use crate::util::progress_bar::ProgressBar; pub type CreateHmrRunnerCb = Box HmrRunner + Send + Sync>; -pub type CreateCoverageCollectorCb = Box< - dyn Fn(deno_core::LocalInspectorSession) -> CoverageCollector + Send + Sync, ->; +pub type CreateCoverageCollectorCb = + Box CoverageCollectorState + Send + Sync>; pub struct CliMainWorkerOptions { pub create_hmr_runner: Option, @@ -73,8 +73,7 @@ impl CliMainWorker { } pub async fn run(&mut self) -> Result { - let mut maybe_coverage_collector = - self.maybe_setup_coverage_collector().await?; + let mut maybe_coverage_collector = self.maybe_setup_coverage_collector()?; let mut maybe_hmr_runner = self.maybe_setup_hmr_runner().await?; // WARNING: Remember to update cli/lib/worker.rs to align with @@ -130,14 +129,7 @@ impl CliMainWorker { self.worker.dispatch_process_exit_event()?; if let Some(coverage_collector) = maybe_coverage_collector.as_mut() { - self - .worker - .js_runtime() - .with_event_loop_future( - coverage_collector.stop_collecting().boxed_local(), - PollEventLoopOptions::default(), - ) - .await?; + coverage_collector.stop_collecting()?; } if let Some(hmr_runner) = maybe_hmr_runner.as_mut() { self @@ -255,7 +247,7 @@ impl CliMainWorker { Ok(Some(hmr_runner)) } - pub async fn maybe_setup_coverage_collector( + pub fn maybe_setup_coverage_collector( &mut self, ) -> Result, CoreError> { let Some(create_coverage_collector) = @@ -264,16 +256,15 @@ impl CliMainWorker { return Ok(None); }; - let session = self.worker.create_inspector_session(); - let mut coverage_collector = create_coverage_collector(session); - self - .worker - .js_runtime() - .with_event_loop_future( - coverage_collector.start_collecting().boxed_local(), - PollEventLoopOptions::default(), - ) - .await?; + let coverage_collector_state = create_coverage_collector(); + let state = coverage_collector_state.clone(); + + let callback = + Box::new(move |message| coverage_collector_state.callback(message)); + let session = self.worker.create_sync_inspector_session(callback); + let mut coverage_collector = CoverageCollector::new(state, session); + coverage_collector.start_collecting(); + Ok(Some(coverage_collector)) } diff --git a/runtime/worker.rs b/runtime/worker.rs index 8c4890307e9bd1..0ec7f18ea872f8 100644 --- a/runtime/worker.rs +++ b/runtime/worker.rs @@ -20,6 +20,7 @@ use deno_core::InspectorSessionKind; use deno_core::InspectorSessionOptions; use deno_core::JsRuntime; use deno_core::LocalInspectorSession; +use deno_core::LocalSyncInspectorSession; use deno_core::ModuleCodeString; use deno_core::ModuleId; use deno_core::ModuleLoader; @@ -921,6 +922,25 @@ impl MainWorker { ) } + /// Create new inspector session. This function panics if Worker + /// was not configured to create inspector. + pub fn create_sync_inspector_session( + &mut self, + cb: deno_core::InspectorSessionSend, + ) -> LocalSyncInspectorSession { + self.js_runtime.maybe_init_inspector(); + self + .js_runtime + .inspector() + .borrow() + .create_local_sync_session( + cb, + InspectorSessionOptions { + kind: InspectorSessionKind::Blocking, + }, + ) + } + pub async fn run_event_loop( &mut self, wait_for_inspector: bool, From 7c659a14d45b176850ca288136fadb326b73bc6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Thu, 11 Sep 2025 02:21:47 +0200 Subject: [PATCH 02/20] watcher, but doesn't compile --- cli/factory.rs | 6 +- cli/tools/run/hmr.rs | 383 ++++++++++++++++++++++++------------------ cli/tools/test/mod.rs | 2 +- cli/worker.rs | 55 +++--- 4 files changed, 240 insertions(+), 206 deletions(-) diff --git a/cli/factory.rs b/cli/factory.rs index 92b634a25564da..00e0e303123b57 100644 --- a/cli/factory.rs +++ b/cli/factory.rs @@ -102,7 +102,7 @@ use crate::sys::CliSys; 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 { diff --git a/cli/tools/run/hmr.rs b/cli/tools/run/hmr.rs index cc003d3cd88d82..6ec1a1f319950d 100644 --- a/cli/tools/run/hmr.rs +++ b/cli/tools/run/hmr.rs @@ -6,9 +6,10 @@ use std::sync::Arc; use deno_core::InspectorPostMessageError; use deno_core::InspectorPostMessageErrorKind; -use deno_core::LocalInspectorSession; +use deno_core::LocalSyncInspectorSession; use deno_core::error::CoreError; use deno_core::futures::StreamExt; +use deno_core::parking_lot::Mutex; use deno_core::serde_json::json; use deno_core::serde_json::{self}; use deno_core::url::Url; @@ -62,6 +63,78 @@ fn should_retry(status: &cdp::Status) -> bool { } } +#[derive(Debug)] +pub struct HmrRunnerInner { + watcher_communicator: Arc, + script_ids: HashMap, + emitter: Arc, +} + +#[derive(Clone, Debug)] +pub struct HmrRunnerState(Arc>); + +impl HmrRunnerState { + pub fn new( + emitter: Arc, + watcher_communicator: Arc, + ) -> Self { + Self(Arc::new(Mutex::new(HmrRunnerInner { + emitter, + watcher_communicator, + script_ids: HashMap::new(), + }))) + } + + pub fn callback(&self, msg: deno_core::InspectorMsg) { + let deno_core::InspectorMsgKind::Message(_msg_id) = msg.kind else { + let message: serde_json::Value = + serde_json::from_str(&msg.content).unwrap(); + log::error!("CoverageCollector received a notification {:?}", message); + return; + }; + + // Check if we know this `msg_id` + let _message: cdp::TakePreciseCoverageResponse = + serde_json::from_str(&msg.content).unwrap(); + // self.write_coverages(message.result); + } + + fn handle_notification(&self, _notification: cdp::Notification) { + // if notification.method == "Runtime.exceptionThrown" { + // let exception_thrown = + // serde_json::from_value::(notification.params) + // .map_err(JsErrorBox::from_err)?; + // let (message, description) = exception_thrown + // .exception_details + // .get_message_and_description(); + // break Err( + // JsErrorBox::generic(format!("{} {}", message, description)).into(), + // ); + // } + + // if notification.method == "Debugger.scriptParsed" { + // let params = + // serde_json::from_value::(notification.params) + // .map_err(JsErrorBox::from_err)?; + // if params.url.starts_with("file://") { + // let file_url = Url::parse(¶ms.url).unwrap(); + // let file_path = file_url.to_file_path().unwrap(); + // if let Ok(canonicalized_file_path) = file_path.canonicalize() { + // let canonicalized_file_url = + // Url::from_file_path(canonicalized_file_path).unwrap(); + // self + // .0 + // .lock() + // .script_ids + // .insert(canonicalized_file_url.into(), params.script_id); + // } + // } + // } + + todo!() + } +} + /// This structure is responsible for providing Hot Module Replacement /// functionality. /// @@ -78,206 +151,184 @@ fn should_retry(status: &cdp::Status) -> bool { /// of an ES module cannot be hot-replaced. In such situation the runner will /// force a full restart of a program by notifying the `FileWatcher`. pub struct HmrRunner { - session: LocalInspectorSession, - watcher_communicator: Arc, - script_ids: HashMap, - emitter: Arc, + session: LocalSyncInspectorSession, + state: HmrRunnerState, } impl HmrRunner { pub fn new( - emitter: Arc, - session: LocalInspectorSession, - watcher_communicator: Arc, + state: HmrRunnerState, + session: LocalSyncInspectorSession, ) -> Self { - Self { - session, - emitter, - watcher_communicator, - script_ids: HashMap::new(), - } + Self { session, state } } // TODO(bartlomieju): this code is duplicated in `cli/tools/coverage/mod.rs` - pub async fn start(&mut self) -> Result<(), InspectorPostMessageError> { - self.enable_debugger().await + pub fn start(&mut self) { + self.enable_debugger(); } + fn watcher(&self) -> Arc { + self.state.0.lock().watcher_communicator.clone() + } // TODO(bartlomieju): this code is duplicated in `cli/tools/coverage/mod.rs` - pub async fn stop(&mut self) -> Result<(), InspectorPostMessageError> { + pub fn stop(&mut self) { self - .watcher_communicator + .watcher() .change_restart_mode(WatcherRestartMode::Automatic); - self.disable_debugger().await + self.disable_debugger(); } pub async fn run(&mut self) -> Result<(), CoreError> { - self - .watcher_communicator - .change_restart_mode(WatcherRestartMode::Manual); - let mut session_rx = self.session.take_notification_rx(); - loop { - select! { - biased; - Some(notification) = session_rx.next() => { - let notification = serde_json::from_value::(notification).map_err(JsErrorBox::from_err)?; - if notification.method == "Runtime.exceptionThrown" { - let exception_thrown = serde_json::from_value::(notification.params).map_err(JsErrorBox::from_err)?; - let (message, description) = exception_thrown.exception_details.get_message_and_description(); - break Err(JsErrorBox::generic(format!("{} {}", message, description)).into()); - } else if notification.method == "Debugger.scriptParsed" { - let params = serde_json::from_value::(notification.params).map_err(JsErrorBox::from_err)?; - if params.url.starts_with("file://") { - let file_url = Url::parse(¶ms.url).unwrap(); - let file_path = file_url.to_file_path().unwrap(); - if let Ok(canonicalized_file_path) = file_path.canonicalize() { - let canonicalized_file_url = Url::from_file_path(canonicalized_file_path).unwrap(); - self.script_ids.insert(canonicalized_file_url.into(), params.script_id); - } - } - } - } - changed_paths = self.watcher_communicator.watch_for_changed_paths() => { - let changed_paths = changed_paths.map_err(JsErrorBox::from_err)?; - - let Some(changed_paths) = changed_paths else { - let _ = self.watcher_communicator.force_restart(); - continue; - }; - - let filtered_paths: Vec = changed_paths.into_iter().filter(|p| p.extension().is_some_and(|ext| { - let ext_str = ext.to_str().unwrap(); - matches!(ext_str, "js" | "ts" | "jsx" | "tsx") - })).collect(); - - // If after filtering there are no paths it means it's either a file - // we can't HMR or an external file that was passed explicitly to - // `--watch-hmr=` path. - if filtered_paths.is_empty() { - let _ = self.watcher_communicator.force_restart(); - continue; - } + // self + // .watcher() + // .change_restart_mode(WatcherRestartMode::Manual); - for path in filtered_paths { - let Some(path_str) = path.to_str() else { - let _ = self.watcher_communicator.force_restart(); - continue; - }; - let Ok(module_url) = Url::from_file_path(path_str) else { - let _ = self.watcher_communicator.force_restart(); - continue; - }; - - let Some(id) = self.script_ids.get(module_url.as_str()).cloned() else { - let _ = self.watcher_communicator.force_restart(); - continue; - }; - - let source_code = tokio::fs::read_to_string(deno_path_util::url_to_file_path(&module_url).unwrap()).await?; - let source_code = self.emitter.emit_for_hmr( - &module_url, - source_code, - )?; - - let mut tries = 1; - loop { - let result = self.set_script_source(&id, source_code.as_str()).await?; - - if matches!(result.status, cdp::Status::Ok) { - self.dispatch_hmr_event(module_url.as_str()).await?; - self.watcher_communicator.print(format!("Replaced changed module {}", module_url.as_str())); - break; - } - - self.watcher_communicator.print(format!("Failed to reload module {}: {}.", module_url, colors::gray(&explain(&result)))); - if should_retry(&result.status) && tries <= 2 { - tries += 1; - tokio::time::sleep(std::time::Duration::from_millis(100)).await; - continue; - } - - let _ = self.watcher_communicator.force_restart(); - break; - } - } - } - _ = self.session.receive_from_v8_session() => {} - } - } + // loop { + // select! { + // biased; + // // Some(notification) = session_rx.next() => { + // // let notification = serde_json::from_value::(notification).map_err(JsErrorBox::from_err)?; + // // if notification.method == "Runtime.exceptionThrown" { + // // let exception_thrown = serde_json::from_value::(notification.params).map_err(JsErrorBox::from_err)?; + // // let (message, description) = exception_thrown.exception_details.get_message_and_description(); + // // break Err(JsErrorBox::generic(format!("{} {}", message, description)).into()); + // // } else if notification.method == "Debugger.scriptParsed" { + // // let params = serde_json::from_value::(notification.params).map_err(JsErrorBox::from_err)?; + // // if params.url.starts_with("file://") { + // // let file_url = Url::parse(¶ms.url).unwrap(); + // // let file_path = file_url.to_file_path().unwrap(); + // // if let Ok(canonicalized_file_path) = file_path.canonicalize() { + // // let canonicalized_file_url = Url::from_file_path(canonicalized_file_path).unwrap(); + // // self.state.0.lock().script_ids.insert(canonicalized_file_url.into(), params.script_id); + // // } + // // } + // // } + // // } + // changed_paths = self.watcher().watch_for_changed_paths() => { + // let changed_paths = changed_paths.map_err(JsErrorBox::from_err)?; + + // let Some(changed_paths) = changed_paths else { + // let _ = self.watcher().force_restart(); + // continue; + // }; + + // let filtered_paths: Vec = changed_paths.into_iter().filter(|p| p.extension().is_some_and(|ext| { + // let ext_str = ext.to_str().unwrap(); + // matches!(ext_str, "js" | "ts" | "jsx" | "tsx") + // })).collect(); + + // // If after filtering there are no paths it means it's either a file + // // we can't HMR or an external file that was passed explicitly to + // // `--watch-hmr=` path. + // if filtered_paths.is_empty() { + // let _ = self.watcher().force_restart(); + // continue; + // } + + // for path in filtered_paths { + // let Some(path_str) = path.to_str() else { + // let _ = self.watcher().force_restart(); + // continue; + // }; + // let Ok(module_url) = Url::from_file_path(path_str) else { + // let _ = self.watcher().force_restart(); + // continue; + // }; + + // let Some(id) = self.state.0.lock().script_ids.get(module_url.as_str()).cloned() else { + // let _ = self.watcher().force_restart(); + // continue; + // }; + + // let source_code = tokio::fs::read_to_string(deno_path_util::url_to_file_path(&module_url).unwrap()).await?; + // let source_code = self.state.0.lock().emitter.emit_for_hmr( + // &module_url, + // source_code, + // )?; + + // let mut tries = 1; + // loop { + // let result = self.set_script_source(&id, source_code.as_str()).await?; + + // if matches!(result.status, cdp::Status::Ok) { + // self.dispatch_hmr_event(module_url.as_str()); + // self.watcher().print(format!("Replaced changed module {}", module_url.as_str())); + // break; + // } + + // self.watcher().print(format!("Failed to reload module {}: {}.", module_url, colors::gray(&explain(&result)))); + // if should_retry(&result.status) && tries <= 2 { + // tries += 1; + // tokio::time::sleep(std::time::Duration::from_millis(100)).await; + // continue; + // } + + // let _ = self.watcher().force_restart(); + // break; + // } + // } + // } + // } + // } + + todo!() } // TODO(bartlomieju): this code is duplicated in `cli/tools/coverage/mod.rs` - async fn enable_debugger(&mut self) -> Result<(), InspectorPostMessageError> { - self - .session - .post_message::<()>("Debugger.enable", None) - .await?; - self - .session - .post_message::<()>("Runtime.enable", None) - .await?; - Ok(()) + fn enable_debugger(&mut self) { + // TODO(barltomieju): is it actually necessary to call this? + self.session.post_message::<()>("Debugger.enable", None); + + self.session.post_message::<()>("Runtime.enable", None); } // TODO(bartlomieju): this code is duplicated in `cli/tools/coverage/mod.rs` - async fn disable_debugger( - &mut self, - ) -> Result<(), InspectorPostMessageError> { - self - .session - .post_message::<()>("Debugger.disable", None) - .await?; - self - .session - .post_message::<()>("Runtime.disable", None) - .await?; - Ok(()) + fn disable_debugger(&mut self) { + // TODO(barltomieju): is it actually necessary to call this? + self.session.post_message::<()>("Debugger.disable", None); + // TODO(barltomieju): is it actually necessary to call this? + self.session.post_message::<()>("Runtime.disable", None); } - async fn set_script_source( + #[allow(unused)] + fn set_script_source( &mut self, script_id: &str, source: &str, ) -> Result { - let result = self - .session - .post_message( - "Debugger.setScriptSource", - Some(json!({ - "scriptId": script_id, - "scriptSource": source, - "allowTopFrameEditing": true, - })), - ) - .await?; - - serde_json::from_value::(result).map_err( - |e| { - InspectorPostMessageErrorKind::JsBox(JsErrorBox::from_err(e)).into_box() - }, - ) + todo!() + // let result = self + // .session + // .post_message( + // "Debugger.setScriptSource", + // Some(json!({ + // "scriptId": script_id, + // "scriptSource": source, + // "allowTopFrameEditing": true, + // })), + // ) + // .await?; + + // serde_json::from_value::(result).map_err( + // |e| { + // InspectorPostMessageErrorKind::JsBox(JsErrorBox::from_err(e)).into_box() + // }, + // ) } - async fn dispatch_hmr_event( - &mut self, - script_id: &str, - ) -> Result<(), InspectorPostMessageError> { + fn dispatch_hmr_event(&mut self, script_id: &str) { let expr = format!( "dispatchEvent(new CustomEvent(\"hmr\", {{ detail: {{ path: \"{}\" }} }}));", script_id ); - let _result = self - .session - .post_message( - "Runtime.evaluate", - Some(json!({ - "expression": expr, - "contextId": Some(1), - })), - ) - .await?; - - Ok(()) + self.session.post_message( + "Runtime.evaluate", + Some(json!({ + "expression": expr, + "contextId": Some(1), + })), + ); } } diff --git a/cli/tools/test/mod.rs b/cli/tools/test/mod.rs index 3d30d113629c28..bd4e1a99c72d44 100644 --- a/cli/tools/test/mod.rs +++ b/cli/tools/test/mod.rs @@ -667,7 +667,7 @@ async fn configure_main_worker( None, ) .await?; - let coverage_collector = worker.maybe_setup_coverage_collector()?; + let coverage_collector = worker.maybe_setup_coverage_collector(); if options.trace_leaks { worker .execute_script_static( diff --git a/cli/worker.rs b/cli/worker.rs index 633dd7848908c9..763ec1bc5fc857 100644 --- a/cli/worker.rs +++ b/cli/worker.rs @@ -32,12 +32,12 @@ use crate::sys::CliSys; use crate::tools::coverage::CoverageCollector; use crate::tools::coverage::CoverageCollectorState; use crate::tools::run::hmr::HmrRunner; +use crate::tools::run::hmr::HmrRunnerState; use crate::util::file_watcher::WatcherCommunicator; use crate::util::file_watcher::WatcherRestartMode; use crate::util::progress_bar::ProgressBar; -pub type CreateHmrRunnerCb = - Box HmrRunner + Send + Sync>; +pub type CreateHmrRunnerCb = Box HmrRunnerState + Send + Sync>; pub type CreateCoverageCollectorCb = Box CoverageCollectorState + Send + Sync>; @@ -73,8 +73,8 @@ impl CliMainWorker { } pub async fn run(&mut self) -> Result { - let mut maybe_coverage_collector = self.maybe_setup_coverage_collector()?; - let mut maybe_hmr_runner = self.maybe_setup_hmr_runner().await?; + let mut maybe_coverage_collector = self.maybe_setup_coverage_collector(); + let mut maybe_hmr_runner = self.maybe_setup_hmr_runner(); // WARNING: Remember to update cli/lib/worker.rs to align with // changes made here so that they affect deno_compile as well. @@ -110,6 +110,7 @@ impl CliMainWorker { return Err(e); } } else { + // TODO(bartlomieju): this might not be needed anymore self .worker .run_event_loop(maybe_coverage_collector.is_none()) @@ -132,14 +133,7 @@ impl CliMainWorker { coverage_collector.stop_collecting()?; } if let Some(hmr_runner) = maybe_hmr_runner.as_mut() { - self - .worker - .js_runtime() - .with_event_loop_future( - hmr_runner.stop().boxed_local(), - PollEventLoopOptions::default(), - ) - .await?; + hmr_runner.stop(); } Ok(self.worker.exit_code()) @@ -225,36 +219,25 @@ impl CliMainWorker { self.worker.js_runtime().op_state() } - pub async fn maybe_setup_hmr_runner( - &mut self, - ) -> Result, CoreError> { - let Some(setup_hmr_runner) = self.shared.create_hmr_runner.as_ref() else { - return Ok(None); - }; + pub fn maybe_setup_hmr_runner(&mut self) -> Option { + let setup_hmr_runner = self.shared.create_hmr_runner.as_ref()?; - let session = self.worker.create_inspector_session(); + let hmr_runner_state = setup_hmr_runner(); + let state = hmr_runner_state.clone(); - let mut hmr_runner = setup_hmr_runner(session); + let callback = Box::new(move |message| hmr_runner_state.callback(message)); + let session = self.worker.create_sync_inspector_session(callback); + let mut hmr_runner = HmrRunner::new(state, session); + hmr_runner.start(); - self - .worker - .js_runtime() - .with_event_loop_future( - hmr_runner.start().boxed_local(), - PollEventLoopOptions::default(), - ) - .await?; - Ok(Some(hmr_runner)) + Some(hmr_runner) } pub fn maybe_setup_coverage_collector( &mut self, - ) -> Result, CoreError> { - let Some(create_coverage_collector) = - self.shared.create_coverage_collector.as_ref() - else { - return Ok(None); - }; + ) -> Option { + let create_coverage_collector = + self.shared.create_coverage_collector.as_ref()?; let coverage_collector_state = create_coverage_collector(); let state = coverage_collector_state.clone(); @@ -265,7 +248,7 @@ impl CliMainWorker { let mut coverage_collector = CoverageCollector::new(state, session); coverage_collector.start_collecting(); - Ok(Some(coverage_collector)) + Some(coverage_collector) } pub fn execute_script_static( From 68a1d057d25476dc8ee50b62a85a72cc675b7db2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Thu, 11 Sep 2025 23:09:56 +0200 Subject: [PATCH 03/20] just hangs on coverage collection - deno_core is still async wrt to inspector sessions --- cli/tools/coverage/mod.rs | 54 +++-- cli/tools/run/hmr.rs | 353 +++++++++++++++------------- cli/tools/test/mod.rs | 1 + cli/worker.rs | 1 - tests/integration/coverage_tests.rs | 1 + 5 files changed, 228 insertions(+), 182 deletions(-) diff --git a/cli/tools/coverage/mod.rs b/cli/tools/coverage/mod.rs index 1fc76da0f98fb7..0e7dc408e3c2bd 100644 --- a/cli/tools/coverage/mod.rs +++ b/cli/tools/coverage/mod.rs @@ -57,6 +57,7 @@ use merge::ProcessCoverage; #[derive(Debug)] pub struct CoverageCollectorInner { dir: PathBuf, + coverage_msg_id: Option, } #[derive(Clone, Debug)] @@ -64,21 +65,35 @@ pub struct CoverageCollectorState(Arc>); impl CoverageCollectorState { pub fn new(dir: PathBuf) -> Self { - Self(Arc::new(Mutex::new(CoverageCollectorInner { dir }))) + Self(Arc::new(Mutex::new(CoverageCollectorInner { + dir, + coverage_msg_id: None, + }))) } pub fn callback(&self, msg: deno_core::InspectorMsg) { - let deno_core::InspectorMsgKind::Message(_msg_id) = msg.kind else { - let message: serde_json::Value = + let deno_core::InspectorMsgKind::Message(msg_id) = msg.kind else { + let _message: serde_json::Value = serde_json::from_str(&msg.content).unwrap(); - log::error!("CoverageCollector received a notification {:?}", message); + // log::error!("CoverageCollector received a notification {:?}", message); return; }; - // Check if we know this `msg_id` - let message: cdp::TakePreciseCoverageResponse = - serde_json::from_str(&msg.content).unwrap(); - self.write_coverages(message.result); + eprintln!( + "callback {} {:#?} {}", + msg_id, + self.0.lock().coverage_msg_id.as_ref(), + msg.content + ); + if let Some(coverage_msg_id) = self.0.lock().coverage_msg_id.as_ref() + && *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); + } } fn write_coverages(&self, script_coverages: Vec) { @@ -148,8 +163,8 @@ impl CoverageCollector { self.take_precise_coverage(); // TODO(barltomieju): is it actually necessary to call these? - // self.disable_debugger(); - // self.disable_profiler(); + self.disable_debugger(); + self.disable_profiler(); Ok(()) } @@ -164,14 +179,14 @@ impl CoverageCollector { } // TODO(barltomieju): is it actually necessary to call this? - // fn disable_debugger(&mut self) { - // self.session.post_message::<()>("Debugger.disable", None); - // } + fn disable_debugger(&mut self) { + self.session.post_message::<()>("Debugger.disable", None); + } // TODO(barltomieju): is it actually necessary to call this? - // fn disable_profiler(&mut self) { - // self.session.post_message::<()>("Profiler.disable", None); - // } + fn disable_profiler(&mut self) { + self.session.post_message::<()>("Profiler.disable", None); + } fn start_precise_coverage( &mut self, @@ -183,16 +198,17 @@ impl CoverageCollector { } fn take_precise_coverage(&mut self) { - let _msg_id = self + let msg_id = self .session .post_message::<()>("Profiler.takePreciseCoverage", None); - + eprintln!("take precise coverage {}", 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) - todo!() + // todo!() } } diff --git a/cli/tools/run/hmr.rs b/cli/tools/run/hmr.rs index 6ec1a1f319950d..b223c660f19677 100644 --- a/cli/tools/run/hmr.rs +++ b/cli/tools/run/hmr.rs @@ -8,7 +8,6 @@ use deno_core::InspectorPostMessageError; use deno_core::InspectorPostMessageErrorKind; use deno_core::LocalSyncInspectorSession; use deno_core::error::CoreError; -use deno_core::futures::StreamExt; use deno_core::parking_lot::Mutex; use deno_core::serde_json::json; use deno_core::serde_json::{self}; @@ -16,6 +15,10 @@ use deno_core::url::Url; use deno_error::JsErrorBox; use deno_terminal::colors; use tokio::select; +use tokio::sync::mpsc; +use tokio::sync::mpsc::UnboundedReceiver; +use tokio::sync::mpsc::UnboundedSender; +use tokio::sync::oneshot; use crate::cdp; use crate::module_loader::CliEmitter; @@ -63,11 +66,20 @@ fn should_retry(status: &cdp::Status) -> bool { } } +#[derive(Debug)] +enum InspectorMessageState { + Ready(serde_json::Value), + WaitingFor(oneshot::Sender), +} + #[derive(Debug)] pub struct HmrRunnerInner { watcher_communicator: Arc, script_ids: HashMap, + messages: HashMap, emitter: Arc, + exception_tx: UnboundedSender, + exception_rx: Option>, } #[derive(Clone, Debug)] @@ -78,60 +90,76 @@ impl HmrRunnerState { emitter: Arc, watcher_communicator: Arc, ) -> Self { + let (exception_tx, exception_rx) = mpsc::unbounded_channel(); + Self(Arc::new(Mutex::new(HmrRunnerInner { emitter, watcher_communicator, script_ids: HashMap::new(), + messages: HashMap::new(), + exception_tx, + exception_rx: Some(exception_rx), }))) } pub fn callback(&self, msg: deno_core::InspectorMsg) { - let deno_core::InspectorMsgKind::Message(_msg_id) = msg.kind else { - let message: serde_json::Value = - serde_json::from_str(&msg.content).unwrap(); - log::error!("CoverageCollector received a notification {:?}", message); - return; - }; - - // Check if we know this `msg_id` - let _message: cdp::TakePreciseCoverageResponse = - serde_json::from_str(&msg.content).unwrap(); - // self.write_coverages(message.result); + match msg.kind { + deno_core::InspectorMsgKind::Message(msg_id) => { + let message: serde_json::Value = + serde_json::from_str(&msg.content).unwrap(); + let mut state = self.0.lock(); + let Some(message_state) = state.messages.remove(&msg_id) else { + state + .messages + .insert(msg_id, InspectorMessageState::Ready(message)); + return; + }; + let InspectorMessageState::WaitingFor(sender) = message_state else { + // Maybe log? This should be unreachable + return; + }; + let _ = sender.send(message); + } + deno_core::InspectorMsgKind::Notification => { + let notification = serde_json::from_str(&msg.content).unwrap(); + self.handle_notification(notification); + } + } } - fn handle_notification(&self, _notification: cdp::Notification) { - // if notification.method == "Runtime.exceptionThrown" { - // let exception_thrown = - // serde_json::from_value::(notification.params) - // .map_err(JsErrorBox::from_err)?; - // let (message, description) = exception_thrown - // .exception_details - // .get_message_and_description(); - // break Err( - // JsErrorBox::generic(format!("{} {}", message, description)).into(), - // ); - // } - - // if notification.method == "Debugger.scriptParsed" { - // let params = - // serde_json::from_value::(notification.params) - // .map_err(JsErrorBox::from_err)?; - // if params.url.starts_with("file://") { - // let file_url = Url::parse(¶ms.url).unwrap(); - // let file_path = file_url.to_file_path().unwrap(); - // if let Ok(canonicalized_file_path) = file_path.canonicalize() { - // let canonicalized_file_url = - // Url::from_file_path(canonicalized_file_path).unwrap(); - // self - // .0 - // .lock() - // .script_ids - // .insert(canonicalized_file_url.into(), params.script_id); - // } - // } - // } - - todo!() + fn handle_notification(&self, notification: cdp::Notification) { + if notification.method == "Runtime.exceptionThrown" { + let exception_thrown = + serde_json::from_value::(notification.params) + .unwrap(); + // .map_err(JsErrorBox::from_err)?; + let (message, description) = exception_thrown + .exception_details + .get_message_and_description(); + let _ = self + .0 + .lock() + .exception_tx + .send(JsErrorBox::generic(format!("{} {}", message, description))); + } else if notification.method == "Debugger.scriptParsed" { + let params = + serde_json::from_value::(notification.params) + .unwrap(); + // .map_err(JsErrorBox::from_err)?; + if params.url.starts_with("file://") { + let file_url = Url::parse(¶ms.url).unwrap(); + let file_path = file_url.to_file_path().unwrap(); + if let Ok(canonicalized_file_path) = file_path.canonicalize() { + let canonicalized_file_url = + Url::from_file_path(canonicalized_file_path).unwrap(); + self + .0 + .lock() + .script_ids + .insert(canonicalized_file_url.into(), params.script_id); + } + } + } } } @@ -171,6 +199,7 @@ impl HmrRunner { fn watcher(&self) -> Arc { self.state.0.lock().watcher_communicator.clone() } + // TODO(bartlomieju): this code is duplicated in `cli/tools/coverage/mod.rs` pub fn stop(&mut self) { self @@ -180,99 +209,88 @@ impl HmrRunner { } pub async fn run(&mut self) -> Result<(), CoreError> { - // self - // .watcher() - // .change_restart_mode(WatcherRestartMode::Manual); - - // loop { - // select! { - // biased; - // // Some(notification) = session_rx.next() => { - // // let notification = serde_json::from_value::(notification).map_err(JsErrorBox::from_err)?; - // // if notification.method == "Runtime.exceptionThrown" { - // // let exception_thrown = serde_json::from_value::(notification.params).map_err(JsErrorBox::from_err)?; - // // let (message, description) = exception_thrown.exception_details.get_message_and_description(); - // // break Err(JsErrorBox::generic(format!("{} {}", message, description)).into()); - // // } else if notification.method == "Debugger.scriptParsed" { - // // let params = serde_json::from_value::(notification.params).map_err(JsErrorBox::from_err)?; - // // if params.url.starts_with("file://") { - // // let file_url = Url::parse(¶ms.url).unwrap(); - // // let file_path = file_url.to_file_path().unwrap(); - // // if let Ok(canonicalized_file_path) = file_path.canonicalize() { - // // let canonicalized_file_url = Url::from_file_path(canonicalized_file_path).unwrap(); - // // self.state.0.lock().script_ids.insert(canonicalized_file_url.into(), params.script_id); - // // } - // // } - // // } - // // } - // changed_paths = self.watcher().watch_for_changed_paths() => { - // let changed_paths = changed_paths.map_err(JsErrorBox::from_err)?; - - // let Some(changed_paths) = changed_paths else { - // let _ = self.watcher().force_restart(); - // continue; - // }; - - // let filtered_paths: Vec = changed_paths.into_iter().filter(|p| p.extension().is_some_and(|ext| { - // let ext_str = ext.to_str().unwrap(); - // matches!(ext_str, "js" | "ts" | "jsx" | "tsx") - // })).collect(); - - // // If after filtering there are no paths it means it's either a file - // // we can't HMR or an external file that was passed explicitly to - // // `--watch-hmr=` path. - // if filtered_paths.is_empty() { - // let _ = self.watcher().force_restart(); - // continue; - // } - - // for path in filtered_paths { - // let Some(path_str) = path.to_str() else { - // let _ = self.watcher().force_restart(); - // continue; - // }; - // let Ok(module_url) = Url::from_file_path(path_str) else { - // let _ = self.watcher().force_restart(); - // continue; - // }; - - // let Some(id) = self.state.0.lock().script_ids.get(module_url.as_str()).cloned() else { - // let _ = self.watcher().force_restart(); - // continue; - // }; - - // let source_code = tokio::fs::read_to_string(deno_path_util::url_to_file_path(&module_url).unwrap()).await?; - // let source_code = self.state.0.lock().emitter.emit_for_hmr( - // &module_url, - // source_code, - // )?; - - // let mut tries = 1; - // loop { - // let result = self.set_script_source(&id, source_code.as_str()).await?; - - // if matches!(result.status, cdp::Status::Ok) { - // self.dispatch_hmr_event(module_url.as_str()); - // self.watcher().print(format!("Replaced changed module {}", module_url.as_str())); - // break; - // } - - // self.watcher().print(format!("Failed to reload module {}: {}.", module_url, colors::gray(&explain(&result)))); - // if should_retry(&result.status) && tries <= 2 { - // tries += 1; - // tokio::time::sleep(std::time::Duration::from_millis(100)).await; - // continue; - // } - - // let _ = self.watcher().force_restart(); - // break; - // } - // } - // } - // } - // } - - todo!() + self + .watcher() + .change_restart_mode(WatcherRestartMode::Manual); + let watcher = self.watcher(); + let mut exception_rx = self.state.0.lock().exception_rx.take().unwrap(); + loop { + select! { + biased; + + maybe_error = exception_rx.recv() => { + if let Some(err) = maybe_error { + break Err(err.into()); + } + }, + + changed_paths = watcher.watch_for_changed_paths() => { + let changed_paths = changed_paths.map_err(JsErrorBox::from_err)?; + + let Some(changed_paths) = changed_paths else { + let _ = self.watcher().force_restart(); + continue; + }; + + let filtered_paths: Vec = changed_paths.into_iter().filter(|p| p.extension().is_some_and(|ext| { + let ext_str = ext.to_str().unwrap(); + matches!(ext_str, "js" | "ts" | "jsx" | "tsx") + })).collect(); + + // If after filtering there are no paths it means it's either a file + // we can't HMR or an external file that was passed explicitly to + // `--watch-hmr=` path. + if filtered_paths.is_empty() { + let _ = self.watcher().force_restart(); + continue; + } + + for path in filtered_paths { + let Some(path_str) = path.to_str() else { + let _ = self.watcher().force_restart(); + continue; + }; + let Ok(module_url) = Url::from_file_path(path_str) else { + let _ = self.watcher().force_restart(); + continue; + }; + + let Some(id) = self.state.0.lock().script_ids.get(module_url.as_str()).cloned() else { + let _ = self.watcher().force_restart(); + continue; + }; + + let source_code = tokio::fs::read_to_string(deno_path_util::url_to_file_path(&module_url).unwrap()).await?; + let source_code = self.state.0.lock().emitter.emit_for_hmr( + &module_url, + source_code, + )?; + + let mut tries = 1; + loop { + let msg_id = self.set_script_source(&id, source_code.as_str()); + let result = self.wait_for_response::(msg_id).await?; + + if matches!(result.status, cdp::Status::Ok) { + self.dispatch_hmr_event(module_url.as_str()); + self.watcher().print(format!("Replaced changed module {}", module_url.as_str())); + break; + } + + self.watcher().print(format!("Failed to reload module {}: {}.", module_url, colors::gray(&explain(&result)))); + if should_retry(&result.status) && tries <= 2 { + tries += 1; + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + continue; + } + + let _ = self.watcher().force_restart(); + break; + } + } + } + } + } } // TODO(bartlomieju): this code is duplicated in `cli/tools/coverage/mod.rs` @@ -291,30 +309,41 @@ impl HmrRunner { self.session.post_message::<()>("Runtime.disable", None); } - #[allow(unused)] - fn set_script_source( - &mut self, - script_id: &str, - source: &str, - ) -> Result { - todo!() - // let result = self - // .session - // .post_message( - // "Debugger.setScriptSource", - // Some(json!({ - // "scriptId": script_id, - // "scriptSource": source, - // "allowTopFrameEditing": true, - // })), - // ) - // .await?; - - // serde_json::from_value::(result).map_err( - // |e| { - // InspectorPostMessageErrorKind::JsBox(JsErrorBox::from_err(e)).into_box() - // }, - // ) + async fn wait_for_response( + &self, + msg_id: i32, + ) -> Result { + let value = if let Some(message_state) = + self.state.0.lock().messages.remove(&msg_id) + { + let InspectorMessageState::Ready(value) = message_state else { + unreachable!(); + }; + value + } else { + let (tx, rx) = oneshot::channel(); + self + .state + .0 + .lock() + .messages + .insert(msg_id, InspectorMessageState::WaitingFor(tx)); + rx.await.unwrap() + }; + serde_json::from_value::(value).map_err(|e| { + InspectorPostMessageErrorKind::JsBox(JsErrorBox::from_err(e)).into_box() + }) + } + + fn set_script_source(&mut self, script_id: &str, source: &str) -> i32 { + self.session.post_message( + "Debugger.setScriptSource", + Some(json!({ + "scriptId": script_id, + "scriptSource": source, + "allowTopFrameEditing": true, + })), + ) } fn dispatch_hmr_event(&mut self, script_id: &str) { diff --git a/cli/tools/test/mod.rs b/cli/tools/test/mod.rs index bd4e1a99c72d44..b3e9c2691861aa 100644 --- a/cli/tools/test/mod.rs +++ b/cli/tools/test/mod.rs @@ -805,6 +805,7 @@ async fn test_specifier_inner( if let Some(coverage_collector) = &mut coverage_collector { coverage_collector.stop_collecting()?; } + worker.run_up_to_duration(Duration::from_millis(0)).await?; Ok(()) } diff --git a/cli/worker.rs b/cli/worker.rs index 763ec1bc5fc857..d508742c2533f8 100644 --- a/cli/worker.rs +++ b/cli/worker.rs @@ -7,7 +7,6 @@ use std::sync::Arc; use deno_ast::ModuleSpecifier; use deno_core::Extension; use deno_core::OpState; -use deno_core::PollEventLoopOptions; use deno_core::error::CoreError; use deno_core::error::JsError; use deno_core::futures::FutureExt; diff --git a/tests/integration/coverage_tests.rs b/tests/integration/coverage_tests.rs index e0eaede7107cf0..a9975bd5457f56 100644 --- a/tests/integration/coverage_tests.rs +++ b/tests/integration/coverage_tests.rs @@ -184,6 +184,7 @@ fn multifile_coverage() { .run(); output.assert_exit_code(0); + eprintln!("output {:#?}", output.print_output()); output.skip_output_check(); let output = context From 52e246e91574f927f3725da3ed80359dce2f8c88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Fri, 12 Sep 2025 19:48:42 +0200 Subject: [PATCH 04/20] coverage tests working --- cli/tools/coverage/mod.rs | 69 +++++++++++++++++++++-------- cli/tools/run/hmr.rs | 29 ++++++++++-- cli/tools/test/mod.rs | 2 +- ext/node/ops/inspector.rs | 7 ++- runtime/inspector_server.rs | 6 +-- runtime/worker.rs | 3 ++ tests/integration/coverage_tests.rs | 4 +- 7 files changed, 88 insertions(+), 32 deletions(-) diff --git a/cli/tools/coverage/mod.rs b/cli/tools/coverage/mod.rs index 0e7dc408e3c2bd..c0080e12819067 100644 --- a/cli/tools/coverage/mod.rs +++ b/cli/tools/coverage/mod.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; @@ -54,6 +55,8 @@ pub mod reporter; mod util; use merge::ProcessCoverage; +static NEXT_MSG_ID: AtomicI32 = AtomicI32::new(0); + #[derive(Debug)] pub struct CoverageCollectorInner { dir: PathBuf, @@ -79,27 +82,33 @@ impl CoverageCollectorState { return; }; - eprintln!( - "callback {} {:#?} {}", - msg_id, - self.0.lock().coverage_msg_id.as_ref(), - msg.content - ); - if let Some(coverage_msg_id) = self.0.lock().coverage_msg_id.as_ref() - && *coverage_msg_id == msg_id + // eprintln!( + // "callback {} {:#?}", + // // "callback {} {:#?} {}", + // msg_id, + // self.0.lock().coverage_msg_id.as_ref(), + // // msg.content + // ); + 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); + // eprintln!("written coverages"); } } fn write_coverages(&self, script_coverages: Vec) { + // eprintln!("got coverages {}", script_coverages.len()); 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 + // eprintln!("script coverage url {}", script_coverage.url); if script_coverage.url.is_empty() || script_coverage.url.starts_with("ext:") || script_coverage.url.starts_with("[ext:") @@ -171,38 +180,60 @@ impl CoverageCollector { // TODO(barltomieju): is it actually necessary to call this? fn enable_debugger(&mut self) { - self.session.post_message::<()>("Debugger.enable", None); + self.session.post_message::<()>( + NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed), + "Debugger.enable", + None, + ); } fn enable_profiler(&mut self) { - self.session.post_message::<()>("Profiler.enable", None); + self.session.post_message::<()>( + NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed), + "Profiler.enable", + None, + ); } // TODO(barltomieju): is it actually necessary to call this? fn disable_debugger(&mut self) { - self.session.post_message::<()>("Debugger.disable", None); + self.session.post_message::<()>( + NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed), + "Debugger.disable", + None, + ); } // TODO(barltomieju): is it actually necessary to call this? fn disable_profiler(&mut self) { - self.session.post_message::<()>("Profiler.disable", None); + self.session.post_message::<()>( + NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed), + "Profiler.disable", + None, + ); } fn start_precise_coverage( &mut self, parameters: cdp::StartPreciseCoverageArgs, ) { - self - .session - .post_message("Profiler.startPreciseCoverage", Some(parameters)); + self.session.post_message( + NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed), + "Profiler.startPreciseCoverage", + Some(parameters), + ); } fn take_precise_coverage(&mut self) { - let msg_id = self - .session - .post_message::<()>("Profiler.takePreciseCoverage", None); - eprintln!("take precise coverage {}", msg_id); + let msg_id = NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed); self.state.0.lock().coverage_msg_id.replace(msg_id); + + self.session.post_message::<()>( + msg_id, + "Profiler.takePreciseCoverage", + None, + ); + // eprintln!("take precise coverage {}", msg_id); // let return_object = serde_json::from_value(return_value).map_err(|e| { // InspectorPostMessageErrorKind::JsBox(JsErrorBox::from_err(e)).into_box() // })?; diff --git a/cli/tools/run/hmr.rs b/cli/tools/run/hmr.rs index b223c660f19677..41c6a26333d541 100644 --- a/cli/tools/run/hmr.rs +++ b/cli/tools/run/hmr.rs @@ -3,6 +3,7 @@ use std::collections::HashMap; use std::path::PathBuf; use std::sync::Arc; +use std::sync::atomic::AtomicI32; use deno_core::InspectorPostMessageError; use deno_core::InspectorPostMessageErrorKind; @@ -25,6 +26,8 @@ use crate::module_loader::CliEmitter; use crate::util::file_watcher::WatcherCommunicator; use crate::util::file_watcher::WatcherRestartMode; +static NEXT_MSG_ID: AtomicI32 = AtomicI32::new(0); + fn explain(response: &cdp::SetScriptSourceResponse) -> String { match response.status { cdp::Status::Ok => "OK".to_string(), @@ -296,17 +299,33 @@ impl HmrRunner { // TODO(bartlomieju): this code is duplicated in `cli/tools/coverage/mod.rs` fn enable_debugger(&mut self) { // TODO(barltomieju): is it actually necessary to call this? - self.session.post_message::<()>("Debugger.enable", None); + self.session.post_message::<()>( + NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed), + "Debugger.enable", + None, + ); - self.session.post_message::<()>("Runtime.enable", None); + self.session.post_message::<()>( + NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed), + "Runtime.enable", + None, + ); } // TODO(bartlomieju): this code is duplicated in `cli/tools/coverage/mod.rs` fn disable_debugger(&mut self) { // TODO(barltomieju): is it actually necessary to call this? - self.session.post_message::<()>("Debugger.disable", None); + self.session.post_message::<()>( + NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed), + "Debugger.disable", + None, + ); // TODO(barltomieju): is it actually necessary to call this? - self.session.post_message::<()>("Runtime.disable", None); + self.session.post_message::<()>( + NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed), + "Runtime.disable", + None, + ); } async fn wait_for_response( @@ -337,6 +356,7 @@ impl HmrRunner { fn set_script_source(&mut self, script_id: &str, source: &str) -> i32 { self.session.post_message( + NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed), "Debugger.setScriptSource", Some(json!({ "scriptId": script_id, @@ -353,6 +373,7 @@ impl HmrRunner { ); self.session.post_message( + NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed), "Runtime.evaluate", Some(json!({ "expression": expr, diff --git a/cli/tools/test/mod.rs b/cli/tools/test/mod.rs index b3e9c2691861aa..984e2e4d86c1b6 100644 --- a/cli/tools/test/mod.rs +++ b/cli/tools/test/mod.rs @@ -805,7 +805,7 @@ async fn test_specifier_inner( if let Some(coverage_collector) = &mut coverage_collector { coverage_collector.stop_collecting()?; } - worker.run_up_to_duration(Duration::from_millis(0)).await?; + // worker.run_up_to_duration(Duration::from_millis(0)).await?; Ok(()) } diff --git a/ext/node/ops/inspector.rs b/ext/node/ops/inspector.rs index 180ae1e36ca6ac..5f41d1b2ecc7db 100644 --- a/ext/node/ops/inspector.rs +++ b/ext/node/ops/inspector.rs @@ -8,7 +8,6 @@ use deno_core::InspectorSessionKind; use deno_core::InspectorSessionOptions; use deno_core::JsRuntimeInspector; use deno_core::OpState; -use deno_core::futures::channel::mpsc; use deno_core::op2; use deno_core::v8; use deno_error::JsErrorBox; @@ -81,7 +80,7 @@ pub fn op_inspector_emit_protocol_event( } struct JSInspectorSession { - tx: RefCell>>, + tx: RefCell>>, } // SAFETY: we're sure this can be GCed @@ -135,7 +134,7 @@ where .borrow::>>() .borrow_mut(); - let tx = inspector.create_raw_session( + let (_, tx) = inspector.create_raw_session( InspectorSessionOptions { kind: InspectorSessionKind::NonBlocking { wait_for_disconnect: false, @@ -173,7 +172,7 @@ pub fn op_inspector_dispatch( #[string] message: String, ) { if let Some(tx) = &*session.tx.borrow() { - let _ = tx.unbounded_send(message); + let _ = tx.send(message); } } diff --git a/runtime/inspector_server.rs b/runtime/inspector_server.rs index 7e9ae65d055850..9792b8909ed467 100644 --- a/runtime/inspector_server.rs +++ b/runtime/inspector_server.rs @@ -204,7 +204,7 @@ fn handle_ws_request( // The 'outbound' channel carries messages sent to the websocket. let (outbound_tx, outbound_rx) = mpsc::unbounded(); // The 'inbound' channel carries messages received from the websocket. - let (inbound_tx, inbound_rx) = mpsc::unbounded(); + let (inbound_tx, inbound_rx) = std::sync::mpsc::channel(); let inspector_session_proxy = InspectorSessionProxy { tx: outbound_tx, @@ -417,7 +417,7 @@ async fn server( /// task yielding. async fn pump_websocket_messages( mut websocket: WebSocket>, - inbound_tx: UnboundedSender, + inbound_tx: std::sync::mpsc::Sender, mut outbound_rx: UnboundedReceiver, ) { 'pump: loop { @@ -430,7 +430,7 @@ async fn pump_websocket_messages( match msg.opcode { OpCode::Text => { if let Ok(s) = String::from_utf8(msg.payload.to_vec()) { - let _ = inbound_tx.unbounded_send(s); + let _ = inbound_tx.send(s); } } OpCode::Close => { diff --git a/runtime/worker.rs b/runtime/worker.rs index 0ec7f18ea872f8..a60b0d1239499b 100644 --- a/runtime/worker.rs +++ b/runtime/worker.rs @@ -929,11 +929,14 @@ impl MainWorker { cb: deno_core::InspectorSessionSend, ) -> LocalSyncInspectorSession { self.js_runtime.maybe_init_inspector(); + let insp = self.js_runtime.inspector(); + self .js_runtime .inspector() .borrow() .create_local_sync_session( + insp, cb, InspectorSessionOptions { kind: InspectorSessionKind::Blocking, diff --git a/tests/integration/coverage_tests.rs b/tests/integration/coverage_tests.rs index a9975bd5457f56..c4ae75eb225a44 100644 --- a/tests/integration/coverage_tests.rs +++ b/tests/integration/coverage_tests.rs @@ -173,6 +173,7 @@ fn multifile_coverage() { let tempdir = context.temp_dir(); let tempdir = tempdir.path().join("cov"); + eprintln!("before test"); let output = context .new_command() .args_vec(vec![ @@ -182,7 +183,7 @@ fn multifile_coverage() { format!("coverage/multifile/"), ]) .run(); - + eprintln!("after test"); output.assert_exit_code(0); eprintln!("output {:#?}", output.print_output()); output.skip_output_check(); @@ -198,6 +199,7 @@ fn multifile_coverage() { .run(); // Verify there's no "Check" being printed + eprintln!("output2 {:#?}", output.print_output()); assert!(output.stderr().is_empty()); output.assert_stdout_matches_file( From 745e3806679f79a85f3afe4cf6b8c3c73968fc6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Fri, 12 Sep 2025 20:46:10 +0200 Subject: [PATCH 05/20] REPL working --- cli/lib/worker.rs | 6 -- cli/tools/repl/session.rs | 121 +++++++++++++++++++++++++++++++++----- ext/node/ops/inspector.rs | 3 +- runtime/worker.rs | 12 ---- 4 files changed, 109 insertions(+), 33 deletions(-) diff --git a/cli/lib/worker.rs b/cli/lib/worker.rs index 822b3117f6f521..f2255724224d1d 100644 --- a/cli/lib/worker.rs +++ b/cli/lib/worker.rs @@ -23,7 +23,6 @@ use deno_runtime::deno_core; use deno_runtime::deno_core::CompiledWasmModuleStore; use deno_runtime::deno_core::Extension; use deno_runtime::deno_core::JsRuntime; -use deno_runtime::deno_core::LocalInspectorSession; use deno_runtime::deno_core::LocalSyncInspectorSession; use deno_runtime::deno_core::ModuleLoader; use deno_runtime::deno_core::SharedArrayBufferStore; @@ -816,11 +815,6 @@ impl LibMainWorker { &mut self.worker.js_runtime } - #[inline] - pub fn create_inspector_session(&mut self) -> LocalInspectorSession { - self.worker.create_inspector_session() - } - #[inline] pub fn create_sync_inspector_session( &mut self, diff --git a/cli/tools/repl/session.rs b/cli/tools/repl/session.rs index 14a8c7decdb2d2..ab3dd72ad058f9 100644 --- a/cli/tools/repl/session.rs +++ b/cli/tools/repl/session.rs @@ -1,6 +1,8 @@ // Copyright 2018-2025 the Deno authors. MIT license. +use std::collections::HashMap; use std::sync::Arc; +use std::sync::atomic::AtomicI32; use deno_ast::ImportsNotUsedAsValues; use deno_ast::JsxAutomaticOptions; @@ -18,8 +20,9 @@ use deno_ast::swc::common::comments::CommentKind; use deno_ast::swc::ecma_visit::Visit; use deno_ast::swc::ecma_visit::VisitWith; use deno_ast::swc::ecma_visit::noop_visit_type; +use deno_core::InspectorMsgKind; use deno_core::InspectorPostMessageError; -use deno_core::LocalInspectorSession; +use deno_core::LocalSyncInspectorSession; use deno_core::PollEventLoopOptions; use deno_core::anyhow::anyhow; use deno_core::error::AnyError; @@ -27,6 +30,9 @@ use deno_core::error::CoreError; use deno_core::futures::FutureExt; use deno_core::futures::StreamExt; use deno_core::futures::channel::mpsc::UnboundedReceiver; +use deno_core::futures::channel::mpsc::UnboundedSender; +use deno_core::futures::channel::mpsc::unbounded; +use deno_core::parking_lot::Mutex as SyncMutex; use deno_core::serde_json; use deno_core::serde_json::Value; use deno_core::unsync::spawn; @@ -45,6 +51,7 @@ use once_cell::sync::Lazy; use regex::Match; use regex::Regex; use tokio::sync::Mutex; +use tokio::sync::oneshot; use crate::args::CliOptions; use crate::cdp; @@ -172,8 +179,11 @@ pub struct ReplSession { internal_object_id: Option, npm_installer: Option>, resolver: Arc, + // NB: `session` and `state` must come before Worker, so that relevant V8 objects + // are dropped before the isolate is dropped with `worker`. + session: LocalSyncInspectorSession, + state: ReplSessionState, pub worker: MainWorker, - session: LocalInspectorSession, pub context_id: u64, pub language_server: ReplLanguageServer, pub notifications: Arc>>, @@ -186,6 +196,87 @@ pub struct ReplSession { decorators: deno_ast::DecoratorsTranspileOption, } +// TODO: duplicated in `cli/tools/run/hmr.rs` +#[derive(Debug)] +enum InspectorMessageState { + Ready(serde_json::Value), + WaitingFor(oneshot::Sender), +} + +#[derive(Debug)] +pub struct ReplSessionInner { + messages: HashMap, + notification_tx: UnboundedSender, +} + +#[derive(Clone, Debug)] +pub struct ReplSessionState(Arc>); + +impl ReplSessionState { + pub fn new(notification_tx: UnboundedSender) -> Self { + Self(Arc::new(SyncMutex::new(ReplSessionInner { + messages: HashMap::new(), + notification_tx, + }))) + } + + fn callback(&self, msg: deno_core::InspectorMsg) { + match msg.kind { + // TODO: duplicated in `cli/tools/run/hmr.rs` + deno_core::InspectorMsgKind::Message(msg_id) => { + let message: serde_json::Value = + serde_json::from_str(&msg.content).unwrap(); + let mut state = self.0.lock(); + let Some(message_state) = state.messages.remove(&msg_id) else { + state + .messages + .insert(msg_id, InspectorMessageState::Ready(message)); + return; + }; + let InspectorMessageState::WaitingFor(sender) = message_state else { + // Maybe log? This should be unreachable + return; + }; + let _ = sender.send(message); + } + InspectorMsgKind::Notification => { + if let Ok(value) = serde_json::from_str(&msg.content) { + let _ = self.0.lock().notification_tx.unbounded_send(value); + } + } + } + } + + async fn wait_for_response( + &self, + msg_id: i32, + ) -> Result { + let value = + if let Some(message_state) = self.0.lock().messages.remove(&msg_id) { + let InspectorMessageState::Ready(value) = message_state else { + unreachable!(); + }; + value + } else { + let (tx, rx) = oneshot::channel(); + self + .0 + .lock() + .messages + .insert(msg_id, InspectorMessageState::WaitingFor(tx)); + rx.await.unwrap() + }; + // TODO(bartlomieju): very specific, but that's probably okay for now, that + // each caller casts to wanted type? + Ok(value["result"].clone()) + } +} + +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 ReplSession { #[allow(clippy::too_many_arguments)] pub async fn initialize( @@ -198,24 +289,22 @@ impl ReplSession { test_event_receiver: TestEventReceiver, ) -> Result { let language_server = ReplLanguageServer::new_initialized().await?; - let mut session = worker.create_inspector_session(); - worker - .js_runtime - .with_event_loop_future( - session - .post_message::<()>("Runtime.enable", None) - .boxed_local(), - PollEventLoopOptions::default(), - ) - .await?; + let (notification_tx, mut notification_rx) = unbounded(); + let repl_session_state = ReplSessionState::new(notification_tx); + let state = repl_session_state.clone(); + let callback = + Box::new(move |message| repl_session_state.callback(message)); + let mut session = worker.create_sync_inspector_session(callback); + + session.post_message::<()>(next_msg_id(), "Runtime.enable", None); // Enabling the runtime domain will always send trigger one executionContextCreated for each // context the inspector knows about so we grab the execution context from that since // our inspector does not support a default context (0 is an invalid context id). let context_id: u64; - let mut notification_rx = session.take_notification_rx(); + // TODO(bartlomieju): this will probably hang, might need to actually spin the event loop a bit here loop { let notification = notification_rx.next().await.unwrap(); let notification = @@ -260,6 +349,7 @@ impl ReplSession { resolver, worker, session, + state, context_id, language_server, referrer, @@ -316,11 +406,14 @@ impl ReplSession { method: &str, params: Option, ) -> Result { + let msg_id = self.session.post_message(next_msg_id(), method, params); + let fut = self.state.wait_for_response(msg_id).boxed_local(); + self .worker .js_runtime .with_event_loop_future( - self.session.post_message(method, params).boxed_local(), + fut, PollEventLoopOptions { // NOTE(bartlomieju): this is an important bit; we don't want to pump V8 // message loop here, so that GC won't run. Otherwise, the resulting diff --git a/ext/node/ops/inspector.rs b/ext/node/ops/inspector.rs index 5f41d1b2ecc7db..d34a66ed67cf5b 100644 --- a/ext/node/ops/inspector.rs +++ b/ext/node/ops/inspector.rs @@ -134,7 +134,8 @@ where .borrow::>>() .borrow_mut(); - let (_, tx) = inspector.create_raw_session( + // TODO(bartlomieju): rewrite to use a different API + let tx = inspector.create_raw_session( InspectorSessionOptions { kind: InspectorSessionKind::NonBlocking { wait_for_disconnect: false, diff --git a/runtime/worker.rs b/runtime/worker.rs index a60b0d1239499b..9ee9a4ccb332f7 100644 --- a/runtime/worker.rs +++ b/runtime/worker.rs @@ -19,7 +19,6 @@ use deno_core::Extension; use deno_core::InspectorSessionKind; use deno_core::InspectorSessionOptions; use deno_core::JsRuntime; -use deno_core::LocalInspectorSession; use deno_core::LocalSyncInspectorSession; use deno_core::ModuleCodeString; use deno_core::ModuleId; @@ -911,17 +910,6 @@ impl MainWorker { } } - /// Create new inspector session. This function panics if Worker - /// was not configured to create inspector. - pub fn create_inspector_session(&mut self) -> LocalInspectorSession { - self.js_runtime.maybe_init_inspector(); - self.js_runtime.inspector().borrow().create_local_session( - InspectorSessionOptions { - kind: InspectorSessionKind::Blocking, - }, - ) - } - /// Create new inspector session. This function panics if Worker /// was not configured to create inspector. pub fn create_sync_inspector_session( From 0410eabe283eb56e4053a1a5bffe475540016ad8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Fri, 12 Sep 2025 21:44:30 +0200 Subject: [PATCH 06/20] compiles again --- cli/tools/repl/session.rs | 29 +++++++++++++++++-- cli/tools/run/hmr.rs | 6 ++-- ext/node/ops/inspector.rs | 59 ++++++++++++++++++++------------------- runtime/worker.rs | 19 ++++++------- 4 files changed, 68 insertions(+), 45 deletions(-) diff --git a/cli/tools/repl/session.rs b/cli/tools/repl/session.rs index ab3dd72ad058f9..c40e4e4e85875a 100644 --- a/cli/tools/repl/session.rs +++ b/cli/tools/repl/session.rs @@ -222,10 +222,32 @@ impl ReplSessionState { fn callback(&self, msg: deno_core::InspectorMsg) { match msg.kind { - // TODO: duplicated in `cli/tools/run/hmr.rs` + // TODO: duplicated in `cli/tools/run/hmr.rs` for the most part deno_core::InspectorMsgKind::Message(msg_id) => { let message: serde_json::Value = - serde_json::from_str(&msg.content).unwrap(); + match serde_json::from_str(&msg.content) { + Ok(v) => v, + Err(error) => match error.classify() { + serde_json::error::Category::Syntax => serde_json::json!({ + "id": msg_id, + "result": { + "result": { + "type": "error", + "description": "Unterminated string literal", + "value": "Unterminated string literal", + }, + "exceptionDetails": { + "exceptionId": 0, + "text": "Unterminated string literal", + "lineNumber": 0, + "columnNumber": 0 + }, + }, + }), + _ => panic!("Could not parse inspector message"), + }, + }; + let mut state = self.0.lock(); let Some(message_state) = state.messages.remove(&msg_id) else { state @@ -406,7 +428,8 @@ impl ReplSession { method: &str, params: Option, ) -> Result { - let msg_id = self.session.post_message(next_msg_id(), method, params); + let msg_id = next_msg_id(); + self.session.post_message(msg_id, method, params); let fut = self.state.wait_for_response(msg_id).boxed_local(); self diff --git a/cli/tools/run/hmr.rs b/cli/tools/run/hmr.rs index 41c6a26333d541..7d94c9a7ca72fd 100644 --- a/cli/tools/run/hmr.rs +++ b/cli/tools/run/hmr.rs @@ -355,15 +355,17 @@ impl HmrRunner { } fn set_script_source(&mut self, script_id: &str, source: &str) -> i32 { + let msg_id = NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed); self.session.post_message( - NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed), + msg_id, "Debugger.setScriptSource", Some(json!({ "scriptId": script_id, "scriptSource": source, "allowTopFrameEditing": true, })), - ) + ); + msg_id } fn dispatch_hmr_event(&mut self, script_id: &str) { diff --git a/ext/node/ops/inspector.rs b/ext/node/ops/inspector.rs index d34a66ed67cf5b..4a69b9cf44e8a6 100644 --- a/ext/node/ops/inspector.rs +++ b/ext/node/ops/inspector.rs @@ -4,6 +4,7 @@ use std::cell::RefCell; use std::rc::Rc; use deno_core::GarbageCollected; +use deno_core::InspectorMsg; use deno_core::InspectorSessionKind; use deno_core::InspectorSessionOptions; use deno_core::JsRuntimeInspector; @@ -80,7 +81,7 @@ pub fn op_inspector_emit_protocol_event( } struct JSInspectorSession { - tx: RefCell>>, + session: RefCell>, } // SAFETY: we're sure this can be GCed @@ -130,40 +131,40 @@ where let context = v8::Global::new(scope, context); let callback = v8::Global::new(scope, callback); - let inspector = state - .borrow::>>() - .borrow_mut(); + let inspector = state.borrow::>>().clone(); + + // The inspector connection does not keep the event loop alive but + // when the inspector sends a message to the frontend, the JS that + // that runs may keep the event loop alive so we have to call back + // synchronously, instead of using the usual LocalInspectorSession + // UnboundedReceiver API. + let callback = Box::new(move |message: InspectorMsg| { + // SAFETY: This function is called directly by the inspector, so + // 1) The isolate is still valid + // 2) We are on the same thread as the Isolate + let scope = unsafe { &mut v8::CallbackScope::new(&mut *isolate) }; + let context = v8::Local::new(scope, context.clone()); + let scope = &mut v8::ContextScope::new(scope, context); + let scope = &mut v8::TryCatch::new(scope); + let recv = v8::undefined(scope); + if let Some(message) = v8::String::new(scope, &message.content) { + let callback = v8::Local::new(scope, callback.clone()); + callback.call(scope, recv.into(), &[message.into()]); + } + }); - // TODO(bartlomieju): rewrite to use a different API - let tx = inspector.create_raw_session( + let session = JsRuntimeInspector::create_local_sync_session( + inspector, + callback, InspectorSessionOptions { kind: InspectorSessionKind::NonBlocking { wait_for_disconnect: false, }, }, - // The inspector connection does not keep the event loop alive but - // when the inspector sends a message to the frontend, the JS that - // that runs may keep the event loop alive so we have to call back - // synchronously, instead of using the usual LocalInspectorSession - // UnboundedReceiver API. - Box::new(move |message| { - // SAFETY: This function is called directly by the inspector, so - // 1) The isolate is still valid - // 2) We are on the same thread as the Isolate - let scope = unsafe { &mut v8::CallbackScope::new(&mut *isolate) }; - let context = v8::Local::new(scope, context.clone()); - let scope = &mut v8::ContextScope::new(scope, context); - let scope = &mut v8::TryCatch::new(scope); - let recv = v8::undefined(scope); - if let Some(message) = v8::String::new(scope, &message.content) { - let callback = v8::Local::new(scope, callback.clone()); - callback.call(scope, recv.into(), &[message.into()]); - } - }), ); Ok(JSInspectorSession { - tx: RefCell::new(Some(tx)), + session: RefCell::new(Some(session)), }) } @@ -172,12 +173,12 @@ pub fn op_inspector_dispatch( #[cppgc] session: &JSInspectorSession, #[string] message: String, ) { - if let Some(tx) = &*session.tx.borrow() { - let _ = tx.send(message); + if let Some(session) = &mut *session.session.borrow_mut() { + session.dispatch(message); } } #[op2(fast)] pub fn op_inspector_disconnect(#[cppgc] session: &JSInspectorSession) { - drop(session.tx.borrow_mut().take()); + drop(session.session.borrow_mut().take()); } diff --git a/runtime/worker.rs b/runtime/worker.rs index 9ee9a4ccb332f7..a71420140d6bce 100644 --- a/runtime/worker.rs +++ b/runtime/worker.rs @@ -19,6 +19,7 @@ use deno_core::Extension; use deno_core::InspectorSessionKind; use deno_core::InspectorSessionOptions; use deno_core::JsRuntime; +use deno_core::JsRuntimeInspector; use deno_core::LocalSyncInspectorSession; use deno_core::ModuleCodeString; use deno_core::ModuleId; @@ -919,17 +920,13 @@ impl MainWorker { self.js_runtime.maybe_init_inspector(); let insp = self.js_runtime.inspector(); - self - .js_runtime - .inspector() - .borrow() - .create_local_sync_session( - insp, - cb, - InspectorSessionOptions { - kind: InspectorSessionKind::Blocking, - }, - ) + JsRuntimeInspector::create_local_sync_session( + insp, + cb, + InspectorSessionOptions { + kind: InspectorSessionKind::Blocking, + }, + ) } pub async fn run_event_loop( From a96b3a2418dc35a317c806cb9ad7b5aa449b68ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Mon, 15 Sep 2025 12:36:12 +0200 Subject: [PATCH 07/20] wip --- a.js | 1 + cli/lib/worker.rs | 2 +- runtime/inspector_server.rs | 5 +++-- runtime/worker.rs | 2 +- 4 files changed, 6 insertions(+), 4 deletions(-) create mode 100644 a.js diff --git a/a.js b/a.js new file mode 100644 index 00000000000000..7d37e87e63e2af --- /dev/null +++ b/a.js @@ -0,0 +1 @@ +setInterval(() => console.log("hello there"), 5_000); diff --git a/cli/lib/worker.rs b/cli/lib/worker.rs index f2255724224d1d..31f00a2acd4d3f 100644 --- a/cli/lib/worker.rs +++ b/cli/lib/worker.rs @@ -818,7 +818,7 @@ impl LibMainWorker { #[inline] pub fn create_sync_inspector_session( &mut self, - cb: deno_core::InspectorSessionSend, + cb: deno_core::InspectorSessionCallback, ) -> LocalSyncInspectorSession { self.worker.create_sync_inspector_session(cb) } diff --git a/runtime/inspector_server.rs b/runtime/inspector_server.rs index 9792b8909ed467..1c47422d0a3502 100644 --- a/runtime/inspector_server.rs +++ b/runtime/inspector_server.rs @@ -204,7 +204,7 @@ fn handle_ws_request( // The 'outbound' channel carries messages sent to the websocket. let (outbound_tx, outbound_rx) = mpsc::unbounded(); // The 'inbound' channel carries messages received from the websocket. - let (inbound_tx, inbound_rx) = std::sync::mpsc::channel(); + let (inbound_tx, inbound_rx) = mpsc::unbounded(); let inspector_session_proxy = InspectorSessionProxy { tx: outbound_tx, @@ -417,7 +417,7 @@ async fn server( /// task yielding. async fn pump_websocket_messages( mut websocket: WebSocket>, - inbound_tx: std::sync::mpsc::Sender, + mut inbound_tx: UnboundedSender, mut outbound_rx: UnboundedReceiver, ) { 'pump: loop { @@ -431,6 +431,7 @@ async fn pump_websocket_messages( OpCode::Text => { if let Ok(s) = String::from_utf8(msg.payload.to_vec()) { let _ = inbound_tx.send(s); + eprintln!("sent a message from WS to inspector"); } } OpCode::Close => { diff --git a/runtime/worker.rs b/runtime/worker.rs index a71420140d6bce..16de1e720ff1cc 100644 --- a/runtime/worker.rs +++ b/runtime/worker.rs @@ -915,7 +915,7 @@ impl MainWorker { /// was not configured to create inspector. pub fn create_sync_inspector_session( &mut self, - cb: deno_core::InspectorSessionSend, + cb: deno_core::InspectorSessionCallback, ) -> LocalSyncInspectorSession { self.js_runtime.maybe_init_inspector(); let insp = self.js_runtime.inspector(); From a519e0dbc06049f6922a700e3b2ec2795e63486f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Mon, 15 Sep 2025 23:55:53 +0200 Subject: [PATCH 08/20] minimal changes --- runtime/inspector_server.rs | 5 ++--- runtime/worker.rs | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/runtime/inspector_server.rs b/runtime/inspector_server.rs index 1c47422d0a3502..7e9ae65d055850 100644 --- a/runtime/inspector_server.rs +++ b/runtime/inspector_server.rs @@ -417,7 +417,7 @@ async fn server( /// task yielding. async fn pump_websocket_messages( mut websocket: WebSocket>, - mut inbound_tx: UnboundedSender, + inbound_tx: UnboundedSender, mut outbound_rx: UnboundedReceiver, ) { 'pump: loop { @@ -430,8 +430,7 @@ async fn pump_websocket_messages( match msg.opcode { OpCode::Text => { if let Ok(s) = String::from_utf8(msg.payload.to_vec()) { - let _ = inbound_tx.send(s); - eprintln!("sent a message from WS to inspector"); + let _ = inbound_tx.unbounded_send(s); } } OpCode::Close => { diff --git a/runtime/worker.rs b/runtime/worker.rs index 16de1e720ff1cc..a71420140d6bce 100644 --- a/runtime/worker.rs +++ b/runtime/worker.rs @@ -915,7 +915,7 @@ impl MainWorker { /// was not configured to create inspector. pub fn create_sync_inspector_session( &mut self, - cb: deno_core::InspectorSessionCallback, + cb: deno_core::InspectorSessionSend, ) -> LocalSyncInspectorSession { self.js_runtime.maybe_init_inspector(); let insp = self.js_runtime.inspector(); From aae0acfebd11920e821d3ee96f6ad4851595df77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 16 Sep 2025 00:06:56 +0200 Subject: [PATCH 09/20] change branch --- Cargo.lock | 3 +++ Cargo.toml | 2 +- cli/lib/worker.rs | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dccd776dfc97aa..6893e7c401f7f6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1931,6 +1931,7 @@ dependencies = [ [[package]] name = "deno_core" version = "0.357.0" +source = "git+https://github.com/bartlomieju/deno_core.git?branch=sync_inspector2#f097d066d78e7c4fa136cd5fdac7addd13d3d323" dependencies = [ "anyhow", "az", @@ -2647,6 +2648,7 @@ dependencies = [ [[package]] name = "deno_ops" version = "0.233.0" +source = "git+https://github.com/bartlomieju/deno_core.git?branch=sync_inspector2#f097d066d78e7c4fa136cd5fdac7addd13d3d323" dependencies = [ "indexmap 2.9.0", "proc-macro-rules", @@ -8248,6 +8250,7 @@ dependencies = [ [[package]] name = "serde_v8" version = "0.266.0" +source = "git+https://github.com/bartlomieju/deno_core.git?branch=sync_inspector2#f097d066d78e7c4fa136cd5fdac7addd13d3d323" dependencies = [ "deno_error", "num-bigint", diff --git a/Cargo.toml b/Cargo.toml index 889e2e445e2e82..9fe75d68be2620 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -539,4 +539,4 @@ opt-level = 3 opt-level = 3 [patch.crates-io] -deno_core = { path = "../deno_core/core" } +deno_core = { git = "https://github.com/bartlomieju/deno_core.git", branch = "sync_inspector2" } diff --git a/cli/lib/worker.rs b/cli/lib/worker.rs index 31f00a2acd4d3f..f2255724224d1d 100644 --- a/cli/lib/worker.rs +++ b/cli/lib/worker.rs @@ -818,7 +818,7 @@ impl LibMainWorker { #[inline] pub fn create_sync_inspector_session( &mut self, - cb: deno_core::InspectorSessionCallback, + cb: deno_core::InspectorSessionSend, ) -> LocalSyncInspectorSession { self.worker.create_sync_inspector_session(cb) } From cff9c41d35823f92009f9ff7cf3b2feab10149c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 16 Sep 2025 15:05:47 +0200 Subject: [PATCH 10/20] update to latest --- Cargo.lock | 6 +++--- Cargo.toml | 6 ++---- cli/lib/worker.rs | 4 ++-- cli/tools/coverage/mod.rs | 6 +++--- cli/tools/repl/session.rs | 4 ++-- cli/tools/run/hmr.rs | 9 +++------ ext/node/ops/inspector.rs | 4 ++-- runtime/worker.rs | 6 +++--- 8 files changed, 20 insertions(+), 25 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6893e7c401f7f6..8973dd2a9132e3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1931,7 +1931,7 @@ dependencies = [ [[package]] name = "deno_core" version = "0.357.0" -source = "git+https://github.com/bartlomieju/deno_core.git?branch=sync_inspector2#f097d066d78e7c4fa136cd5fdac7addd13d3d323" +source = "git+https://github.com/denoland/deno_core.git?branch=main#63d156ca9d704e10b85b4938a7593801d46859ab" dependencies = [ "anyhow", "az", @@ -2648,7 +2648,7 @@ dependencies = [ [[package]] name = "deno_ops" version = "0.233.0" -source = "git+https://github.com/bartlomieju/deno_core.git?branch=sync_inspector2#f097d066d78e7c4fa136cd5fdac7addd13d3d323" +source = "git+https://github.com/denoland/deno_core.git?branch=main#63d156ca9d704e10b85b4938a7593801d46859ab" dependencies = [ "indexmap 2.9.0", "proc-macro-rules", @@ -8250,7 +8250,7 @@ dependencies = [ [[package]] name = "serde_v8" version = "0.266.0" -source = "git+https://github.com/bartlomieju/deno_core.git?branch=sync_inspector2#f097d066d78e7c4fa136cd5fdac7addd13d3d323" +source = "git+https://github.com/denoland/deno_core.git?branch=main#63d156ca9d704e10b85b4938a7593801d46859ab" dependencies = [ "deno_error", "num-bigint", diff --git a/Cargo.toml b/Cargo.toml index 9fe75d68be2620..e489003d947c68 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -62,7 +62,8 @@ 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.357.0" } +deno_core = { git = "https://github.com/denoland/deno_core.git", branch = "main" } deno_cache_dir = "=0.25.0" deno_doc = "=0.183.0" @@ -537,6 +538,3 @@ opt-level = 3 opt-level = 3 [profile.release.package.deno_npm_cache] opt-level = 3 - -[patch.crates-io] -deno_core = { git = "https://github.com/bartlomieju/deno_core.git", branch = "sync_inspector2" } diff --git a/cli/lib/worker.rs b/cli/lib/worker.rs index f2255724224d1d..05aff03ab363ea 100644 --- a/cli/lib/worker.rs +++ b/cli/lib/worker.rs @@ -23,7 +23,7 @@ use deno_runtime::deno_core; use deno_runtime::deno_core::CompiledWasmModuleStore; use deno_runtime::deno_core::Extension; use deno_runtime::deno_core::JsRuntime; -use deno_runtime::deno_core::LocalSyncInspectorSession; +use deno_runtime::deno_core::LocalInspectorSession; use deno_runtime::deno_core::ModuleLoader; use deno_runtime::deno_core::SharedArrayBufferStore; use deno_runtime::deno_core::error::CoreError; @@ -819,7 +819,7 @@ impl LibMainWorker { pub fn create_sync_inspector_session( &mut self, cb: deno_core::InspectorSessionSend, - ) -> LocalSyncInspectorSession { + ) -> LocalInspectorSession { self.worker.create_sync_inspector_session(cb) } diff --git a/cli/tools/coverage/mod.rs b/cli/tools/coverage/mod.rs index c0080e12819067..c708112a0107ff 100644 --- a/cli/tools/coverage/mod.rs +++ b/cli/tools/coverage/mod.rs @@ -16,7 +16,7 @@ use deno_config::glob::FileCollector; use deno_config::glob::FilePatterns; use deno_config::glob::PathOrPattern; use deno_config::glob::PathOrPatternSet; -use deno_core::LocalSyncInspectorSession; +use deno_core::LocalInspectorSession; use deno_core::anyhow::Context; use deno_core::anyhow::anyhow; use deno_core::error::AnyError; @@ -144,13 +144,13 @@ impl CoverageCollectorState { pub struct CoverageCollector { pub state: CoverageCollectorState, - session: LocalSyncInspectorSession, + session: LocalInspectorSession, } impl CoverageCollector { pub fn new( state: CoverageCollectorState, - session: LocalSyncInspectorSession, + session: LocalInspectorSession, ) -> Self { Self { state, session } } diff --git a/cli/tools/repl/session.rs b/cli/tools/repl/session.rs index c40e4e4e85875a..25733dcf624480 100644 --- a/cli/tools/repl/session.rs +++ b/cli/tools/repl/session.rs @@ -22,7 +22,7 @@ use deno_ast::swc::ecma_visit::VisitWith; use deno_ast::swc::ecma_visit::noop_visit_type; use deno_core::InspectorMsgKind; use deno_core::InspectorPostMessageError; -use deno_core::LocalSyncInspectorSession; +use deno_core::LocalInspectorSession; use deno_core::PollEventLoopOptions; use deno_core::anyhow::anyhow; use deno_core::error::AnyError; @@ -181,7 +181,7 @@ pub struct ReplSession { resolver: Arc, // NB: `session` and `state` must come before Worker, so that relevant V8 objects // are dropped before the isolate is dropped with `worker`. - session: LocalSyncInspectorSession, + session: LocalInspectorSession, state: ReplSessionState, pub worker: MainWorker, pub context_id: u64, diff --git a/cli/tools/run/hmr.rs b/cli/tools/run/hmr.rs index 7d94c9a7ca72fd..f638fd28612abb 100644 --- a/cli/tools/run/hmr.rs +++ b/cli/tools/run/hmr.rs @@ -7,7 +7,7 @@ use std::sync::atomic::AtomicI32; use deno_core::InspectorPostMessageError; use deno_core::InspectorPostMessageErrorKind; -use deno_core::LocalSyncInspectorSession; +use deno_core::LocalInspectorSession; use deno_core::error::CoreError; use deno_core::parking_lot::Mutex; use deno_core::serde_json::json; @@ -182,15 +182,12 @@ impl HmrRunnerState { /// of an ES module cannot be hot-replaced. In such situation the runner will /// force a full restart of a program by notifying the `FileWatcher`. pub struct HmrRunner { - session: LocalSyncInspectorSession, + session: LocalInspectorSession, state: HmrRunnerState, } impl HmrRunner { - pub fn new( - state: HmrRunnerState, - session: LocalSyncInspectorSession, - ) -> Self { + pub fn new(state: HmrRunnerState, session: LocalInspectorSession) -> Self { Self { session, state } } diff --git a/ext/node/ops/inspector.rs b/ext/node/ops/inspector.rs index 4a69b9cf44e8a6..3937444b1990ca 100644 --- a/ext/node/ops/inspector.rs +++ b/ext/node/ops/inspector.rs @@ -81,7 +81,7 @@ pub fn op_inspector_emit_protocol_event( } struct JSInspectorSession { - session: RefCell>, + session: RefCell>, } // SAFETY: we're sure this can be GCed @@ -153,7 +153,7 @@ where } }); - let session = JsRuntimeInspector::create_local_sync_session( + let session = JsRuntimeInspector::create_local_session( inspector, callback, InspectorSessionOptions { diff --git a/runtime/worker.rs b/runtime/worker.rs index a71420140d6bce..cdb3d5721cb718 100644 --- a/runtime/worker.rs +++ b/runtime/worker.rs @@ -20,7 +20,7 @@ use deno_core::InspectorSessionKind; use deno_core::InspectorSessionOptions; use deno_core::JsRuntime; use deno_core::JsRuntimeInspector; -use deno_core::LocalSyncInspectorSession; +use deno_core::LocalInspectorSession; use deno_core::ModuleCodeString; use deno_core::ModuleId; use deno_core::ModuleLoader; @@ -916,11 +916,11 @@ impl MainWorker { pub fn create_sync_inspector_session( &mut self, cb: deno_core::InspectorSessionSend, - ) -> LocalSyncInspectorSession { + ) -> LocalInspectorSession { self.js_runtime.maybe_init_inspector(); let insp = self.js_runtime.inspector(); - JsRuntimeInspector::create_local_sync_session( + JsRuntimeInspector::create_local_session( insp, cb, InspectorSessionOptions { From 73ee158b57555582a4574ed1459f52eb1bc928eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 16 Sep 2025 15:06:30 +0200 Subject: [PATCH 11/20] remove a.jks --- a.js | 1 - tests/util/std | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) delete mode 100644 a.js diff --git a/a.js b/a.js deleted file mode 100644 index 7d37e87e63e2af..00000000000000 --- a/a.js +++ /dev/null @@ -1 +0,0 @@ -setInterval(() => console.log("hello there"), 5_000); diff --git a/tests/util/std b/tests/util/std index 63b3348ab11941..1f032bb7e112ea 160000 --- a/tests/util/std +++ b/tests/util/std @@ -1 +1 @@ -Subproject commit 63b3348ab11941b92e3ab4da8429b32e488f8df1 +Subproject commit 1f032bb7e112ea572ce9df5d83675220d331079e From 2fbbabca08461d66f6bc1c460cf1a7ff4d606c7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 16 Sep 2025 15:12:39 +0200 Subject: [PATCH 12/20] simplify --- cli/lib/worker.rs | 4 +- cli/tools/coverage/mod.rs | 102 +++++++------------------------------- cli/tools/repl/session.rs | 2 +- cli/worker.rs | 4 +- runtime/worker.rs | 2 +- 5 files changed, 23 insertions(+), 91 deletions(-) diff --git a/cli/lib/worker.rs b/cli/lib/worker.rs index 05aff03ab363ea..3cc45a9e5d5e4d 100644 --- a/cli/lib/worker.rs +++ b/cli/lib/worker.rs @@ -816,11 +816,11 @@ impl LibMainWorker { } #[inline] - pub fn create_sync_inspector_session( + pub fn create_inspector_session( &mut self, cb: deno_core::InspectorSessionSend, ) -> LocalInspectorSession { - self.worker.create_sync_inspector_session(cb) + self.worker.create_inspector_session(cb) } #[inline] diff --git a/cli/tools/coverage/mod.rs b/cli/tools/coverage/mod.rs index c708112a0107ff..47f30ed7c1218d 100644 --- a/cli/tools/coverage/mod.rs +++ b/cli/tools/coverage/mod.rs @@ -57,6 +57,10 @@ use merge::ProcessCoverage; static NEXT_MSG_ID: AtomicI32 = AtomicI32::new(0); +fn next_msg_id() -> i32 { + NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed) +} + #[derive(Debug)] pub struct CoverageCollectorInner { dir: PathBuf, @@ -76,19 +80,8 @@ impl CoverageCollectorState { pub fn callback(&self, msg: deno_core::InspectorMsg) { let deno_core::InspectorMsgKind::Message(msg_id) = msg.kind else { - let _message: serde_json::Value = - serde_json::from_str(&msg.content).unwrap(); - // log::error!("CoverageCollector received a notification {:?}", message); return; }; - - // eprintln!( - // "callback {} {:#?}", - // // "callback {} {:#?} {}", - // msg_id, - // self.0.lock().coverage_msg_id.as_ref(), - // // msg.content - // ); let maybe_coverage_msg_id = self.0.lock().coverage_msg_id.as_ref().cloned(); if let Some(coverage_msg_id) = maybe_coverage_msg_id @@ -99,12 +92,10 @@ impl CoverageCollectorState { let coverages: cdp::TakePreciseCoverageResponse = serde_json::from_value(message["result"].clone()).unwrap(); self.write_coverages(coverages.result); - // eprintln!("written coverages"); } } fn write_coverages(&self, script_coverages: Vec) { - // eprintln!("got coverages {}", script_coverages.len()); 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 @@ -156,76 +147,23 @@ impl CoverageCollector { } pub fn start_collecting(&mut self) { - // TODO(barltomieju): is it actually necessary to call this one? - self.enable_debugger(); - - self.enable_profiler(); - self.start_precise_coverage(cdp::StartPreciseCoverageArgs { - call_count: true, - detailed: true, - allow_triggered_updates: false, - }); - } - - pub fn stop_collecting(&mut self) -> Result<(), CoreError> { - fs::create_dir_all(&self.state.0.lock().dir)?; - self.take_precise_coverage(); - - // TODO(barltomieju): is it actually necessary to call these? - self.disable_debugger(); - self.disable_profiler(); - - Ok(()) - } - - // TODO(barltomieju): is it actually necessary to call this? - fn enable_debugger(&mut self) { - self.session.post_message::<()>( - NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed), - "Debugger.enable", - None, - ); - } - - fn enable_profiler(&mut self) { - self.session.post_message::<()>( - NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed), - "Profiler.enable", - None, - ); - } - - // TODO(barltomieju): is it actually necessary to call this? - fn disable_debugger(&mut self) { - self.session.post_message::<()>( - NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed), - "Debugger.disable", - None, - ); - } - - // TODO(barltomieju): is it actually necessary to call this? - fn disable_profiler(&mut self) { - self.session.post_message::<()>( - NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed), - "Profiler.disable", - None, - ); - } - - fn start_precise_coverage( - &mut self, - parameters: cdp::StartPreciseCoverageArgs, - ) { + self + .session + .post_message::<()>(next_msg_id(), "Profiler.enable", None); self.session.post_message( - NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed), + next_msg_id(), "Profiler.startPreciseCoverage", - Some(parameters), + Some(cdp::StartPreciseCoverageArgs { + call_count: true, + detailed: true, + allow_triggered_updates: false, + }), ); } - fn take_precise_coverage(&mut self) { - let msg_id = NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + 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); self.session.post_message::<()>( @@ -233,13 +171,7 @@ impl CoverageCollector { "Profiler.takePreciseCoverage", None, ); - // eprintln!("take precise coverage {}", 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) - // todo!() + Ok(()) } } diff --git a/cli/tools/repl/session.rs b/cli/tools/repl/session.rs index 25733dcf624480..6334f98ac08acd 100644 --- a/cli/tools/repl/session.rs +++ b/cli/tools/repl/session.rs @@ -317,7 +317,7 @@ impl ReplSession { let state = repl_session_state.clone(); let callback = Box::new(move |message| repl_session_state.callback(message)); - let mut session = worker.create_sync_inspector_session(callback); + let mut session = worker.create_inspector_session(callback); session.post_message::<()>(next_msg_id(), "Runtime.enable", None); diff --git a/cli/worker.rs b/cli/worker.rs index d508742c2533f8..52167b01250aef 100644 --- a/cli/worker.rs +++ b/cli/worker.rs @@ -225,7 +225,7 @@ impl CliMainWorker { let state = hmr_runner_state.clone(); let callback = Box::new(move |message| hmr_runner_state.callback(message)); - let session = self.worker.create_sync_inspector_session(callback); + let session = self.worker.create_inspector_session(callback); let mut hmr_runner = HmrRunner::new(state, session); hmr_runner.start(); @@ -243,7 +243,7 @@ impl CliMainWorker { let callback = Box::new(move |message| coverage_collector_state.callback(message)); - let session = self.worker.create_sync_inspector_session(callback); + let session = self.worker.create_inspector_session(callback); let mut coverage_collector = CoverageCollector::new(state, session); coverage_collector.start_collecting(); diff --git a/runtime/worker.rs b/runtime/worker.rs index 6b0cf6e92b74a5..c7aebef92f2281 100644 --- a/runtime/worker.rs +++ b/runtime/worker.rs @@ -909,7 +909,7 @@ impl MainWorker { /// Create new inspector session. This function panics if Worker /// was not configured to create inspector. - pub fn create_sync_inspector_session( + pub fn create_inspector_session( &mut self, cb: deno_core::InspectorSessionSend, ) -> LocalInspectorSession { From 0b4dc5bfbeedb6cfab1f94a8c6ca0ca4221a0582 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 16 Sep 2025 15:56:50 +0200 Subject: [PATCH 13/20] should work for hmr --- cli/tools/run/hmr.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cli/tools/run/hmr.rs b/cli/tools/run/hmr.rs index f638fd28612abb..883e20bc4142fe 100644 --- a/cli/tools/run/hmr.rs +++ b/cli/tools/run/hmr.rs @@ -112,9 +112,10 @@ impl HmrRunnerState { serde_json::from_str(&msg.content).unwrap(); let mut state = self.0.lock(); let Some(message_state) = state.messages.remove(&msg_id) else { - state - .messages - .insert(msg_id, InspectorMessageState::Ready(message)); + state.messages.insert( + msg_id, + InspectorMessageState::Ready(message["result"].clone()), + ); return; }; let InspectorMessageState::WaitingFor(sender) = message_state else { From 37ff364cf85e21fd879283894fb0ac30146e5e61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Tue, 16 Sep 2025 23:52:16 +0200 Subject: [PATCH 14/20] temp --- Cargo.lock | 3 --- Cargo.toml | 7 +++++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1c0346d4f0795b..869a452532aea8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1931,7 +1931,6 @@ dependencies = [ [[package]] name = "deno_core" version = "0.357.0" -source = "git+https://github.com/denoland/deno_core.git?branch=main#63d156ca9d704e10b85b4938a7593801d46859ab" dependencies = [ "anyhow", "az", @@ -2648,7 +2647,6 @@ dependencies = [ [[package]] name = "deno_ops" version = "0.233.0" -source = "git+https://github.com/denoland/deno_core.git?branch=main#63d156ca9d704e10b85b4938a7593801d46859ab" dependencies = [ "indexmap 2.9.0", "proc-macro-rules", @@ -8251,7 +8249,6 @@ dependencies = [ [[package]] name = "serde_v8" version = "0.266.0" -source = "git+https://github.com/denoland/deno_core.git?branch=main#63d156ca9d704e10b85b4938a7593801d46859ab" dependencies = [ "deno_error", "num-bigint", diff --git a/Cargo.toml b/Cargo.toml index e489003d947c68..7c6ffa258926bf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -62,8 +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 = { git = "https://github.com/denoland/deno_core.git", branch = "main" } +deno_core = { version = "0.357.0" } deno_cache_dir = "=0.25.0" deno_doc = "=0.183.0" @@ -538,3 +537,7 @@ opt-level = 3 opt-level = 3 [profile.release.package.deno_npm_cache] opt-level = 3 + +[patch.crates-io] +# deno_core = { git = "https://github.com/bartlomieju/deno_core.git", branch = "sync_inspector2" } +deno_core = { path = "../deno_core/core" } From 2038565f352f5f470122fabd75b7c0f22bd64e9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 17 Sep 2025 00:15:12 +0200 Subject: [PATCH 15/20] try with another branch --- Cargo.lock | 3 +++ Cargo.toml | 7 ++----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 869a452532aea8..fb8b3eceb0e84f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1931,6 +1931,7 @@ dependencies = [ [[package]] name = "deno_core" version = "0.357.0" +source = "git+https://github.com/bartlomieju/deno_core.git?branch=inspector_local_session_borrow_mut_panic#fc6ae985a967fa25cf12f3d6f3c05276eeba6060" dependencies = [ "anyhow", "az", @@ -2647,6 +2648,7 @@ dependencies = [ [[package]] name = "deno_ops" version = "0.233.0" +source = "git+https://github.com/bartlomieju/deno_core.git?branch=inspector_local_session_borrow_mut_panic#fc6ae985a967fa25cf12f3d6f3c05276eeba6060" dependencies = [ "indexmap 2.9.0", "proc-macro-rules", @@ -8249,6 +8251,7 @@ dependencies = [ [[package]] name = "serde_v8" version = "0.266.0" +source = "git+https://github.com/bartlomieju/deno_core.git?branch=inspector_local_session_borrow_mut_panic#fc6ae985a967fa25cf12f3d6f3c05276eeba6060" dependencies = [ "deno_error", "num-bigint", diff --git a/Cargo.toml b/Cargo.toml index 7c6ffa258926bf..4cea95e4d59675 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -62,7 +62,8 @@ 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.357.0" } +deno_core = { git = "https://github.com/bartlomieju/deno_core.git", branch = "inspector_local_session_borrow_mut_panic" } deno_cache_dir = "=0.25.0" deno_doc = "=0.183.0" @@ -537,7 +538,3 @@ opt-level = 3 opt-level = 3 [profile.release.package.deno_npm_cache] opt-level = 3 - -[patch.crates-io] -# deno_core = { git = "https://github.com/bartlomieju/deno_core.git", branch = "sync_inspector2" } -deno_core = { path = "../deno_core/core" } From ed77f39bb72549cf1e1c750a15787e4290ae8633 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 17 Sep 2025 00:52:59 +0200 Subject: [PATCH 16/20] should fix now? --- Cargo.lock | 6 +++--- Cargo.toml | 3 +++ tests/integration/inspector_tests.rs | 2 ++ 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fb8b3eceb0e84f..9aa9da950dc889 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1931,7 +1931,7 @@ dependencies = [ [[package]] name = "deno_core" version = "0.357.0" -source = "git+https://github.com/bartlomieju/deno_core.git?branch=inspector_local_session_borrow_mut_panic#fc6ae985a967fa25cf12f3d6f3c05276eeba6060" +source = "git+https://github.com/bartlomieju/deno_core.git?branch=inspector_local_session_borrow_mut_panic#cbc72f4671bcad12f29ce779cf11318918f06aac" dependencies = [ "anyhow", "az", @@ -2648,7 +2648,7 @@ dependencies = [ [[package]] name = "deno_ops" version = "0.233.0" -source = "git+https://github.com/bartlomieju/deno_core.git?branch=inspector_local_session_borrow_mut_panic#fc6ae985a967fa25cf12f3d6f3c05276eeba6060" +source = "git+https://github.com/bartlomieju/deno_core.git?branch=inspector_local_session_borrow_mut_panic#cbc72f4671bcad12f29ce779cf11318918f06aac" dependencies = [ "indexmap 2.9.0", "proc-macro-rules", @@ -8251,7 +8251,7 @@ dependencies = [ [[package]] name = "serde_v8" version = "0.266.0" -source = "git+https://github.com/bartlomieju/deno_core.git?branch=inspector_local_session_borrow_mut_panic#fc6ae985a967fa25cf12f3d6f3c05276eeba6060" +source = "git+https://github.com/bartlomieju/deno_core.git?branch=inspector_local_session_borrow_mut_panic#cbc72f4671bcad12f29ce779cf11318918f06aac" dependencies = [ "deno_error", "num-bigint", diff --git a/Cargo.toml b/Cargo.toml index 4cea95e4d59675..e09d45aa3b0d31 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -538,3 +538,6 @@ opt-level = 3 opt-level = 3 [profile.release.package.deno_npm_cache] opt-level = 3 + +# [patch.crates-io] +# deno_core = { path = "../deno_core/core" } diff --git a/tests/integration/inspector_tests.rs b/tests/integration/inspector_tests.rs index 18f5cbd54d0714..0c5c70f7d1855f 100644 --- a/tests/integration/inspector_tests.rs +++ b/tests/integration/inspector_tests.rs @@ -603,6 +603,7 @@ async fn inspector_runtime_evaluate_does_not_crash() { .arg("repl") .arg("--allow-read") .arg(inspect_flag_with_unique_port("--inspect")) + .env("RUST_BACKTRACE", "1") .stdin(std::process::Stdio::piped()) .piped_output() .spawn() @@ -1025,6 +1026,7 @@ async fn inspector_memory() { .arg("run") .arg(inspect_flag_with_unique_port("--inspect-brk")) .arg(script) + .env("RUST_BACKTRACE", "1") .piped_output() .spawn() .unwrap(); From 5a04da929a8aea4e036fd7f16e8949f0d2776c37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 17 Sep 2025 01:26:10 +0200 Subject: [PATCH 17/20] update std --- tests/util/std | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/util/std b/tests/util/std index 1f032bb7e112ea..63b3348ab11941 160000 --- a/tests/util/std +++ b/tests/util/std @@ -1 +1 @@ -Subproject commit 1f032bb7e112ea572ce9df5d83675220d331079e +Subproject commit 63b3348ab11941b92e3ab4da8429b32e488f8df1 From e9dbd30898cebe4a6579e2325532344faff7c332 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 17 Sep 2025 12:00:06 +0200 Subject: [PATCH 18/20] bump --- Cargo.lock | 6 +++--- Cargo.toml | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e06bb5ada44771..8ae4c75417c13c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1931,7 +1931,7 @@ dependencies = [ [[package]] name = "deno_core" version = "0.357.0" -source = "git+https://github.com/bartlomieju/deno_core.git?branch=inspector_local_session_borrow_mut_panic#cbc72f4671bcad12f29ce779cf11318918f06aac" +source = "git+https://github.com/denoland/deno_core.git?branch=main#54ed5dc346a20e5993b575b3bea8ac4ac47b2d92" dependencies = [ "anyhow", "az", @@ -2648,7 +2648,7 @@ dependencies = [ [[package]] name = "deno_ops" version = "0.233.0" -source = "git+https://github.com/bartlomieju/deno_core.git?branch=inspector_local_session_borrow_mut_panic#cbc72f4671bcad12f29ce779cf11318918f06aac" +source = "git+https://github.com/denoland/deno_core.git?branch=main#54ed5dc346a20e5993b575b3bea8ac4ac47b2d92" dependencies = [ "indexmap 2.9.0", "proc-macro-rules", @@ -8251,7 +8251,7 @@ dependencies = [ [[package]] name = "serde_v8" version = "0.266.0" -source = "git+https://github.com/bartlomieju/deno_core.git?branch=inspector_local_session_borrow_mut_panic#cbc72f4671bcad12f29ce779cf11318918f06aac" +source = "git+https://github.com/denoland/deno_core.git?branch=main#54ed5dc346a20e5993b575b3bea8ac4ac47b2d92" dependencies = [ "deno_error", "num-bigint", diff --git a/Cargo.toml b/Cargo.toml index 667eedb8e0d090..753b83bb763a63 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -63,7 +63,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 = { git = "https://github.com/bartlomieju/deno_core.git", branch = "inspector_local_session_borrow_mut_panic" } +deno_core = { git = "https://github.com/denoland/deno_core.git", branch = "main" } deno_cache_dir = "=0.25.0" deno_doc = "=0.183.0" From b02927595abbb904fb4255dd888babfe990e1da2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 17 Sep 2025 13:05:42 +0200 Subject: [PATCH 19/20] update to 0.358.0 --- Cargo.lock | 15 +++++++++------ Cargo.toml | 6 +----- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c54ab890588c63..223d7873de631f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1930,8 +1930,9 @@ dependencies = [ [[package]] name = "deno_core" -version = "0.357.0" -source = "git+https://github.com/denoland/deno_core.git?branch=main#54ed5dc346a20e5993b575b3bea8ac4ac47b2d92" +version = "0.358.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "12b2303b5bd690de06b04fcd1b544d06e6a73a3ed30569da2504890223d435d8" dependencies = [ "anyhow", "az", @@ -2647,8 +2648,9 @@ dependencies = [ [[package]] name = "deno_ops" -version = "0.233.0" -source = "git+https://github.com/denoland/deno_core.git?branch=main#54ed5dc346a20e5993b575b3bea8ac4ac47b2d92" +version = "0.234.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f0d3a944fa9ee07021e2514aaf2754db6b30c294899730646b9b3bbae4a9583" dependencies = [ "indexmap 2.9.0", "proc-macro-rules", @@ -8250,8 +8252,9 @@ dependencies = [ [[package]] name = "serde_v8" -version = "0.266.0" -source = "git+https://github.com/denoland/deno_core.git?branch=main#54ed5dc346a20e5993b575b3bea8ac4ac47b2d92" +version = "0.267.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cc490dcfb7c8ded6aa0946af8c2f5833ca8ea22c414b9392366c9a8d5e1283c4" dependencies = [ "deno_error", "num-bigint", diff --git a/Cargo.toml b/Cargo.toml index debb299661f857..f36bdd47417a56 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -62,8 +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 = { git = "https://github.com/denoland/deno_core.git", branch = "main" } +deno_core = { version = "0.358.0" } deno_cache_dir = "=0.25.0" deno_doc = "=0.183.0" @@ -538,6 +537,3 @@ opt-level = 3 opt-level = 3 [profile.release.package.deno_npm_cache] opt-level = 3 - -# [patch.crates-io] -# deno_core = { path = "../deno_core/core" } From be9272a5b83d430c3eee1b7787b1169002b21cd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 17 Sep 2025 13:26:55 +0200 Subject: [PATCH 20/20] cleanup --- cli/tools/coverage/mod.rs | 40 ++++++++---- cli/tools/repl/session.rs | 116 +++++++++++++++------------------ cli/tools/run/hmr.rs | 133 +++++++++++++++----------------------- cli/tools/test/mod.rs | 1 - 4 files changed, 133 insertions(+), 157 deletions(-) diff --git a/cli/tools/coverage/mod.rs b/cli/tools/coverage/mod.rs index 47f30ed7c1218d..3084a454a07408 100644 --- a/cli/tools/coverage/mod.rs +++ b/cli/tools/coverage/mod.rs @@ -25,7 +25,6 @@ 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; @@ -99,7 +98,6 @@ impl CoverageCollectorState { 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 - // eprintln!("script coverage url {}", script_coverage.url); if script_coverage.url.is_empty() || script_coverage.url.starts_with("ext:") || script_coverage.url.starts_with("[ext:") @@ -114,21 +112,41 @@ impl CoverageCollectorState { let filename = format!("{}.json", Uuid::new_v4()); let filepath = self.0.lock().dir.join(filename); - // TODO(bartlomieju): remove unwrap - let mut out = BufWriter::new(File::create(&filepath).unwrap()); - // TODO(bartlomieju): remove unwrap - let coverage = serde_json::to_string(&script_coverage) - .map_err(JsErrorBox::from_err) - .unwrap(); + 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); - // TODO(bartlomieju): add logging - let _ = out.write_all(formatted_coverage.as_bytes()); - let _ = 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; + } } } } diff --git a/cli/tools/repl/session.rs b/cli/tools/repl/session.rs index 6334f98ac08acd..d692bbd5c8f15f 100644 --- a/cli/tools/repl/session.rs +++ b/cli/tools/repl/session.rs @@ -20,7 +20,6 @@ use deno_ast::swc::common::comments::CommentKind; use deno_ast::swc::ecma_visit::Visit; use deno_ast::swc::ecma_visit::VisitWith; use deno_ast::swc::ecma_visit::noop_visit_type; -use deno_core::InspectorMsgKind; use deno_core::InspectorPostMessageError; use deno_core::LocalInspectorSession; use deno_core::PollEventLoopOptions; @@ -221,76 +220,68 @@ impl ReplSessionState { } fn callback(&self, msg: deno_core::InspectorMsg) { - match msg.kind { - // TODO: duplicated in `cli/tools/run/hmr.rs` for the most part - deno_core::InspectorMsgKind::Message(msg_id) => { - let message: serde_json::Value = - match serde_json::from_str(&msg.content) { - Ok(v) => v, - Err(error) => match error.classify() { - serde_json::error::Category::Syntax => serde_json::json!({ - "id": msg_id, - "result": { - "result": { - "type": "error", - "description": "Unterminated string literal", - "value": "Unterminated string literal", - }, - "exceptionDetails": { - "exceptionId": 0, - "text": "Unterminated string literal", - "lineNumber": 0, - "columnNumber": 0 - }, - }, - }), - _ => panic!("Could not parse inspector message"), + let deno_core::InspectorMsgKind::Message(msg_id) = msg.kind else { + if let Ok(value) = serde_json::from_str(&msg.content) { + let _ = self.0.lock().notification_tx.unbounded_send(value); + } + return; + }; + + let message: serde_json::Value = match serde_json::from_str(&msg.content) { + Ok(v) => v, + Err(error) => match error.classify() { + serde_json::error::Category::Syntax => serde_json::json!({ + "id": msg_id, + "result": { + "result": { + "type": "error", + "description": "Unterminated string literal", + "value": "Unterminated string literal", + }, + "exceptionDetails": { + "exceptionId": 0, + "text": "Unterminated string literal", + "lineNumber": 0, + "columnNumber": 0 }, - }; + }, + }), + _ => panic!("Could not parse inspector message"), + }, + }; - let mut state = self.0.lock(); - let Some(message_state) = state.messages.remove(&msg_id) else { - state - .messages - .insert(msg_id, InspectorMessageState::Ready(message)); - return; - }; - let InspectorMessageState::WaitingFor(sender) = message_state else { - // Maybe log? This should be unreachable - return; - }; - let _ = sender.send(message); - } - InspectorMsgKind::Notification => { - if let Ok(value) = serde_json::from_str(&msg.content) { - let _ = self.0.lock().notification_tx.unbounded_send(value); - } - } - } + let mut state = self.0.lock(); + let Some(message_state) = state.messages.remove(&msg_id) else { + state + .messages + .insert(msg_id, InspectorMessageState::Ready(message)); + return; + }; + let InspectorMessageState::WaitingFor(sender) = message_state else { + return; + }; + let _ = sender.send(message); } async fn wait_for_response( &self, msg_id: i32, ) -> Result { - let value = - if let Some(message_state) = self.0.lock().messages.remove(&msg_id) { - let InspectorMessageState::Ready(value) = message_state else { - unreachable!(); - }; - value - } else { - let (tx, rx) = oneshot::channel(); - self - .0 - .lock() - .messages - .insert(msg_id, InspectorMessageState::WaitingFor(tx)); - rx.await.unwrap() + if let Some(message_state) = self.0.lock().messages.remove(&msg_id) { + let InspectorMessageState::Ready(mut value) = message_state else { + unreachable!(); }; - // TODO(bartlomieju): very specific, but that's probably okay for now, that - // each caller casts to wanted type? - Ok(value["result"].clone()) + return Ok(value["result"].take()); + } + + let (tx, rx) = oneshot::channel(); + self + .0 + .lock() + .messages + .insert(msg_id, InspectorMessageState::WaitingFor(tx)); + let mut value = rx.await.unwrap(); + Ok(value["result"].take()) } } @@ -326,7 +317,6 @@ impl ReplSession { // our inspector does not support a default context (0 is an invalid context id). let context_id: u64; - // TODO(bartlomieju): this will probably hang, might need to actually spin the event loop a bit here loop { let notification = notification_rx.next().await.unwrap(); let notification = diff --git a/cli/tools/run/hmr.rs b/cli/tools/run/hmr.rs index 883e20bc4142fe..346ae25e304d97 100644 --- a/cli/tools/run/hmr.rs +++ b/cli/tools/run/hmr.rs @@ -27,6 +27,9 @@ use crate::util::file_watcher::WatcherCommunicator; use crate::util::file_watcher::WatcherRestartMode; static NEXT_MSG_ID: AtomicI32 = AtomicI32::new(0); +fn next_id() -> i32 { + NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed) +} fn explain(response: &cdp::SetScriptSourceResponse) -> String { match response.status { @@ -106,29 +109,25 @@ impl HmrRunnerState { } pub fn callback(&self, msg: deno_core::InspectorMsg) { - match msg.kind { - deno_core::InspectorMsgKind::Message(msg_id) => { - let message: serde_json::Value = - serde_json::from_str(&msg.content).unwrap(); - let mut state = self.0.lock(); - let Some(message_state) = state.messages.remove(&msg_id) else { - state.messages.insert( - msg_id, - InspectorMessageState::Ready(message["result"].clone()), - ); - return; - }; - let InspectorMessageState::WaitingFor(sender) = message_state else { - // Maybe log? This should be unreachable - return; - }; - let _ = sender.send(message); - } - deno_core::InspectorMsgKind::Notification => { - let notification = serde_json::from_str(&msg.content).unwrap(); - self.handle_notification(notification); - } - } + let deno_core::InspectorMsgKind::Message(msg_id) = msg.kind else { + let notification = serde_json::from_str(&msg.content).unwrap(); + self.handle_notification(notification); + return; + }; + + let message: serde_json::Value = + serde_json::from_str(&msg.content).unwrap(); + let mut state = self.0.lock(); + let Some(message_state) = state.messages.remove(&msg_id) else { + state + .messages + .insert(msg_id, InspectorMessageState::Ready(message)); + return; + }; + let InspectorMessageState::WaitingFor(sender) = message_state else { + return; + }; + let _ = sender.send(message); } fn handle_notification(&self, notification: cdp::Notification) { @@ -192,21 +191,23 @@ impl HmrRunner { Self { session, state } } - // TODO(bartlomieju): this code is duplicated in `cli/tools/coverage/mod.rs` pub fn start(&mut self) { - self.enable_debugger(); + self + .session + .post_message::<()>(next_id(), "Debugger.enable", None); + self + .session + .post_message::<()>(next_id(), "Runtime.enable", None); } fn watcher(&self) -> Arc { self.state.0.lock().watcher_communicator.clone() } - // TODO(bartlomieju): this code is duplicated in `cli/tools/coverage/mod.rs` pub fn stop(&mut self) { self .watcher() .change_restart_mode(WatcherRestartMode::Automatic); - self.disable_debugger(); } pub async fn run(&mut self) -> Result<(), CoreError> { @@ -270,7 +271,11 @@ impl HmrRunner { let mut tries = 1; loop { let msg_id = self.set_script_source(&id, source_code.as_str()); - let result = self.wait_for_response::(msg_id).await?; + let value = self.wait_for_response(msg_id).await?; + let result: cdp::SetScriptSourceResponse = serde_json::from_value(value).map_err(|e| { + InspectorPostMessageErrorKind::JsBox(JsErrorBox::from_err(e)).into_box() + })?; + if matches!(result.status, cdp::Status::Ok) { self.dispatch_hmr_event(module_url.as_str()); @@ -294,66 +299,30 @@ impl HmrRunner { } } - // TODO(bartlomieju): this code is duplicated in `cli/tools/coverage/mod.rs` - fn enable_debugger(&mut self) { - // TODO(barltomieju): is it actually necessary to call this? - self.session.post_message::<()>( - NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed), - "Debugger.enable", - None, - ); - - self.session.post_message::<()>( - NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed), - "Runtime.enable", - None, - ); - } - - // TODO(bartlomieju): this code is duplicated in `cli/tools/coverage/mod.rs` - fn disable_debugger(&mut self) { - // TODO(barltomieju): is it actually necessary to call this? - self.session.post_message::<()>( - NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed), - "Debugger.disable", - None, - ); - // TODO(barltomieju): is it actually necessary to call this? - self.session.post_message::<()>( - NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed), - "Runtime.disable", - None, - ); - } - - async fn wait_for_response( + async fn wait_for_response( &self, msg_id: i32, - ) -> Result { - let value = if let Some(message_state) = - self.state.0.lock().messages.remove(&msg_id) - { - let InspectorMessageState::Ready(value) = message_state else { + ) -> Result { + if let Some(message_state) = self.state.0.lock().messages.remove(&msg_id) { + let InspectorMessageState::Ready(mut value) = message_state else { unreachable!(); }; - value - } else { - let (tx, rx) = oneshot::channel(); - self - .state - .0 - .lock() - .messages - .insert(msg_id, InspectorMessageState::WaitingFor(tx)); - rx.await.unwrap() - }; - serde_json::from_value::(value).map_err(|e| { - InspectorPostMessageErrorKind::JsBox(JsErrorBox::from_err(e)).into_box() - }) + return Ok(value["result"].take()); + } + + let (tx, rx) = oneshot::channel(); + self + .state + .0 + .lock() + .messages + .insert(msg_id, InspectorMessageState::WaitingFor(tx)); + let mut value = rx.await.unwrap(); + Ok(value["result"].take()) } fn set_script_source(&mut self, script_id: &str, source: &str) -> i32 { - let msg_id = NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + let msg_id = next_id(); self.session.post_message( msg_id, "Debugger.setScriptSource", @@ -373,7 +342,7 @@ impl HmrRunner { ); self.session.post_message( - NEXT_MSG_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed), + next_id(), "Runtime.evaluate", Some(json!({ "expression": expr, diff --git a/cli/tools/test/mod.rs b/cli/tools/test/mod.rs index 984e2e4d86c1b6..bd4e1a99c72d44 100644 --- a/cli/tools/test/mod.rs +++ b/cli/tools/test/mod.rs @@ -805,7 +805,6 @@ async fn test_specifier_inner( if let Some(coverage_collector) = &mut coverage_collector { coverage_collector.stop_collecting()?; } - // worker.run_up_to_duration(Duration::from_millis(0)).await?; Ok(()) }