Moving `node-inspect` into core

[Trott]

Talked to a handful of @nodejs/ctc members (particularly @ofrobots) as well as @jkrems at Node Interactive today about this. A decision really needs to be made on what to do about the fact that Node.js CLI debugging is going to stop working in the foreseeable future. Unfortunately, there really doesn't seem to be consensus among the sampling of CTC folks I talked to.

Two obvious paths:

• Just remove it. People can use the inspector with a browser or they can install a userland package such as node-inspect

• Bring node-inspect into core. This has been discussed a little bit in the Diagnostics WG (see Bringing node-inspect into the fold diagnostics#67) but I don't think it's really their call. @jkrems, the author and maintainer, has shown how it can be integrated with just a few additional lines of code. See https://github.com/nodejs/node/compare/master...jkrems:jk-node-inspect?expand=1

There are a subtleties and issues. I think it would be best if we make sure that both @ofrobots and @jkrems are at the meeting to fill in gaps, answer questions, etc.

I imagine we'll be canceling the meeting tomorrow, but hopefully we can talk about this at the meeting on Wednesday, December 7, 2016 at 16:00:00 UTC (8AM Pacific Time).

[sam-github]

/cc @bajtos

[bajtos]

@sam-github thank you for pinging me.

Honestly, I don't have much opinions here. At one side, I see the value of a built-in node inspect command that launches a CLI debugger, so that people don't have to rely on userland npm modules. On the other hand, I would not call 1500 lines of code in lib/_inspect.js "just a few additional lines of code" - there will be maintenance costs.

Here is a point that may be a decision-maker: how much activity do we expect in this new inspect component? Assuming the current node-inspect codebase is MVP-like and there is lots of space for improvement (even beyond what node debug provides): If node-inspect becomes a part of Node core, users on older Node versions (e.g. Node v6) will have to wait for new features (and possibly bugfixes too), since new development will be happening in newer Node versions only (v7, v8, etc.). If node-inspect remains a standalone package, then users can pull a newer version while still using their old Node runtime.

Isn't this similar to what we have already learned with (readable) stream(s)?

[sam-github]

I find that pretty compelling - does the debugger have to be in node core? If not, it shouldn't be, and as it communicates with node over a TCP protocol, I don't see why it has to be in core.

[jkrems]

@bajtos Yes, "a few additional lines of code" might be misleading. "Additional" as "in addition to the code that exists in node-inspect", not "additional to the code in node core".

There are definite advantages to all 3 options ("deprecate CLI debugging support", "provide an official standalone CLI debugger", and "provide a built-in CLI debug command in node itself"). Given the changes in the ecosystem since the original CLI debugger was added, "just drop CLI debugging from the feature list" is pretty tempting.

[jasnell]

I certainly agree with @sam-github on this. This could simply be an opportunity to remove that debugger from core in favor of the new --inspect tooling.

[wraithan]

I feel like this dismisses the fact that moving to using the chrome debugger protocol more directly was partially because node-inspector was userland, lagged behind on versions occasionally, etc. Basically making it unreliable for many folks. Now you're casting that on us CLI debugger users. I've dropped out of the node community to stay out of these arguments, but c'mon, you can't just kill CLI debugging completely and leave us with a userland solution that will likely have issues with time and drift of protocols and such.

[ofrobots]

@wraithan there is a qualitative difference between the node-inspector situation and here as there is a proper documented protocol on top of which, not only DevTools, but other tools such as Visual Studio Code, Firefox Debugger, WebStorm and others are built upon. CLI is just one more client.

[MylesBorins]

it is also worth mentioning that if we were to remove the CLI client from core the userland alternative would most likely be a project within the foundation, a very different situation from node-inspector

The idea to separate from core would primarily be to help encourage more maintainers and keep a healthy separation of concerns. It would still be possible for the project to shipped with core, even if it is maintained separately.

[wraithan]

@ofrobots Doesn't solve the problem. Just because there are multiple clients, doesn't mean that client/protocol drift isn't possible. node debug works regardless of version. This new CLI tool does not have that assertion.

Built in means it will always work with new versions, userland means it will update some time after a new version has come out or at least changed. It means keeping installs in tandem or making the client able to handle every protocol difference with time.

Finally the idea that this is required to make tools better is a fallacy at best. replpad got built back in the day, python has pdb built in and ipdb as a userland module. You can have good tools in userland without removing the more stripped down versions that are good to have when things hit the fan.

EDIT: I don't care if it is in the same source repo, I am asserting this needs to be distributed as part of node. If there is an issue for me to argue that without getting into "where the source is stored" debate. I'll go fight my fight there.

[sam-github]

@wraithan you do know that we are required to delete the node core CLI debugger, because the code behind it is being from v8? So, merely having code in core is not enough to say "it will work forever". The question is... do we then replace it wit a new one, or just let the userland CLI debugger continue to exist in userland? Where it was already created? And where (maybe) someone could even do some work to make it work on multiple protocols?

Its not often we have the chance to delete code from core, lets take it.

Btw, I think the comparison to the CLI debuggers many other scripting languages have is interesting, but I read it another way.

ruby/lua/python/perl/tcl/etc. authors all implement debug capability, and then as a kind of "proof it works", they implement one debugger on top, one that involves as little UI work as possible. So, they get a gdb like CLI debugger.

In node's case, v8 has the debug hooks, and it's primary author, google, implement a POC debugger as well - but they do it as a full UI in Chrome. So, Chrome's debugger is the debugger that has total commitment from the js/v8 language authors, and the CLI debugger in node is a historical relic.

[wraithan]

@sam-github I am aware that the current debugger cannot continue to live. I don't think I said "forever", I said the command works for the current version. Presumably the command would be removed, fixed or replaced rather than leaving it in, in a broken state. Right now we're in the "remove" discussion where I'd prefer it got "replaced", even under a new name to cause a new set of expectations.

I agree that those languages use their CLI debugger as more than what node may need it for. But the majority of folks who use those tools, are not using them because they are a proof of concept that the language can be debugged, they are using it to debug their applications and code.

[addaleax]

Right now we're in the "remove" discussion where I'd prefer it got "replaced", even under a new name to cause a new set of expectations.

The top comment in this issue specifically mentions the possible choice of bringing node-inspect into core, and as @thealphanerd mentioned, it should be completely possible to ship it with Node itself.