Skip to content

Latest commit

 

History

History
172 lines (105 loc) · 7.24 KB

2017-03-23.md

File metadata and controls

172 lines (105 loc) · 7.24 KB
Raw

Diag WG Meeting - March 2017

Attendees

  • Thomas Watson @watson
  • Richard Lau @richardlau
  • Zbyszek Tenerowicz @naugtur
  • Jan Krems @jkrems
  • Matt Loring @matthewloring
  • Ali Sheikh @ofrobots
  • Eugene Ostroukhov @eugeneo
  • Josh Gavant @joshgav

Today’s Agenda

Previous Meeting Review

Minutes

inspector: make debug an alias for inspect node#11441

  • Switch the CLI debugger to V8 inspector node#11421
  • debug: activate inspector with _debugProcess node#11431

To discuss:

  1. Alias node debug script.js to node inspect script.js.
  2. Alias --debug to --inspect.
  3. Deprecate vm.runInDebugContext API.
  4. Switch SIGUSR1 to activate Inspector.

Need to decide what to change now before 8.0.0.

1. Alias node debug script.js to node inspect script.js.

Conclusion: We should alias node debug to node inspect and include a deprecation warning.

2. Alias --debug to --inspect.

@joshgav: Does this depend on Node@8 including V8 5.7 or 5.8?

@jkrems: Even if Node@8 starts with V8 5.7 we should alias --debug to --inspect. That way we won’t have to support the old interface through the lifetime of Node@8.

@joshgav: Are we concerned about breaking ecosystem? Concerns about node-inspector?

@ofrobots: People have moved on to other tools.

@joshgav: Any reason to not alias --debug and instead remove entirely?

@jkrems: If end goal is to encourage --inspect might be best to just completely remove, reduce confusion. But might be unexpected to current users.

@ofrobots: Best to keep it around as an alias for a little while.

@joshgav: Propose a) making it an alias to --inspect, b) with a runtime deprecation warning, and c) plan to remove it in a later version entirely.

@jkrems: Any objections? A: No.

3. Deprecate vm.runInDebugContext API.

vm.runInDebugContext will go away at end of 2017, so we should deprecate now in preparation for Node 10 (April 2018).

@ofrobots: Are there other use cases for this? Most common use case is var debug = vm.runInDebugContext(‘Debug’) to access debugging methods.

@joshgav: open a PR for the deprecation and see what turns up?

4. Switch SIGUSR1 to activate Inspector.

Based on above, this needs to land in 8.0.0. Need reviews of node#11431.

Next steps

  • @ofrobots to update #11441 to alias --debug to --inspect and node debug to node inspect, to land in 8.0.0.
  • @joshgav to submit PR to deprecate vm.runInDebugContext.
  • Please review node#11431 - switch SIGUSR1.

[WIP] inspector: hint text update #11207

@joshgav: Suggestion:

Debugger listening at ws://127.0.0.1:port/uuid
For more info go to https://nodejs.org/en/docs/guides/debugging-getting-started

@jkrems: Prefer just port as previously with --debug. Tools know how to handle changing UUIDs, so better not to surface UUID, would be confusing.

@joshgav: IP address can be important, for example when attaching to process in a Docker container, which doesn’t necessarily allow access to localhost. Also UUIDs are needed for lower-level tool users.

What about the chrome-devtools URL (e.g. chrome-devtools://devtools/bundled/inspector.html?experiments=true&v8only=true&ws=127.0.0.1:9229/e027329e-7b98-4b7d-85f0-184a0ea24b74)? Should that continue to be included?

@joshgav: Could Chrome DevTools accept the short ws URL and interpolate into the full chrome-devtools URL? That would also solve some of the feedback about the URL being too long to copy out of the terminal.

@ofrobots: May be security issues, chrome-devtools scheme has special rights.

@jkrems: Possible to add different IPs and ports in chrome://inspect now, but would need to reconfigure for all available combinations.

@joshgav: Are there strong opinions that we should keep the chrome-devtools URL?

Next steps

  • @joshgav to open PR with suggestion above and continue discussion there.

What will Domain be replaced with? #10843

Remove from diag-agenda, perhaps close the issue.

[trace] tracking issue #84

@matthewloring: Macros we added from V8 fell out of date with V8 so they don’t work. Will update them shortly, and would be good to also add an actual use and test case to core to know if they fall out of date.

Propose 2 PRs:

  1. Update macros to get back in sync with V8.
  2. Add a couple actual instrumentation points in core.

No objections.

[async_hooks] tracking issue #29

New PR: nodejs/node#11883.

Q&A

Schedule? Okay to follow the last TSC meeting each month? No objections.