fix(websocket): time out stalled sends and reset failed connections - #240
Merged
Conversation
Contributor
Contributor
Author
|
@greptileai Please re-review the current commit with the source-backed explanation in #240 (comment) and the test changes described in #240 (comment). The previous run started at 12:05:42 UTC, before those replies were posted. The remaining descriptor-reuse warning assumes the PHP API receives a raw OS descriptor. In Swoole it receives a session ID, captured by value in this closure. |
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Swoole can leave
push()coroutines suspended after a slow WebSocket client disconnects, retaining their messages indefinitely (swoole/swoole-src#6196). The adapter also ignores failed pushes, so a connected client can miss events without being disconnected.Set a five-second default
send_timeout, configured once with the Swoole adapter’ssendTimeoutconstructor parameter (new Swoole(sendTimeout: 2.0)). The parameter accepts finite positive seconds; there is no timeout setter. Keep yielding enabled so brief stalls can recover. If a push fails and the connection still exists, reset it immediately to discard queued output; a graceful close can runonClosewhile retaining the undrained socket. Clients must reconnect and refresh application state after a failed send.This is the timeout-based alternative to #225, following the Swoole maintainer's workaround. It leaves the existing output buffer size unchanged. The timeout applies to each wait, and a retry can start another wait; it is not an absolute send deadline or a cap on pending bytes. Large bursts still require application backpressure or fewer updates. Appwrite will need to consume the released library to receive this behavior.
Validation
bin/monorepo check websocket: Pint, PHPStan, and Rector pass.bin/monorepo test websocket: 9 unit tests and 5 end-to-end tests pass on macOS/PHP 8.5.9/Swoole 6.2.2, including Workerman coverage.pcntl, so its Workerman fixture was not run.bin/monorepo validate, Vale, andgit diff --checkpass.