-
Notifications
You must be signed in to change notification settings - Fork 3
inspector: Add WebSocket support for Chrome DevTools Protocol integration #106
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
Conversation
Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
yorkie
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Remove the build files which should not be committed
- Follow the clang-format style in your C/C++ files
- Does the SSL necessary?
- Add a sample to connect & send/recv from the inspector websocket
Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
All feedback addressed in commits a47862e and 79eaa34:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move the inspector example to the fixtures/inspector-client
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved inspector examples to fixtures/inspector-client/ directory as requested in commit af2033b.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use 2 spaces
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed HTML indentation to use 2 spaces instead of 4 spaces in commit af2033b.
| for (const auto &client : clients_) | ||
| { | ||
| if (client->isWebSocket()) | ||
| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| { | |
| count++; |
src/runtime/inspector_server.hpp
Outdated
| public: | ||
| void tryAccept(); | ||
| void tick(); | ||
| int getWebSocketConnectionCount() const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| int getWebSocketConnectionCount() const; | |
| int coutWebSocketClients() const; |
src/runtime/inspector_client.cpp
Outdated
|
|
||
| for (const auto &header : headers_) | ||
| { | ||
| std::string lowerKey = header.first; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto
src/runtime/inspector_client.cpp
Outdated
| for (const auto &header : headers_) | ||
| { | ||
| std::string lowerKey = header.first; | ||
| std::transform(lowerKey.begin(), lowerKey.end(), lowerKey.begin(), ::tolower); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto
src/runtime/inspector_client.cpp
Outdated
| DEBUG(LOG_TAG_INSPECTOR, "WebSocket connection limit reached, rejecting upgrade"); | ||
| // Send 503 Service Unavailable | ||
| std::stringstream response; | ||
| response << "HTTP/1.1 503 Service Unavailable\r\n"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can add a method for sending 400 or 500 response, which auto calucate the content-length.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added sendHttpErrorResponse() helper method that auto-calculates Content-Length header in commit af2033b.
src/runtime/inspector_client.cpp
Outdated
| auto hash = sha1(combined); | ||
|
|
||
| // Base64 encode the hash | ||
| auto base64Encode = [](const std::array<uint8_t, 20> &input) -> std::string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move the base64 encoding to the src/common/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved base64 encoding to src/common/utility.hpp as Base64Encode template function in commit af2033b.
src/runtime/inspector_client.cpp
Outdated
| DEBUG(LOG_TAG_INSPECTOR, "Received WebSocket message: %s", message.c_str()); | ||
|
|
||
| // For now, just echo the message back (placeholder for CDP implementation) | ||
| sendWebSocketMessage("Echo: " + message); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be moved to TrInspector such as a lifetime onMessage(client), and you can does this echo logic there :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved echo logic to TrInspector::onMessage(client, message) method - the client now forwards WebSocket messages to the inspector for handling in commit af2033b.
…tility methods Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
…nlikely attribute, remove debug log Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
This PR implements WebSocket support in the JSAR Inspector, enabling real-time debugging communication as the foundation for Chrome DevTools Protocol (CDP) integration.
Changes Made
Core WebSocket Implementation
Connection Limiting
getWebSocketConnectionCount()method to monitor active connectionsEnhanced Inspector Architecture
Backward Compatibility
/json/*endpoints continue to work unchangedUsage Example
Testing
Technical Details
Upgrade: websocket,Connection: Upgrade,Sec-WebSocket-Key)This implementation provides the WebSocket transport layer needed for CDP, with the next phase being the replacement of echo functionality with actual CDP message routing and domain implementations.
Fixes #105.
Warning
Firewall rules blocked me from connecting to one or more addresses
I tried to connect to the following addresses, but was blocked by firewall rules:
registry.npmmirror.comnpm install(dns block)If you need me to access, download, or install something from one of these locations, you can either:
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.