-
Notifications
You must be signed in to change notification settings - Fork 29.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
inspector: add debugging for WebSocket messages #21473
Conversation
PR-URL: nodejs#21473 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: James M Snell <jasnell@gmail.com>
77f9e8d
to
f940c7a
Compare
Landed in 8836a0d. |
PR-URL: #21473 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #21473 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: James M Snell <jasnell@gmail.com>
@TimothyGu I think this may have been undone by 39977db … should we re-apply changes along these lines? |
@addaleax 😞yeah I think this PR could be super helpful for folks trying to develop new Inspector-based front ends. Unfortunately I won't have time to get to it. |
@eugeneo ^^^ |
This would have to be reimplemented... I was unable to keep the functionality when I was doing the refactor (I believe there was something about debug being invoked on a wrong thread) - so this would have to be reimplemented. The problem on the main thread had been there was no UTF8 message but UTF16 wrapped into v8_inspector::StringBuffer. |
@eugeneo oh i already started working on it... i'm also moving Debug off of env because its application scoped not env scoped. |
Useful for seeing how DevTools does something with Node.js.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes