feat: add --inspect-wait flag (#17001)

This commit adds new "--inspect-wait" flag which works similarly
to "--inspect-brk" in that it waits for inspector session to be
established before running code. However it doesn't break on the first
statement of user code, but instead runs it as soon as a session
is established.

cli/args/flags.rs

@@ -312,6 +312,7 @@ pub struct Flags {
   pub ignore: Vec<PathBuf>,
   pub import_map_path: Option<String>,
   pub inspect_brk: Option<SocketAddr>,
+  pub inspect_wait: Option<SocketAddr>,
   pub inspect: Option<SocketAddr>,
   pub location: Option<Url>,
   pub lock_write: bool,
@@ -1482,6 +1483,7 @@ fn run_subcommand<'a>() -> Command<'a> {
     .arg(
       watch_arg(true)
         .conflicts_with("inspect")
+        .conflicts_with("inspect-wait")
         .conflicts_with("inspect-brk"),
     )
     .arg(no_clear_screen_arg())
@@ -1622,6 +1624,7 @@ fn test_subcommand<'a>() -> Command<'a> {
         .takes_value(true)
         .value_name("DIR")
         .conflicts_with("inspect")
+        .conflicts_with("inspect-wait")
         .conflicts_with("inspect-brk")
         .help("UNSTABLE: Collect coverage profile data into DIR"),
     )
@@ -1964,7 +1967,20 @@ fn inspect_args(app: Command) -> Command {
         .long("inspect-brk")
         .value_name("HOST:PORT")
         .help(
-          "Activate inspector on host:port and break at start of user script",
+          "Activate inspector on host:port, wait for debugger to connect and break at the start of user script",
+        )
+        .min_values(0)
+        .max_values(1)
+        .require_equals(true)
+        .takes_value(true)
+        .validator(inspect_arg_validate),
+    )
+    .arg(
+      Arg::new("inspect-wait")
+        .long("inspect-wait")
+        .value_name("HOST:PORT")
+        .help(
+          "Activate inspector on host:port and wait for debugger to connect before running user code",
         )
         .min_values(0)
         .max_values(1)
@@ -3040,6 +3056,15 @@ fn inspect_arg_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
   } else {
     None
   };
+  flags.inspect_wait = if matches.is_present("inspect-wait") {
+    if let Some(host) = matches.value_of("inspect-wait") {
+      Some(host.parse().unwrap())
+    } else {
+      Some(default())
+    }
+  } else {
+    None
+  };
 }
 
 fn import_map_arg_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
@@ -5875,6 +5900,38 @@ mod tests {
     );
   }
 
+  #[test]
+  fn inspect_wait() {
+    let r = flags_from_vec(svec!["deno", "run", "--inspect-wait", "foo.js"]);
+    assert_eq!(
+      r.unwrap(),
+      Flags {
+        subcommand: DenoSubcommand::Run(RunFlags {
+          script: "foo.js".to_string(),
+        }),
+        inspect_wait: Some("127.0.0.1:9229".parse().unwrap()),
+        ..Flags::default()
+      }
+    );
+
+    let r = flags_from_vec(svec![
+      "deno",
+      "run",
+      "--inspect-wait=127.0.0.1:3567",
+      "foo.js"
+    ]);
+    assert_eq!(
+      r.unwrap(),
+      Flags {
+        subcommand: DenoSubcommand::Run(RunFlags {
+          script: "foo.js".to_string(),
+        }),
+        inspect_wait: Some("127.0.0.1:3567".parse().unwrap()),
+        ..Flags::default()
+      }
+    );
+  }
+
   #[test]
   fn compile() {
     let r = flags_from_vec(svec![

cli/args/mod.rs

@@ -341,7 +341,11 @@ impl CliOptions {
   }
 
   pub fn resolve_inspector_server(&self) -> Option<InspectorServer> {
-    let maybe_inspect_host = self.flags.inspect.or(self.flags.inspect_brk);
+    let maybe_inspect_host = self
+      .flags
+      .inspect
+      .or(self.flags.inspect_brk)
+      .or(self.flags.inspect_wait);
     maybe_inspect_host
       .map(|host| InspectorServer::new(host, version::get_user_agent()))
   }
@@ -466,13 +470,19 @@ impl CliOptions {
 
   /// If the --inspect or --inspect-brk flags are used.
   pub fn is_inspecting(&self) -> bool {
-    self.flags.inspect.is_some() || self.flags.inspect_brk.is_some()
+    self.flags.inspect.is_some()
+      || self.flags.inspect_brk.is_some()
+      || self.flags.inspect_wait.is_some()
   }
 
   pub fn inspect_brk(&self) -> Option<SocketAddr> {
     self.flags.inspect_brk
   }
 
+  pub fn inspect_wait(&self) -> Option<SocketAddr> {
+    self.flags.inspect_wait
+  }
+
   pub fn log_level(&self) -> Option<log::Level> {
     self.flags.log_level
   }

cli/standalone.rs

@@ -299,6 +299,7 @@ pub async fn run(
     web_worker_pre_execute_module_cb: web_worker_cb,
     maybe_inspector_server: None,
     should_break_on_first_statement: false,
+    should_wait_for_inspector_session: false,
     module_loader,
     npm_resolver: None, // not currently supported
     get_error_class_fn: Some(&get_error_class_name),

cli/tests/inspector_tests.rs

@@ -13,6 +13,7 @@ use deno_runtime::deno_websocket::tokio_tungstenite::tungstenite;
 use std::io::BufRead;
 use std::process::Child;
 use test_util as util;
+use test_util::TempDir;
 use tokio::net::TcpStream;
 use util::http_server;
 
@@ -1319,4 +1320,59 @@ mod inspector {
 
     assert_eq!(tester.child.wait().unwrap().code(), Some(1));
   }
+
+  #[tokio::test]
+  async fn inspector_wait() {
+    let script = util::testdata_path().join("inspector/inspect_wait.js");
+    let temp_dir = TempDir::new();
+
+    let child = util::deno_cmd()
+      .current_dir(temp_dir.path())
+      .arg("run")
+      .arg("--quiet")
+      .arg("-A")
+      .arg(inspect_flag_with_unique_port("--inspect-wait"))
+      .arg(script)
+      .stdout(std::process::Stdio::piped())
+      .stderr(std::process::Stdio::piped())
+      .spawn()
+      .unwrap();
+
+    tokio::time::sleep(tokio::time::Duration::from_millis(300)).await;
+    assert!(!temp_dir.path().join("hello.txt").exists());
+
+    let mut tester = InspectorTester::create(child, ignore_script_parsed).await;
+
+    tester.assert_stderr_for_inspect_brk();
+    tester
+      .send_many(&[
+        json!({"id":1,"method":"Runtime.enable"}),
+        json!({"id":2,"method":"Debugger.enable"}),
+      ])
+      .await;
+    tester.assert_received_messages(
+      &[
+        r#"{"id":1,"result":{}}"#,
+        r#"{"id":2,"result":{"debuggerId":"#,
+      ],
+      &[
+        r#"{"method":"Runtime.executionContextCreated","params":{"context":{"id":1,"#,
+      ],
+    )
+    .await;
+    // TODO(bartlomieju): ideally this shouldn't be needed, but currently there's
+    // no way to express that in inspector code. Most clients always send this
+    // message anyway.
+    tester
+      .send(json!({"id":3,"method":"Runtime.runIfWaitingForDebugger"}))
+      .await;
+    tester
+      .assert_received_messages(&[r#"{"id":3,"result":{}}"#], &[])
+      .await;
+    assert_eq!(&tester.stderr_line(), "Debugger session started.");
+    tokio::time::sleep(tokio::time::Duration::from_millis(300)).await;
+    assert_eq!(&tester.stderr_line(), "did run");
+    assert!(temp_dir.path().join("hello.txt").exists());
+    tester.child.kill().unwrap();
+  }
 }

cli/tests/testdata/inspector/inspect_wait.js

@@ -0,0 +1,2 @@
+Deno.writeTextFileSync("./hello.txt", "hello world");
+console.error("did run");

cli/worker.rs

@@ -472,7 +472,6 @@ async fn create_main_worker_internal(
   let module_loader = CliModuleLoader::new(ps.clone());
 
   let maybe_inspector_server = ps.maybe_inspector_server.clone();
-  let should_break_on_first_statement = ps.options.inspect_brk().is_some();
 
   let create_web_worker_cb =
     create_web_worker_callback(ps.clone(), stdio.clone());
@@ -533,7 +532,8 @@ async fn create_main_worker_internal(
     web_worker_preload_module_cb,
     web_worker_pre_execute_module_cb,
     maybe_inspector_server,
-    should_break_on_first_statement,
+    should_break_on_first_statement: ps.options.inspect_brk().is_some(),
+    should_wait_for_inspector_session: ps.options.inspect_wait().is_some(),
     module_loader,
     npm_resolver: Some(Rc::new(ps.npm_resolver.clone())),
     get_error_class_fn: Some(&errors::get_error_class_name),
@@ -755,6 +755,7 @@ mod tests {
       create_web_worker_cb: Arc::new(|_| unreachable!()),
       maybe_inspector_server: None,
       should_break_on_first_statement: false,
+      should_wait_for_inspector_session: false,
       module_loader: Rc::new(FsModuleLoader),
       npm_resolver: None,
       get_error_class_fn: None,

core/inspector.rs

@@ -347,6 +347,23 @@ impl JsRuntimeInspector {
     }
   }
 
+  /// This function blocks the thread until at least one inspector client has
+  /// established a websocket connection.
+  pub fn wait_for_session(&mut self) {
+    loop {
+      match self.sessions.get_mut().established.iter_mut().next() {
+        Some(_session) => {
+          self.flags.get_mut().waiting_for_session = false;
+          break;
+        }
+        None => {
+          self.flags.get_mut().waiting_for_session = true;
+          let _ = self.poll_sessions(None).unwrap();
+        }
+      };
+    }
+  }
+
   /// This function blocks the thread until at least one inspector client has
   /// established a websocket connection.
   ///

runtime/examples/hello_runtime.rs

@@ -54,6 +54,7 @@ async fn main() -> Result<(), AnyError> {
     create_web_worker_cb,
     maybe_inspector_server: None,
     should_break_on_first_statement: false,
+    should_wait_for_inspector_session: false,
     module_loader,
     npm_resolver: None,
     get_error_class_fn: Some(&get_error_class_name),

runtime/inspector_server.rs

@@ -67,7 +67,7 @@ impl InspectorServer {
     &self,
     module_url: String,
     js_runtime: &mut JsRuntime,
-    should_break_on_first_statement: bool,
+    wait_for_session: bool,
   ) {
     let inspector_rc = js_runtime.inspector();
     let mut inspector = inspector_rc.borrow_mut();
@@ -78,7 +78,7 @@ impl InspectorServer {
       session_sender,
       deregister_rx,
       module_url,
-      should_break_on_first_statement,
+      wait_for_session,
     );
     self.register_inspector_tx.unbounded_send(info).unwrap();
   }
@@ -233,7 +233,7 @@ async fn server(
         info.get_websocket_debugger_url()
       );
       eprintln!("Visit chrome://inspect to connect to the debugger.");
-      if info.should_break_on_first_statement {
+      if info.wait_for_session {
         eprintln!("Deno is waiting for debugger to connect.");
       }
       if inspector_map.borrow_mut().insert(info.uuid, info).is_some() {
@@ -370,7 +370,7 @@ pub struct InspectorInfo {
   pub new_session_tx: UnboundedSender<InspectorSessionProxy>,
   pub deregister_rx: oneshot::Receiver<()>,
   pub url: String,
-  pub should_break_on_first_statement: bool,
+  pub wait_for_session: bool,
 }
 
 impl InspectorInfo {
@@ -379,7 +379,7 @@ impl InspectorInfo {
     new_session_tx: mpsc::UnboundedSender<InspectorSessionProxy>,
     deregister_rx: oneshot::Receiver<()>,
     url: String,
-    should_break_on_first_statement: bool,
+    wait_for_session: bool,
   ) -> Self {
     Self {
       host,
@@ -388,7 +388,7 @@ impl InspectorInfo {
       new_session_tx,
       deregister_rx,
       url,
-      should_break_on_first_statement,
+      wait_for_session,
     }
   }
 

runtime/worker.rs

@@ -62,6 +62,7 @@ impl ExitCode {
 pub struct MainWorker {
   pub js_runtime: JsRuntime,
   should_break_on_first_statement: bool,
+  should_wait_for_inspector_session: bool,
   exit_code: ExitCode,
 }
 
@@ -81,7 +82,13 @@ pub struct WorkerOptions {
   pub format_js_error_fn: Option<Arc<FormatJsErrorFn>>,
   pub source_map_getter: Option<Box<dyn SourceMapGetter>>,
   pub maybe_inspector_server: Option<Arc<InspectorServer>>,
+  // If true, the worker will wait for inspector session and break on first
+  // statement of user code. Takes higher precedence than
+  // `should_wait_for_inspector_session`.
   pub should_break_on_first_statement: bool,
+  // If true, the worker will wait for inspector session before executing
+  // user code.
+  pub should_wait_for_inspector_session: bool,
   pub get_error_class_fn: Option<GetErrorClassFn>,
   pub cache_storage_dir: Option<std::path::PathBuf>,
   pub origin_storage_dir: Option<std::path::PathBuf>,
@@ -108,6 +115,7 @@ impl Default for WorkerOptions {
       seed: None,
       unsafely_ignore_certificate_errors: Default::default(),
       should_break_on_first_statement: Default::default(),
+      should_wait_for_inspector_session: Default::default(),
       compiled_wasm_module_store: Default::default(),
       shared_array_buffer_store: Default::default(),
       maybe_inspector_server: Default::default(),
@@ -248,7 +256,8 @@ impl MainWorker {
       server.register_inspector(
         main_module.to_string(),
         &mut js_runtime,
-        options.should_break_on_first_statement,
+        options.should_break_on_first_statement
+          || options.should_wait_for_inspector_session,
       );
 
       // Put inspector handle into the op state so we can put a breakpoint when
@@ -261,6 +270,8 @@ impl MainWorker {
     Self {
       js_runtime,
       should_break_on_first_statement: options.should_break_on_first_statement,
+      should_wait_for_inspector_session: options
+        .should_wait_for_inspector_session,
       exit_code,
     }
   }
@@ -355,7 +366,9 @@ impl MainWorker {
         .js_runtime
         .inspector()
         .borrow_mut()
-        .wait_for_session_and_break_on_next_statement()
+        .wait_for_session_and_break_on_next_statement();
+    } else if self.should_wait_for_inspector_session {
+      self.js_runtime.inspector().borrow_mut().wait_for_session();
     }
   }