feat: add onRequest method to WebSocket adapters #27
Conversation
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant Adapter
participant HTTP Handler
Client->>Server: HTTP request
Server->>Adapter: onRequest(callback)
Adapter->>HTTP Handler: Invoke callback with request/response
HTTP Handler-->>Adapter: Processed response
Adapter-->>Server: Return self
Server-->>Client: HTTP response
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/servers/Workerman/server.php (1)
46-68: Well-implemented HTTP request handler with proper endpoint structure.The onRequest handler demonstrates excellent usage of the new HTTP request handling capability:
✅ Strengths:
- Proper HTTP response format with correct headers and status codes
- Useful test endpoints (
/healthand/info) for monitoring- Clean path-based routing logic
- Appropriate use of
Connection: closeheader for stateless HTTP requests- Integration with WebSocket server's connection management via
$server->getConnections()The implementation correctly follows the adapter pattern and provides valuable functionality for testing and monitoring the WebSocket server.
Optional enhancement for robustness:
->onRequest(function (TcpConnection $connection, Request $request) use ($server) { echo 'HTTP request received: ', $request->path(), PHP_EOL; + try { if ($request->path() === '/health') { $connection->send('HTTP/1.1 200 OK' . "\r\n" . 'Content-Type: application/json' . "\r\n" . 'Connection: close' . "\r\n\r\n" . json_encode(['status' => 'ok', 'message' => 'WebSocket server is running'])); } elseif ($request->path() === '/info') { $connection->send('HTTP/1.1 200 OK' . "\r\n" . 'Content-Type: application/json' . "\r\n" . 'Connection: close' . "\r\n\r\n" . json_encode([ 'server' => 'Workerman WebSocket', 'connections' => count($server->getConnections()), 'timestamp' => time() ])); } else { $connection->send('HTTP/1.1 404 Not Found' . "\r\n" . 'Connection: close' . "\r\n\r\n" . 'Not Found'); } + } catch (Throwable $e) { + echo 'Error handling HTTP request: ', $e->getMessage(), PHP_EOL; + $connection->send('HTTP/1.1 500 Internal Server Error' . "\r\n" . + 'Connection: close' . "\r\n\r\n" . + 'Internal Server Error'); + } })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/WebSocket/Adapter.php(1 hunks)src/WebSocket/Adapter/Swoole.php(2 hunks)src/WebSocket/Adapter/Workerman.php(1 hunks)src/WebSocket/Server.php(1 hunks)tests/servers/Swoole/server.php(2 hunks)tests/servers/Workerman/server.php(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/WebSocket/Adapter.php (3)
src/WebSocket/Server.php (1)
onRequest(204-215)src/WebSocket/Adapter/Workerman.php (1)
onRequest(107-112)src/WebSocket/Adapter/Swoole.php (1)
onRequest(129-136)
src/WebSocket/Server.php (3)
src/WebSocket/Adapter.php (1)
onRequest(90-90)src/WebSocket/Adapter/Workerman.php (1)
onRequest(107-112)src/WebSocket/Adapter/Swoole.php (1)
onRequest(129-136)
src/WebSocket/Adapter/Workerman.php (3)
src/WebSocket/Adapter.php (1)
onRequest(90-90)src/WebSocket/Server.php (1)
onRequest(204-215)src/WebSocket/Adapter/Swoole.php (1)
onRequest(129-136)
tests/servers/Workerman/server.php (3)
src/WebSocket/Adapter/Workerman.php (4)
Workerman(9-140)onRequest(107-112)send(41-46)getConnections(136-139)src/WebSocket/Adapter.php (3)
onRequest(90-90)send(40-40)getConnections(131-131)src/WebSocket/Server.php (3)
onRequest(204-215)send(62-71)getConnections(222-225)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Unit & E2E (8.0)
- GitHub Check: Unit & E2E (8.2)
- GitHub Check: Unit & E2E (8.3)
- GitHub Check: Unit & E2E (8.1)
🔇 Additional comments (7)
src/WebSocket/Adapter.php (1)
85-90: LGTM! Abstract method follows established patterns.The abstract
onRequestmethod declaration is consistent with other event handler methods in the class and properly defines the interface contract for concrete adapters.src/WebSocket/Adapter/Swoole.php (2)
6-6: LGTM! Import needed for Response type hints.The
Swoole\Http\Responseimport is required for the newonRequestmethod implementation.
129-136: LGTM! Implementation follows Swoole patterns.The
onRequestmethod implementation is consistent with Swoole's event handling pattern and other methods in the class. The callback wrapper properly provides Request and Response objects to the user callback.src/WebSocket/Server.php (1)
199-215: LGTM! Consistent with existing event handler pattern.The
onRequestmethod implementation follows the exact same pattern as other event handler methods in the class with proper error handling and method chaining.tests/servers/Swoole/server.php (2)
6-6: LGTM! Import needed for Response type hints.The
Swoole\Http\Responseimport is required for the new HTTP request handler.
46-65: LGTM! Excellent example of HTTP request handling.The HTTP request handler provides good examples of different response scenarios and nicely integrates with the WebSocket server by showing connection counts in the
/infoendpoint.tests/servers/Workerman/server.php (1)
6-7: LGTM! Required imports for HTTP request handling.The new imports are necessary for the onRequest handler implementation and follow the correct Workerman namespace conventions.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/WebSocket/Adapter/Workerman.php (1)
111-117: Signature inconsistency issue remains unresolved.The callback signature inconsistency identified in the previous review still exists. The Workerman adapter calls the callback with
(TcpConnection, Request)while the Swoole adapter uses(Request, Response). This breaks the abstraction and makes it difficult to write adapter-agnostic code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/WebSocket/Adapter/Workerman.php(4 hunks)tests/servers/Workerman/server.php(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/servers/Workerman/server.php
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/WebSocket/Adapter/Workerman.php (3)
src/WebSocket/Adapter.php (2)
onRequest(90-90)onMessage(83-83)src/WebSocket/Server.php (2)
onRequest(204-215)onMessage(168-179)src/WebSocket/Adapter/Swoole.php (2)
onRequest(129-136)onMessage(109-116)
🔇 Additional comments (3)
src/WebSocket/Adapter/Workerman.php (3)
8-8: Import looks good.The addition of the HTTP Request import is appropriate for the new HTTP request handling functionality.
20-23: Good encapsulation of callback storage.The new private properties properly encapsulate the callback storage, following good OOP principles.
96-97: Consistent refactoring pattern.The modification to store the callback and delegate to
setupMessageHandler()maintains consistency with the new unified message handling approach.
There was a problem hiding this comment.
Pull Request Overview
Adds HTTP request handling to the WebSocket server API and adapters, and updates test servers to support /health and /info endpoints.
- Introduce
onRequestmethod inServerandAdapterinterface - Implement HTTP request routing in both Workerman and Swoole adapters
- Update test server scripts to verify new HTTP endpoints
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/servers/Workerman/server.php | Add HTTP /health and /info handlers in Workerman |
| tests/servers/Swoole/server.php | Add HTTP /health and /info handlers in Swoole |
| src/WebSocket/Server.php | Add onRequest to server API and error handling |
| src/WebSocket/Adapter.php | Extend interface with abstract onRequest |
| src/WebSocket/Adapter/Workerman.php | Implement onRequest and unify handler setup |
| src/WebSocket/Adapter/Swoole.php | Implement onRequest for Swoole adapter |
Comments suppressed due to low confidence (1)
src/WebSocket/Server.php:208
- The
catch (Throwable $error)reference isn’t fully qualified or imported in this namespace. Usecatch (\Throwable $error)or adduse Throwable;at the top to avoid a runtime error.
} catch (Throwable $error) {
Summary by CodeRabbit
New Features
/healthfor server status and/infofor server metadata.Tests