Repository navigation
inspector: load network tracking modules eagerly - #64379
harjothkhara wants to merge 2 commits into
Conversation
|
Review requested:
|
|
@legendecas mind taking a look or kicking off CI when you have a moment? Open ~4 weeks with no CI run yet — small, tested fix for #64308. |
legendecas
left a comment
There was a problem hiding this comment.
Related to #65028, I think this does not fully fix the issue of calling into JS during an interrupt. An alternative could be loading the tracking module when --experimental-network-inspection is set at startup, and enable it immediately.
|
Thanks @legendecas. You're right: #65028 changes the async hook toggle, but not Your alternative works for me: with I can rework the PR this way, or make |
cef8981 to
39e1c36
Compare
`Network.enable` is dispatched to the main thread through a V8 interrupt that can run in the middle of arbitrary JavaScript execution, including while another module is still being loaded by require(). Enabling network tracking called back into JS from that interrupt to lazily require() the tracking modules, which pull in `inspector`, `worker_threads` and `stream`. When the interrupt landed mid-require(), require() returned a half-initialized module and threw (for example "Class extends value undefined" from internal/worker/io). The inspector agent treats any exception thrown while toggling network tracking as unrecoverable and aborts the whole process. When --experimental-network-inspection is set, load the tracking modules and subscribe their diagnostics channels at startup instead. Network.enable/Network.disable now only flip the flag that gates event emission in NetworkInspector, so they never call into JS from an interrupt. The now unused setupNetworkTracking binding and the agent's enable/disable toggling are removed. Fixes: nodejs#64308 Signed-off-by: harjoth <harjoth.khara@gmail.com> Assisted-by: Claude
With the network tracking channels subscribed at startup, a request created before any session enabled the Network domain was still given an inspector request id, while its Network.requestWillBeSent was dropped natively. If that request finished or failed after Network.enable, the frontend received Network.loadingFinished or Network.loadingFailed for a request it had never seen. Expose whether any session has the Network domain enabled and only assign request ids, and so track a request, while it is. Refs: nodejs#64308 Signed-off-by: harjoth <harjoth.khara@gmail.com> Assisted-by: Claude, Codex
39e1c36 to
a39b402
Compare
|
@legendecas reworked as you suggested: with |
Summary
--experimental-network-inspectioncan abort the whole Node.js process when a debugger attaches. Reported in #64308.Network.enableis delivered to the main thread via a V8 interrupt that can run in the middle of arbitrary JavaScript execution, including while another module is still being loaded byrequire(). Enabling network tracking called back into JS from that interrupt (Agent::ToggleNetworkTracking) to lazilyrequire()the tracking modules, which pull ininspector,worker_threadsandstream. When the interrupt lands mid-require(),require()returns a half-initialized module and throws (for exampleClass extends value undefinedfrominternal/worker/io), and the agent callsUNREACHABLE(), aborting the process.Fix
As suggested by @legendecas: when
--experimental-network-inspectionis set, load the tracking modules and subscribe their diagnostics channels at startup.Network.enable/Network.disablenow only flipNetworkInspector::enabled_, which already gates event emission inNetworkAgent::emitNotification, so nothing calls into JS from the interrupt.The now unused
setupNetworkTrackingbinding, theinspector_{enable,disable}_network_trackingenv properties andAgent::{Enable,Disable,Toggle}NetworkTrackingare removed.Because the channels are now subscribed before any debugger attaches, the handlers check
internalBinding('inspector').isNetworkDomainEnabled()(true if any session hasNetworkenabled) before assigning a request id. Requests started beforeNetwork.enableare not tracked, so the frontend never getsloadingFinished/loadingFailedfor a request it never saw, and the per-request cost while no session is listening is a single native call.Verification
Repro from the issue:
node --inspect-wait --experimental-network-inspection -e "require('net')", then attach and sendNetwork.enable.Driving the real inspector path (attach over WebSocket,
Runtime.runIfWaitingForDebugger, thenNetwork.enablewith 0-15 ms jitter across therequire()window):Tests:
test-inspector-network-tracking-eager-load.js: tracking modules are loaded and their channels subscribed at startup.test-inspector-network-pre-enable-request.js: http, failing http andfetchrequests started beforeNetwork.enableand finished after it emit no events; a request started afterwards gets the full sequence.Fixes: #64308
AI disclosure: parts of this change were drafted with the help of an AI coding assistant. I've reviewed, tested and take responsibility for the result.