-
Notifications
You must be signed in to change notification settings - Fork 189
feat: Enhance WebSocket transport with advanced features #964
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
base: main
Are you sure you want to change the base?
feat: Enhance WebSocket transport with advanced features #964
Conversation
- Introduced WebsocketListenerConfig for configurable listener settings including TLS, connection limits, and timeouts. - Implemented WebSocketConnectionManager to manage multiple connections with pooling, cleanup, and statistics tracking. - Added SOCKSConnectionManager for WebSocket connections through SOCKS proxies, supporting SOCKS4 and SOCKS5 protocols. - Refactored WebsocketTransport to utilize new configuration and connection management features, including proxy support and improved error handling. - Enhanced connection tracking and statistics gathering for better monitoring of active connections. - Updated listener and transport methods to support new configurations and improve overall robustness.
@acul71 I would like you to take the point on this PR I'm occupied next week, so I may not be able to put more work into it. I've raised the PR and implemented the WebSocket implementation. |
@yashksaini-coder : Great, thank you so much for sharing. Appreciate your wonderful efforts. @acul71 will collaborate with you to get this PR to a production stage. We need this PR ready for final review + merge before the next py-libp2p release in 2+ weeks. @yashksaini-coder : Wish to share that I re-ran the CI/CD pipeline and found important issues and tests failing. Please collaborate with @acul71 and @bomanaps to get this resolved. |
…multiple demo files
@yashksaini-coder : Thank you so much to you and @acul71 for your efforts. Appreciate it. It took me sometime to reach this PR as there were close to 12+ PRs in the review queue. Did re-run the CI/CD pipeline and important test cases are passing. Did notice some good number of test cases are failing too. Wish if you could collaborate with @acul71 and arrive at a good conclusion on this PR soon. Lets try and complete this before the next release of py-libp2p planned in 1.5+ weeks. |
…pdate assertions in tests
…t for better compatibility
…ini-coder/py-libp2p into Feat/WebSocket-Transport
PR Review: #964 - Enhance WebSocket transport with advanced featuresReviewer: acul71 Executive SummaryThis PR attempts to enhance the WebSocket transport implementation with advanced features including connection management, SOCKS proxy support, and improved configuration options. However, the implementation has several critical issues that need to be addressed before it can be merged. Current State AnalysisExisting WebSocket Transport ImplementationThe current WebSocket transport in py-libp2p includes:
Missing Features (Issue #938)Based on the release notes and issue #938, the missing features include:
PR Analysis✅ Strengths
✅ Implementation Quality
|
@asmit27rai Thanks for the commit. Is this summary correct? Summary: Python Server vs JavaScript Server✅ JavaScript Server
❌ Python Server
The ProblemThe interop tests have a protocol mismatch:
What Actually Works
########################## I'll show you the actual working sections with real logs. Let me run the tests that actually work and capture their output: [1 tool called] 1. JavaScript Server Working (HTTP-based)[3 tools called] 2. Python Client → JavaScript Server Communication[4 tools called] 3. Bidirectional Test (Most Comprehensive)[2 tools called] 4. JavaScript Client Test (Shows the Limitation)[2 tools called] 📋 Summary: What Actually Works - Real Logs✅ 1. JavaScript Server (HTTP-based)
Status: ✅ WORKING - Responds to HTTP POST requests with echo ✅ 2. Python Client → JavaScript Server
Status: ✅ WORKING - Python makes HTTP requests to JS server ✅ 3. Bidirectional Test (Most Comprehensive)
Status: ✅ WORKING - 4 successful HTTP connections, multiple message exchanges ❌ 4. JavaScript Client → Python Server (The Limitation)
Status: ❌ FAILING - No HTTP-based Python server available 🔍 Key Insights from the Logs
The logs clearly show that 3 out of 4 test scenarios work perfectly, with the only limitation being the missing HTTP-based Python server for the JavaScript → Python direction. |
@acul71 Let Me Fix It. |
@acul71 Please have a look. ./run_all_tests.sh For all tests result at once. |
I've run it and it is working well. Next, can you restructure the tests so they are properly kept in the official The interoperability testing success is a major achievement. Really appreciate this @asmit27rai 👍🏼 |
@yashksaini-coder and @asmit27rai : Great efforts. Appreciate the contribution. Wish to share that I re-run the CI/CD pipeline. There are some issues. Wish if you could fix them. Reviewing the PR in details. Will share feedback soon. |
Fixes: #938
Added WebSocket Transport Improvements
This PR improves the WebSocket transport implementation with better connection management,
configuration options, and monitoring capabilities.