Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cli): Support using both --watch and --inspect at the same time #20660

Merged
merged 5 commits into from Oct 6, 2023

Conversation

jespertheend
Copy link
Contributor

Fixes #20525

@jespertheend
Copy link
Contributor Author

I tried deno run --watch --inspect foo.js and both vscode as well as dedicated DevTools for node seemed to automatically reconnect after a change.

But unsurprisingly, connecting to a specific thread using chrome://inspect/#devices didn't automatically reconnect.
In fact, it caused Deno to panic

============================================================
Deno has panicked. This is a bug in Deno. Please report this
at https://github.com/denoland/deno/issues/new.
If you can reliably reproduce this panic, include the
reproduction steps and re-run with the RUST_BACKTRACE=1 env
var set and include the backtrace in your report.

Platform: macos aarch64
Version: 1.37.0
Args: ["./target/debug/deno", "run", "--watch", "--inspect", "inspecttest/foo.js"]

thread '<unnamed>' panicked at 'all branches are disabled and there is no else branch', runtime/inspector_server.rs:339:5

Although I can't seem to reproduce this anymore now.

Another issue might be that if the debugger is paused, making a change doesn't restart the application until the debugger is resumed.

@bartlomieju Are these the pointers you were referring to? Or was this about showing the console message?

@jespertheend
Copy link
Contributor Author

jespertheend commented Sep 24, 2023

I managed to reproduce it again.
I'm still not quite sure what the exact conditions are that triggers this, it seems to happen about 50% of the time.

Full stack trace


============================================================
Deno has panicked. This is a bug in Deno. Please report this
at https://github.com/denoland/deno/issues/new.
If you can reliably reproduce this panic, include the
reproduction steps and re-run with the RUST_BACKTRACE=1 env
var set and include the backtrace in your report.

Platform: macos aarch64
Version: 1.37.0
Args: ["./target/debug/deno", "run", "--watch", "--inspect", "inspecttest/foo.js"]

