inspector: add inspector.waitForConnection method #22867
Conversation
This method might be useful in case when we need to do something
with `inspector.url()` before waiting for connection, e.g. save it
to file.
With this method we can do something like:
```
inspector.open();
fs.writeFileSync('abc', inspector.url());
inspector.waitForConnection();
```
nodejs-github-bot
added
c++
|
@eugeneo please take a look. |
alexkozy
added
inspector
There was a problem hiding this comment.
I do not believe this should be a public API (and everthing in inspector module is public API) as it has a high chance of causing deadlocks (it prevents the event loop from pumping both native and V8 events). I just found out it is called for open - I would consider that a bug as well...
I believe this should be asynchronous.
|
Blocking here is exactly what I expect. My use case is:
It is emulation of Alternative might be way to pass to node additional flag next to |
|
Waiting happens before JS begins execution in the case of Is it not possible to report URL before |
|
Is it possible to return full url including port and guid? It will work for my use case. I will try. |
|
IP address and/or port may not be available until the server socket binds if either of them is automatic (i.e. IP is set to 0.0.0.0 or ::0 and/or port is set to 0). |
|
Unfortunately I start node with port 0 and let it detect port by itself. It sounds like I can not get full url until inspector.open is called, I think I have two options: consider adding another --inspector-something flag and if it is set - stored url somewhere, or merge this PR. Wdyt? |
|
Can't you just parse stderr since you seem to have access to the command line already (as you can pass the command line parameter)? |
|
I pass arguments to child processes using NODE_OPTIONS, so parent process might mute or process stderr of child process by itself. At the same time I would like to be able to connect to any node process that started with environment that includes NODE_OPTIONS, in this case I don't have access to stderr stream. |
| @@ -49,6 +49,13 @@ and flow control has been passed to the debugger client. | |||
|
|
|||
| Return the URL of the active inspector, or `undefined` if there is none. | |||
|
|
|||
| ## inspector.waitForConnection() | |||
|
|
|||
| This method blocks until first client has connection, inspector should be open | |||
There was a problem hiding this comment.
My reading of this patch is that this will wait until:
- A connection was made on a WS endpoint.
- Runtime.runIfWaitingForDebugger command was issued
runIfWaitingForDebugger should be mentioned here to reduce confusion
| Environment* env = Environment::GetCurrent(args); | ||
| Agent* agent = env->inspector_agent(); | ||
| if (!agent->IsActive()) { | ||
| env->ThrowError("inspector error, inspector.open should be called first"); |
There was a problem hiding this comment.
Can this check be made from JavaScript code? I believe the guideline was not to throw exceptions from the native code.
| @@ -833,7 +833,8 @@ bool Agent::IsActive() { | |||
|
|
|||
| void Agent::WaitForConnect() { | |||
| CHECK_NOT_NULL(client_); | |||
| client_->waitForFrontend(); | |||
| if (!client_->hasConnectedSessions()) | |||
There was a problem hiding this comment.
I am not sure this change is needed. If I understand correctly, the goal here is to wait for a specific frontend to connect. This check will subvert this expectation.
|
Unfortunately I lost original branch, so I open another PR and addressed all your comments there, please take a look: #28453 |
This method might be useful in case when we need to do something
with
inspector.url()before waiting for connection, e.g. save itto file.
With this method we can do something like:
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes