WIP: src,inspector: add --inspect-silent option

[cjihrig]

Allow the inspector to be used without printing anything to the console. This is useful when working with tools that know how to use the inspector.

If this is OK, I'll add the tests and docs.

Fixes: #12665

R= @eugeneo

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

Affected core subsystem(s)

[jasnell]

Definitely a fan of this.

[Fishrock123]

What's the difference between this and sending a SIGUSR1 signal?

[Fishrock123]

Also, I'd personally prefer NODE_NO_INSPECTOR_HINT, if it is still necessary to implement.

[cjihrig]

A flag can automatically be a boolean, but also allows specifying the port and host. Is that currently an option with an env. variable?

[segrey]

Introducing a new cli option --inspect-silent will make tools more dependent on node version detection. Now, if version detection fails, there are two options to choose from: pass --debug-brk or --inspect --debug-brk. Since these options represent old and new debugging protocol, it's intuitive to have a boolean setting like "Prefer inspector protocol" that is taken into account when node version detection fails. However, introducing the third option (--inspect-silent) would make this setting a bit more complex.
BTW, just curious, will there be --inspect-silent-brk?

[cjihrig]

If you want to turn off the hint, we're going to have to add something new, unless there is already an existing solution. If it is only an environment variable, then you're either going to have to do version detection or continue assuming that the hint output is present.

It's also worth noting that the inspector is still an experimental feature and subject to change. It should stabilize with Node 8. Additionally, in Node 8 there will be no "prefer inspector protocol" since the old debugger is going away.

[segrey]

@cjihrig An environment variable could be boolean and the port and host will still be specified as previously.
With an environment variable, a tool can just always set it without getting "bad option" or changing protocol, i.e. in a backward compatible way.

[cjihrig]

The "bad option" point is correct - although if you're already doing version detection it's a very simple workaround. The flag in this PR can already be used as a Boolean. If your goal is to be able to silence the hint in your tool, you're still going to have to do version detection though, unless you always assume the hint is there.

[refack]

How about a more general -q --quiet nothing to stdout?

[refack]

Re: version detection / protocol selection. IMHO it's a solved problem for all major tools.

[refack]

[bikeshed] why not --debug-silent, since essentially there is no --debug anymore...

[jasnell]

@cjihrig ... just a random thought... if the inspector hint were emitted via the process.emitWarning() mechanism instead, then the existing mechanisms for suppressing warning output would work for this also.

[cjihrig]

@jasnell that's not a bad idea. The inspector hint isn't really a warning though. I foresee people showing up that want one or the other (warnings and no hint, or more likely, hint and no warnings).

[jasnell]

I've been stewing over the idea of allowing --no-warnings and the environment variable to accept a parameter listing the warnings to suppress. e.g. --no-warnings=DeprecationWarning,InspectHint, etc.

[sam-github]

@jasnell was about to post similar. We don't need a new env var (because NODE_OPTIONS), I wonder if --no-warnings could left, but a new CLI --no-messages=warnings (because not all messages are "warnings").

I'm also think its just fine saying "the inspector hint is a warning that the inspector is running" and treating it like a warning because it is. That works pretty well with warning categories as you suggest. Also allows deprecations to be unified with warnings, perhaps?

[jasnell]

A NODE_NO_WARNINGS env var was already added previously. The NODE_OPTIONS var could definitely be used.

Deprecations are already unified with warnings. They use process.emitWarning() under the covers.

[segrey]

@cjihrig I stand corrected - a new flag won't make tools more critically dependent on node version detection, because if version detection fails (unlucky case), a tool just won't suppress hint text which is not a big problem.
Sorry guys, looks like there is a problem with an environment variable. Since by default it's inherited by child processes, IIUC child processes won't emit this hint text line:

Debugger listening on port 48235.

It makes impossible for a tool to debug multi-process node apps, because the tool won't be notified that a child process's debugger is ready for attaching.

[jasnell]

@segrey ... if the debugger hints are evolved to use process.emitWarning(), then the child emissions would also be accessible via process.on('warning') events, allowing tools to tap into those.

[segrey]

@jasnell Yeah, but not sure that Java can listen for process.on('warning') events emitted by spawned node process.

[jasnell]

@segrey ... no, but a preload module can be used to hook into those events to provide custom handling.

[segrey]

@jasnell Will master process receive warnings from child processes? It doesn't happen in 7.9.0. Not sure, but a preload module seems like a complication to hide inspect hint text. Probably, it'd be easier/safer to continue analyzing stderr and update the logic on hint text change... Sorry for the noise.

[jasnell]

No, but it can be made to forward those. Another recently added option is --redirect-warnings which allows sending all warnings to a file rather than to stderr.

[segrey]

@jasnell Seems forwarding warnings to child.on('warning') (where child is a ChildProcess object) wouldn't be enough as a preload module won't have access to the child object. Looks like child warnings should go to master's process.on('warning') by default.
Thanks, didn't know about --redirect-warnings, IMO reading a standard stream is a bit more convenient than reading a file - no need to care about its removal after process termination.