thread '<unnamed>' panicked at 'all branches are disabled and there is no else branch', runtime/inspector_server.rs:339:5
stack backtrace:
   0:        0x107d9244c - std::backtrace_rs::backtrace::libunwind::trace::h65bc41667b946a9b
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/../../backtrace/src/backtrace/libunwind.rs:93:5
   1:        0x107d9244c - std::backtrace_rs::backtrace::trace_unsynchronized::h11e85e00c64d2951
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:        0x107d9244c - std::sys_common::backtrace::_print_fmt::hd30f3b7be3b8d9bb
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/sys_common/backtrace.rs:65:5
   3:        0x107d9244c - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h56d7672c82815b65
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/sys_common/backtrace.rs:44:22
   4:        0x107db8a08 - core::fmt::rt::Argument::fmt::h6ead82aa10d38e42
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/core/src/fmt/rt.rs:138:9
   5:        0x107db8a08 - core::fmt::write::h632b3cc8e66b4f04
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/core/src/fmt/mod.rs:1094:21
   6:        0x107d8b410 - std::io::Write::write_fmt::hc28441b249a4971d
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/io/mod.rs:1714:15
   7:        0x107d922a4 - std::sys_common::backtrace::_print::h0d2f8b8a08c5cc48
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/sys_common/backtrace.rs:47:5
   8:        0x107d922a4 - std::sys_common::backtrace::print::h8057ced4b0f9fdc1
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/sys_common/backtrace.rs:34:9
   9:        0x107d93fc8 - std::panicking::default_hook::{{closure}}::h8157fa8f0f7934b5
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/panicking.rs:269:22
  10:        0x107d93d58 - std::panicking::default_hook::hdaefe3bfb5fe212c
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/panicking.rs:288:9
  11:        0x100557cb8 - <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call::h89efc30d54daf8ea
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/alloc/src/boxed.rs:2007:9
  12:        0x1010cd934 - deno::setup_panic_hook::{{closure}}::haa503d51ff451c6d
                               at /Users/Jesper/repositories/deno/cli/main.rs:227:5
  13:        0x107d946b8 - <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call::h4efc1b5830705729
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/alloc/src/boxed.rs:2007:9
  14:        0x107d946b8 - std::panicking::rust_panic_with_hook::h21091a3c79c5da9c
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/panicking.rs:709:13
  15:        0x107d94450 - std::panicking::begin_panic_handler::{{closure}}::hd2f65b958d3068b8
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/panicking.rs:595:13
  16:        0x107d9282c - std::sys_common::backtrace::__rust_end_short_backtrace::h53ec33e49ec66621
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/sys_common/backtrace.rs:151:18
  17:        0x107d941f8 - rust_begin_unwind
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/panicking.rs:593:5
  18:        0x107db5bec - core::panicking::panic_fmt::h3bbf9265d206434c
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/core/src/panicking.rs:67:14
  19:        0x102348364 - deno_runtime::inspector_server::pump_websocket_messages::{{closure}}::h5a1b9c2b72c414cf
                               at /Users/Jesper/repositories/deno/runtime/inspector_server.rs:339:5
  20:        0x102346414 - deno_runtime::inspector_server::handle_ws_request::{{closure}}::h1c06c1b310f3b52a
                               at /Users/Jesper/repositories/deno/runtime/inspector_server.rs:185:65
  21:        0x102a8e200 - <deno_unsync::task::MaskFutureAsSend<F> as core::future::future::Future>::poll::hfc77d59c7df9d741
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/deno_unsync-0.3.0/src/task.rs:129:13
  22:        0x1027073f4 - tokio::runtime::task::core::Core<T,S>::poll::{{closure}}::h45d2576fc46e3273
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/task/core.rs:334:17
  23:        0x1026f69a0 - tokio::loom::std::unsafe_cell::UnsafeCell<T>::with_mut::h00230be546f09fdf
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/loom/std/unsafe_cell.rs:16:9
  24:        0x1026f69a0 - tokio::runtime::task::core::Core<T,S>::poll::heeeb83f2f91434bf
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/task/core.rs:323:13
  25:        0x102429300 - tokio::runtime::task::harness::poll_future::{{closure}}::hbe17f90884fa02ed
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/task/harness.rs:485:19
  26:        0x102e7d4ec - <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once::h86a4224f4c62f24d
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/core/src/panic/unwind_safe.rs:271:9
  27:        0x102d259dc - std::panicking::try::do_call::h85133d9660da4122
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/panicking.rs:500:40
  28:        0x102dc4c44 - ___rust_try
  29:        0x102cf09d8 - std::panicking::try::he6ab7ec0dfe75a37
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/panicking.rs:464:19
  30:        0x102fcab90 - std::panic::catch_unwind::h3a01c8288205d650
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/panic.rs:142:14
  31:        0x1023e7520 - tokio::runtime::task::harness::poll_future::ha3453b606e0e2d6b
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/task/harness.rs:473:18
  32:        0x102455154 - tokio::runtime::task::harness::Harness<T,S>::poll_inner::h828325aaedece084
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/task/harness.rs:208:27
  33:        0x1024d7428 - tokio::runtime::task::harness::Harness<T,S>::poll::hc2ad2a4c56b44cbe
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/task/harness.rs:153:15
  34:        0x1025a7048 - tokio::runtime::task::raw::poll::hb5fe9387bba2b5b8
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/task/raw.rs:276:5
  35:        0x10694475c - tokio::runtime::task::raw::RawTask::poll::h503514bcfcdc353f
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/task/raw.rs:200:18
  36:        0x10427b04c - tokio::runtime::task::LocalNotified<S>::run::hdded06df7a4bf5ee
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/task/mod.rs:400:9
  37:        0x102a54178 - tokio::runtime::scheduler::current_thread::CoreGuard::block_on::{{closure}}::{{closure}}::h0e5825dc55d555ce
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/scheduler/current_thread/mod.rs:706:25
  38:        0x102a51ce4 - tokio::runtime::coop::with_budget::h99f30f5ccd7fd44e
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/coop.rs:107:5
  39:        0x102a51ce4 - tokio::runtime::coop::budget::h343019412cdac917
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/coop.rs:73:5
  40:        0x102a51ce4 - tokio::runtime::scheduler::current_thread::Context::run_task::{{closure}}::h3835eddd1110a263
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/scheduler/current_thread/mod.rs:343:43
  41:        0x102a50ba4 - tokio::runtime::scheduler::current_thread::Context::enter::h07edc7c5db323e3c
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/scheduler/current_thread/mod.rs:410:19
  42:        0x102a518b0 - tokio::runtime::scheduler::current_thread::Context::run_task::h74d1284a01f0e698
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/scheduler/current_thread/mod.rs:343:23
  43:        0x102a53118 - tokio::runtime::scheduler::current_thread::CoreGuard::block_on::{{closure}}::h787d0e4378cd819d
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/scheduler/current_thread/mod.rs:705:34
  44:        0x102a527f0 - tokio::runtime::scheduler::current_thread::CoreGuard::enter::{{closure}}::h32f16cd577651602
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/scheduler/current_thread/mod.rs:743:68
  45:        0x102c36fa4 - tokio::runtime::context::scoped::Scoped<T>::set::h6541d00e53500b1a
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/context/scoped.rs:40:9
  46:        0x1028fe798 - tokio::runtime::context::set_scheduler::{{closure}}::h60e0a6b6f5301990
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/context.rs:176:26
  47:        0x103022ef0 - std::thread::local::LocalKey<T>::try_with::h3d430385a37d8aaa
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/thread/local.rs:270:16
  48:        0x10301cd54 - std::thread::local::LocalKey<T>::with::h1c6615a64f155562
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/thread/local.rs:246:9
  49:        0x1028fe714 - tokio::runtime::context::set_scheduler::h80cbb2f3e0ee81eb
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/context.rs:176:9
  50:        0x102a52630 - tokio::runtime::scheduler::current_thread::CoreGuard::enter::he05dfb13df1031e8
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/scheduler/current_thread/mod.rs:743:27
  51:        0x102a5289c - tokio::runtime::scheduler::current_thread::CoreGuard::block_on::h18644154f8eca554
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/scheduler/current_thread/mod.rs:652:19
  52:        0x102a31908 - tokio::runtime::scheduler::current_thread::CurrentThread::block_on::{{closure}}::h18884be3551922a8
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/scheduler/current_thread/mod.rs:175:28
  53:        0x102fe1e7c - tokio::runtime::context::runtime::enter_runtime::hfd620552781ab0e7
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/context/runtime.rs:65:16
  54:        0x102a3176c - tokio::runtime::scheduler::current_thread::CurrentThread::block_on::h1d90379b9887d3dc
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/scheduler/current_thread/mod.rs:167:9
  55:        0x102b101e0 - tokio::runtime::runtime::Runtime::block_on::hf5c17839c28cac77
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/runtime/runtime.rs:347:47
  56:        0x1022582b0 - tokio::task::local::LocalSet::block_on::hb861b576d1190da4
                               at /Users/Jesper/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.32.0/src/task/local.rs:536:9
  57:        0x102345a64 - deno_runtime::inspector_server::InspectorServer::new::{{closure}}::hefed1dc983445a92
                               at /Users/Jesper/repositories/deno/runtime/inspector_server.rs:55:7
  58:        0x10226ac30 - std::sys_common::backtrace::__rust_begin_short_backtrace::ha9e1742ee0ad326e
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/sys_common/backtrace.rs:135:18
  59:        0x102fe6034 - std::thread::Builder::spawn_unchecked_::{{closure}}::{{closure}}::hf95daf79f147cee6
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/thread/mod.rs:529:17
  60:        0x102e8dfec - <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once::hfdc3aab798cd0aa1
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/core/src/panic/unwind_safe.rs:271:9
  61:        0x102d02d30 - std::panicking::try::do_call::h0de8958c295620e0
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/panicking.rs:500:40
  62:        0x102dc4c44 - ___rust_try
  63:        0x102c84da8 - std::panicking::try::h1a30cd72ba0d0cd5
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/panicking.rs:464:19
  64:        0x102fe5b28 - std::panic::catch_unwind::hca50962187abcfc8
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/panic.rs:142:14
  65:        0x102fe5b28 - std::thread::Builder::spawn_unchecked_::{{closure}}::h9988b937636c6e3d
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/thread/mod.rs:528:30
  66:        0x1027a0834 - core::ops::function::FnOnce::call_once{{vtable.shim}}::h861496955bf09408
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/core/src/ops/function.rs:250:5
  67:        0x107d9a3a8 - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::hb85015ba7f8e62e3
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/alloc/src/boxed.rs:1993:9
  68:        0x107d9a3a8 - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::h5923e50a88401120
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/alloc/src/boxed.rs:1993:9
  69:        0x107d9a3a8 - std::sys::unix::thread::Thread::new::thread_start::h8d5cf73423142312
                               at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/sys/unix/thread.rs:108:17
  70:        0x1a0587fa8 - __pthread_joiner_wake
