Skip to content
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

Closed
wants to merge 1 commit into from

Conversation

TimothyGu
Copy link
Member

Useful for seeing how DevTools does something with Node.js.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. dont-land-on-v4.x inspector Issues and PRs related to the V8 inspector protocol labels Jun 22, 2018
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>
@TimothyGu
Copy link
Member Author

@TimothyGu
Copy link
Member Author

Landed in 8836a0d.

@TimothyGu TimothyGu closed this Jun 26, 2018
@TimothyGu TimothyGu deleted the debug-inspector branch June 26, 2018 03:19
TimothyGu added a commit that referenced this pull request Jun 26, 2018
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>
targos pushed a commit that referenced this pull request Jun 26, 2018
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>
@targos targos mentioned this pull request Jul 3, 2018
@addaleax
Copy link
Member

addaleax commented Aug 8, 2018

@TimothyGu I think this may have been undone by 39977db … should we re-apply changes along these lines?

@TimothyGu
Copy link
Member Author

TimothyGu commented Aug 9, 2018

@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.

@addaleax
Copy link
Member

addaleax commented Aug 9, 2018

@eugeneo ^^^

@eugeneo
Copy link
Contributor

eugeneo commented Aug 9, 2018

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 eugeneo self-assigned this Aug 9, 2018
@devsnek
Copy link
Member

devsnek commented Aug 9, 2018

@eugeneo oh i already started working on it... i'm also moving Debug off of env because its application scoped not env scoped.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. inspector Issues and PRs related to the V8 inspector protocol
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants