Skip to content

Commit

Permalink
fix: inspector prompts (denoland#13123)
Browse files Browse the repository at this point in the history
This commit fixes prompts printed to the terminal when
running with "--inspect" or "--inspect-brk" flags.

When debugger disconnects error is no longer printed as
users don't care about the reason debugger did disconnect.

A message suggesting to go to "chrome:https://inspect" is printed
if debugger is active.

Additionally and information that process is waiting for
debugger to connect is printed if running with "--inspect-brk"
flag.
  • Loading branch information
bartlomieju authored Dec 17, 2021
1 parent ca1fbdd commit f3cd9a9
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 12 deletions.
12 changes: 10 additions & 2 deletions cli/tests/integration/inspector_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ async fn inspector_break_on_first_line() {

use TestStep::*;
let test_steps = vec![
StdErr("Visit chrome:https://inspect to connect to the debugger."),
StdErr("Deno is waiting for debugger to connect."),
WsSend(r#"{"id":1,"method":"Runtime.enable"}"#),
WsSend(r#"{"id":2,"method":"Debugger.enable"}"#),
WsRecv(
Expand All @@ -119,10 +121,10 @@ async fn inspector_break_on_first_line() {

for step in test_steps {
match step {
StdErr(s) => assert_eq!(&stderr_lines.next().unwrap(), s),
StdOut(s) => assert_eq!(&stdout_lines.next().unwrap(), s),
WsRecv(s) => assert!(socket_rx.next().await.unwrap().starts_with(s)),
WsSend(s) => socket_tx.send(s.into()).await.unwrap(),
_ => unreachable!(),
}
}

Expand Down Expand Up @@ -276,6 +278,8 @@ async fn inspector_does_not_hang() {

use TestStep::*;
let test_steps = vec![
StdErr("Visit chrome:https://inspect to connect to the debugger."),
StdErr("Deno is waiting for debugger to connect."),
WsSend(r#"{"id":1,"method":"Runtime.enable"}"#),
WsSend(r#"{"id":2,"method":"Debugger.enable"}"#),
WsRecv(
Expand All @@ -293,6 +297,7 @@ async fn inspector_does_not_hang() {

for step in test_steps {
match step {
StdErr(s) => assert_eq!(&stderr_lines.next().unwrap(), s),
WsRecv(s) => assert!(socket_rx.next().await.unwrap().starts_with(s)),
WsSend(s) => socket_tx.send(s.into()).await.unwrap(),
_ => unreachable!(),
Expand Down Expand Up @@ -397,6 +402,7 @@ async fn inspector_runtime_evaluate_does_not_crash() {

use TestStep::*;
let test_steps = vec![
StdErr("Visit chrome:https://inspect to connect to the debugger."),
WsSend(r#"{"id":1,"method":"Runtime.enable"}"#),
WsSend(r#"{"id":2,"method":"Debugger.enable"}"#),
WsRecv(
Expand Down Expand Up @@ -555,6 +561,8 @@ async fn inspector_break_on_first_line_in_test() {

use TestStep::*;
let test_steps = vec![
StdErr("Visit chrome:https://inspect to connect to the debugger."),
StdErr("Deno is waiting for debugger to connect."),
WsSend(r#"{"id":1,"method":"Runtime.enable"}"#),
WsSend(r#"{"id":2,"method":"Debugger.enable"}"#),
WsRecv(
Expand Down Expand Up @@ -583,9 +591,9 @@ async fn inspector_break_on_first_line_in_test() {
"Doesn't contain {}",
s
),
StdErr(s) => assert_eq!(&stderr_lines.next().unwrap(), s),
WsRecv(s) => assert!(socket_rx.next().await.unwrap().starts_with(s)),
WsSend(s) => socket_tx.send(s.into()).await.unwrap(),
_ => unreachable!(),
}
}

Expand Down
27 changes: 19 additions & 8 deletions runtime/inspector_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,18 @@ impl InspectorServer {
&self,
module_url: String,
js_runtime: &mut JsRuntime,
should_break_on_first_statement: bool,
) {
let inspector = js_runtime.inspector();
let session_sender = inspector.get_session_sender();
let deregister_rx = inspector.add_deregister_handler();
// TODO(bartlomieju): simplify
let info =
InspectorInfo::new(self.host, session_sender, deregister_rx, module_url);
let info = InspectorInfo::new(
self.host,
session_sender,
deregister_rx,
module_url,
should_break_on_first_statement,
);
self.register_inspector_tx.unbounded_send(info).unwrap();
}
}
Expand Down Expand Up @@ -229,6 +234,10 @@ async fn server(
"Debugger listening on {}",
info.get_websocket_debugger_url()
);
eprintln!("Visit chrome:https://inspect to connect to the debugger.");
if info.should_break_on_first_statement {
eprintln!("Deno is waiting for debugger to connect.");
}
if inspector_map.borrow_mut().insert(info.uuid, info).is_some() {
panic!("Inspector UUID already in map");
}
Expand Down Expand Up @@ -336,7 +345,7 @@ async fn pump_websocket_messages(
.map_err(|_| ());

let inbound_pump = async move {
let result = websocket_rx
let _result = websocket_rx
.map_ok(|msg| msg.into_data())
.map_err(AnyError::from)
.map_ok(|msg| {
Expand All @@ -345,10 +354,9 @@ async fn pump_websocket_messages(
.try_collect::<()>()
.await;

match result {
Ok(_) => eprintln!("Debugger session ended"),
Err(err) => eprintln!("Debugger session ended: {}.", err),
};
// Users don't care if there was an error coming from debugger,
// just about the fact that debugger did disconnect.
eprintln!("Debugger session ended");

Ok(())
};
Expand All @@ -364,6 +372,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,
}

impl InspectorInfo {
Expand All @@ -372,6 +381,7 @@ impl InspectorInfo {
new_session_tx: mpsc::UnboundedSender<InspectorSessionProxy>,
deregister_rx: oneshot::Receiver<()>,
url: String,
should_break_on_first_statement: bool,
) -> Self {
Self {
host,
Expand All @@ -380,6 +390,7 @@ impl InspectorInfo {
new_session_tx,
deregister_rx,
url,
should_break_on_first_statement,
}
}

Expand Down
6 changes: 5 additions & 1 deletion runtime/web_worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,11 @@ impl WebWorker {
});

if let Some(server) = options.maybe_inspector_server.clone() {
server.register_inspector(main_module.to_string(), &mut js_runtime);
server.register_inspector(
main_module.to_string(),
&mut js_runtime,
false,
);
}

let (internal_handle, external_handle) = {
Expand Down
6 changes: 5 additions & 1 deletion runtime/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,11 @@ impl MainWorker {
});

if let Some(server) = options.maybe_inspector_server.clone() {
server.register_inspector(main_module.to_string(), &mut js_runtime);
server.register_inspector(
main_module.to_string(),
&mut js_runtime,
options.should_break_on_first_statement,
);
}

Self {
Expand Down

0 comments on commit f3cd9a9

Please sign in to comment.