Jesper@Jespers-MacBook-Pro deno % RUST_BACKTRACE=full ./target/debug/deno run --watch --inspect inspecttest/foo.js

@bartlomieju
Copy link
Member

@jespertheend nice work! The panic you are hitting is somewhat expected - before this change, we didn't allow using --inspect and --watch together, as the inspector server assumed that it would be closed down when the process finishes. Now we need to handle closure of in-process WS pipe more gracefully, by shutting down the appropriate task.

I think for starters, you can try to add an "else" branch to the tokio::select! macro in pump_websocket_messages that will break out of the loop and finish the spawned task. Looking at this code, I can say that it hasn't been updated much in the recent months, I might want to budget a couple hours this week to clean it up and refresh to make the code paths more obvious.

@jespertheend
Copy link
Contributor Author

Thanks @bartlomieju! That seems to have done the trick, as I am no longer able to reproduce the panic.

As for the console message, my initial thought was to print the message inside the operation of watch_func, and use cli_options to figure out whether the --inspect flag has been set.
But then I figured maybe there's a way to include a flag the PrintConfig somehow.
Even more ideal would be if it only prints the message when an inspector was actually connected, but I'm not really sure how to go about that.

@bartlomieju
Copy link
Member

@jespertheend sorry I'm misunderstanding something; what message are you referring to?

