inspector: add inspector.waitForConnection method#22867
inspector: add inspector.waitForConnection method#22867alexkozy wants to merge 1 commit intonodejs:masterfrom alexkozy:wait-for-connection
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
eugeneo
left a comment
eugeneo
left a comment
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. |
|
|
||
| ## 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.
| 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 |
3 participants
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