SP1: Boot WebSocket worker on :8097 with shared SyncPlayManager#338
Conversation
- Add websocket config block to config/server.php with host/port/timeouts - Modify WebSocketServer to accept injected MessageHandler (for SP1 singletons) - Add setSyncPlayManager() method and stale group cleanup timer in onStart() - Add WS worker (count=1, name=phlix-server-ws) in start.php before Worker::runAll() - Applies curated coroutine hooks - Builds container and creates one SyncPlayManager - Initializes manager with MessageHandler for per-type callbacks - Wires WebSocketServer with manager + ConnectionPool/MessageHandler singletons - Keeps stale-connection cleanup; adds cleanupStaleGroups() timer - Add unit tests for WebSocketServer construction and SyncPlayManager init Success conditions: - php start.php start brings up phlix-server-ws worker on :8097 - Raw WS client receives the 'connected' welcome - One SyncPlayManager instance handles all WS connections - HTTP workers (count=14) remain unaffected
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| BestPractice | 3 medium |
| Documentation | 19 minor |
| CodeStyle | 16 minor |
| Complexity | 2 medium |
🟢 Metrics 34 complexity · 0 duplication
Metric Results Complexity 34 Duplication 0
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c48a2f97b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| $wsConfig['stale_group_timeout'] = $wsConfig['stale_group_timeout'] ?? 3600; | ||
|
|
||
| /** @var array<string, mixed> $wsConfig */ | ||
| $wsServer = new \Phlix\Server\WebSocket\WebSocketServer($wsConfig, $messageHandler); |
There was a problem hiding this comment.
When the daemon is started with php start.php start, the worker created at new Worker('websocket://0.0.0.0:8097') is the one Workerman forks and uses to accept client connections. This line constructs a separate WebSocketServer inside onWorkerStart, which creates its own private Worker after Worker::runAll() has already selected the workers, so its onConnect/onMessage callbacks are not installed on the listening $w. As a result, clients connecting to port 8097 will not receive the connected welcome or have messages routed to MessageHandler/SyncPlayManager; bind the callbacks to $w or create the WebSocketServer worker before runAll() instead.
Useful? React with 👍 / 👎.
| // ----------------------------------------------------------------------------- | ||
|
|
||
| try { | ||
| $wsWorker = new Worker('websocket://0.0.0.0:8097'); |
There was a problem hiding this comment.
The new config/server.php block exposes websocket.host and websocket.port, but the actual Workerman listener is still hard-coded to 0.0.0.0:8097. In any deployment that overrides the config, the code below may log/construct the inner server with the configured values while the socket that clients actually reach still binds to all interfaces on 8097, so the configured WebSocket endpoint is ignored. Build this listen URI from $config['websocket'] before creating $wsWorker.
Useful? React with 👍 / 👎.
1 participant
Summary
Implements SP1 of Phase 8 — Boot the WebSocket worker on :8097 with a shared, initialized SyncPlayManager.
Changes
config/server.php: Add
websocketconfig block withhost,port,stale_connection_timeout, andstale_group_timeoutsettingssrc/Server/WebSocket/WebSocketServer.php:
MessageHandlerin constructor (for SP1 singletons)SyncPlayManagerproperty andsetSyncPlayManager()methodonStart()to includecleanupStaleGroups()timer alongside existing stale-connection cleanupstart.php: Add WS worker (
count=1,name='phlix-server-ws') beforeWorker::runAll():SyncPlayManager$manager->initialize($messageHandler)to bind per-type callbacksWebSocketServerto shared manager +MessageHandler/ConnectionPoolsingletonstests/Unit/Server/WebSocket/WebSocketServerTest.php: New test file verifying WS worker construction and SyncPlayManager initialization
Success Conditions (per SP1 spec)
php start.php startbrings upphlix-server-wsworker listening on :8097connectedwelcomeSyncPlayManagerinstance handles all connections (logged once)count=14) remain unaffectedGate
Notes