@jespertheend
Copy link
Contributor Author

@bartlomieju In #20525 a console message was briefly discussed that warns the user that their inspector has been disconnected. Right now the regular "Debugger session ended" is not printed, and even if it were, the console is cleared when watching.

Not displaying a message is fine by me as well. A disconnected inspector is generally what I'd expect when the process restarts.

@bartlomieju
Copy link
Member

@jespertheend maybe let's skip it for now since as you said the screen is cleared anyway.

@jespertheend jespertheend marked this pull request as ready for review September 29, 2023 07:30
Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jespertheend this looks good to me, any chance you could add a test to cli/tests/integration/watcher_tests.rs that showcases this change?

@jespertheend
Copy link
Contributor Author

@bartlomieju I wasn't sure how to write a test that specifically triggers the right conditions for that panic. But I've added a test that at least checks if the debugger restarts when a watched file is changed.

Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, nice work @jespertheend. I'm gonna change the title of the PR to be "fix" as I think it's not really a new feature, just fixing a problem we had before that prevented it from working. Expect it to be released in v1.37.2 next week.

@bartlomieju bartlomieju merged commit be7e2bd into denoland:main Oct 6, 2023
13 checks passed
@jespertheend jespertheend deleted the inspect-and-watch branch October 30, 2023 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow using --watch and --inspect at the same time.
2 participants