inspector: introduced --inspect-store option

[alexkozy]

This option takes folder name as argument. When it is passed,
inspector will dump content of /json/list endpoint to file with
random name inside of passed folder, inspector will try to cleanup
this file as well at finish but without strict guarantees.

This option is useful when we need to have node targets discovery
with child processes or without child processes. Current solution is
setting --inspect-brk=0 using environment to force each node process
to open inspector socket and listenning for stderr for started
process. This approach has a lot of flows, e.g. it requires stderr
parsing, it does not help with discovering node processes which are
started independently.

With new flag if we need to discover and debug node processes
using inspector:

1. create folder ahead of time,
2. pass this folder with new --inspect-store flag with
   --inspect-brk=0 or --inspect=0 using environment,
3. listen to files created inside this folder using any external
   tool, e.g. another nodes script with fs.watch.
4. as soon as new file created - try to connect,
5. be ready for stale store files,
6. remove store folder later.

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

[nodejs-github-bot]

Lite-CI: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/3459

[alexkozy]

This pull request is not ready to be merged - I will add test and update docs after initial review.
@addaleax and @eugeneo please take a look! What do you think about this approach?

[eugeneo]

My concern is that this is a non-trivial change for something that seems like a fairly narrow use-case. I see quite a few potential problems, e.g. files are never cleaned up, that folder becomes cluttered over time, as the application is restarted it becomes hard to find a file for a specific start, running out of disk space for long-running clusters, etc.

I think this should be doable by having a userland code store the inspector.url() to a file. My understanding is that it does not work in case --inspect-brk is specified as userland code is ran. So, maybe we should look into it?

Is this only an issue with fork/clusters? Maybe we could have a custom inspector port allocator for subprocess ports? It could either provide a predictable port number or communicate the port number to the frontend (e.g. list the port in /json/list)
Another option I see is introducing a primitive for waiting for "Runtime.runIfWaitingForDebugger" so the user code could implement a custom inspect-brk that could write the URL before pausing.

I think we should not proceed straight code. Maybe this should be discussed through the diag WG?

[alexkozy]

Someone who would like to use this flag is responsible to cleanup folder which it uses. E.g., as ndb I would like to maintain a folder where every started node will put its file, as soon as ndb finished it will remove temporary folder or someone who uses flag can just use temporary directory and get it cleanup on restart.

User land does not work to me since any code there pollute environment. When I'm building tool that measure code coverage I do not want to see that someone loaded fs module and this someone is external tool that do it to save data to file.

It is the problem every time when I'd like to have a way to discover debuggable node instances. Custom port allocated will require tool to ping multiple ports all the time, it is cpu overhead that we can easily avoid.

[june07]

https://github.com/june07/NiMS-vscode-extension/blob/master/README.md

I ran into this issue and my solution albeit VSCode specific was this. Another solution would be to again leverage userland code (ie an agent) to provides similar functionality, basically tracking node process (not ports) and associated metadata provided by --inspect.

[alexkozy]

I will close it for now. If for some reason in a future we would go this way - we always can reopen it.