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

Bug: vitest run --coverage throws Not implemented: inspector.Session.prototype.connect #25004

Closed
yasaichi opened this issue Aug 12, 2024 · 9 comments · Fixed by #26471
Closed
Labels
bug Something isn't working correctly node compat

Comments

@yasaichi
Copy link

yasaichi commented Aug 12, 2024

Version: Deno 1.45.5

I ran into the following error (panic) when running DENO_FUTURE=1 deno run -A npm:vitest run --coverage:

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.45.5
Args: ["/Users/yuichigoto/.local/share/mise/installs/deno/1.45.5/bin/deno", "run", "-A", "npm:vitest", "run", "--coverage"]

thread 'main' panicked at cli/module_loader.rs:616:7:
internal error: entered unreachable code: Deno bug. node:inspector was misconfigured internally.
stack backtrace:
   0: _rust_begin_unwind
   1: core::panicking::panic_fmt
   2: deno::module_loader::CliModuleLoaderInner<TGraphContainer>::load_prepared_module_or_defer_emit
   3: <deno::module_loader::CliModuleLoader<TGraphContainer> as deno_core::modules::loaders::ModuleLoader>::load::{{closure}}
   4: deno_core::modules::recursive_load::RecursiveModuleLoad::register_and_recurse_inner::{{closure}}
   5: <deno_core::modules::recursive_load::RecursiveModuleLoad as futures_core::stream::Stream>::poll_next
   6: deno_core::modules::map::ModuleMap::poll_progress
   7: deno_core::runtime::jsruntime::JsRuntime::poll_event_loop
   8: deno_core::runtime::jsruntime::JsRuntime::run_event_loop::{{closure}}
   9: deno_runtime::worker::MainWorker::run_event_loop::{{closure}}
  10: deno::worker::CliMainWorker::run::{{closure}}
  11: deno::tools::run::run_script::{{closure}}
  12: deno::spawn_subcommand::{{closure}}
  13: <deno_unsync::tokio::task::MaskFutureAsSend<F> as core::future::future::Future>::poll
  14: tokio::runtime::task::raw::poll
  15: deno::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

This bug always happens when trying to measure test coverage with @vitest/coverage-v8. Here is the repo to reproduce it. Thanks you in advance!
https://github.com/yasaichi-sandbox/deno-vitest-cov

NOTE:
@vitest/coverage-istanbul is another way to measure test coverage and works well with Deno. One cons is that it seems slower than using v8 because of an effort for instrumentation.

@marvinhagemeister marvinhagemeister added bug Something isn't working correctly node compat labels Aug 12, 2024
@marvinhagemeister
Copy link
Contributor

Looks like this is caused by node:inspector not being well supported in Deno at the moment.

@yasaichi
Copy link
Author

Related to #23882

marvinhagemeister added a commit that referenced this issue Aug 14, 2024
For some reason we didn't register the `node:inspector` module, which
lead to a panic when trying to import it. This PR registers it.

Related: #25004
@yasaichi
Copy link
Author

yasaichi commented Aug 14, 2024

I am feeling sorry that all I can do is report bugs. Unfortunately, I ran into the following new error while trying to measure test coverage in the repo with the HEAD of Deno.

Error: Not implemented: inspector.Session.prototype.constructor
 ❯ notImplemented ext:deno_node/_utils.ts:38:9
 ❯ new Session node:inspector:22:5
 ❯ node_modules/.pnpm/@vitest+coverage-v8@2.0.5_vitest@2.0.5/node_modules/@vitest/coverage-v8/dist/index.js:4:17

It means that thanks to @marvinhagemeister, the bug I originally reported was resolved.

@marvinhagemeister
Copy link
Contributor

Yeah the whole node:inspector code is basically just stubs at the moment. We need to add code to actually attach and create an inspector.

@yasaichi
Copy link
Author

@marvinhagemeister
Thank you so much for your hard work around this topic. BTW as for this issue, feel free to close it.

@yasaichi
Copy link
Author

yasaichi commented Aug 15, 2024

Due to security implications the Deno team does not plan to polyfill these APIs.
https://docs.deno.com/runtime/manual/node/compatibility/

Is the statement about node:inspector still true even now? If so, should I think that I will never be able to measure test coverage in any nodejs-based testing frameworks relying on the module in the future?

@marvinhagemeister
Copy link
Contributor

This statement isn't true. We're currently in the process of updating our documentation in preparation for Deno 2.

@kt3k
Copy link
Member

kt3k commented Sep 10, 2024

The error now looks like the below (Updated the title):

Error: Not implemented: inspector.Session.prototype.connect
 ❯ notImplemented ext:deno_node/_utils.ts:9:9
 ❯ Session.connect node:inspector:12:5
 ❯ startCoverage node_modules/.deno/@vitest+coverage-v8@2.0.5/node_modules/@vitest/coverage-v8/dist/index.js:6:11
 ❯ Object.startCoverage node_modules/.deno/@vitest+coverage-v8@2.0.5/node_modules/@vitest/coverage-v8/dist/index.js:44:12
 ❯ startCoverageInsideWorker node_modules/.deno/vitest@2.0.5/node_modules/vitest/dist/chunks/coverage.CqfT4xaf.js:47:42
 ❯ run node_modules/.deno/vitest@2.0.5/node_modules/vitest/dist/chunks/runBaseTests.CyvqmuC9.js:105:3
 ❯ runBaseTests node_modules/.deno/vitest@2.0.5/node_modules/vitest/dist/chunks/base.CC5R_kgU.js:31:3
 ❯ ForksBaseWorker.executeTests node_modules/.deno/vitest@2.0.5/node_modules/vitest/dist/workers/forks.js:25:7
 ❯ execute node_modules/.deno/vitest@2.0.5/node_modules/vitest/dist/worker.js:115:5
 ❯ onMessage node_modules/.deno/tinypool@1.0.1/node_modules/tinypool/dist/entry/process.js:55:20

@kt3k kt3k changed the title Bug: vitest run --coverage crashes with panic Bug: vitest run --coverage throws with Not implemented: inspector.Session.prototype.connect Sep 10, 2024
@kt3k kt3k changed the title Bug: vitest run --coverage throws with Not implemented: inspector.Session.prototype.connect Bug: vitest run --coverage throws Not implemented: inspector.Session.prototype.connect Sep 10, 2024
@kt3k
Copy link
Member

kt3k commented Sep 10, 2024

#25198 might solve this issue

@devsnek devsnek closed this as completed in 700f54a Nov 6, 2024
littledivy pushed a commit that referenced this issue Nov 10, 2024
implement local inspector

future changes:
- wire up InspectorServer to enable open/close/url
- wire up connectToMainThread

Fixes #25004
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly node compat
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants