Conversation
This was referenced Sep 13, 2026
Open
Contributor
Author
|
@MrAlders0n please review this change with your Claude agent as part of the September 13 Beacon review batch. The current candidate is Please check connection churn, concurrent capacity, close 1013, slot release and existing-client survival. Preserve both this PR and #140 when integrating router/config arguments. Issue #105 is linked for closure on merge. |
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.
What this PR does
Closes #105. Repeated connect/close cycles currently bypass the concurrent WebSocket cap, and its pre-upgrade 429 gives browsers no useful close code.
Add
websocket.max_connects_per_minute(default 10; zero/omitted uses the default; negative values reject configuration). Upgrade attempts, including failed handshakes, consume an in-memory sliding-window budget keyed by the already validated client address. IPv6 clients share a /64 attempt budget. Exhausted budgets return HTTP 429 andRetry-After: 60before upgrade.An attempt within that budget can complete its handshake. If the concurrent application-connection cap is full, close the new socket with 1013 before creating a hub client or sending hello. Established connections stay open. The current web client already uses a 30-second base backoff, with jitter, on 1013. Handshake rejection remains an HTTP response; it does not carry a WebSocket close code.
This PR is independently based on
dev. It shares the exact httprate v0.16.0 pin with #140 without importing that PR's REST changes. Combining them requires preserving both sets of router/config arguments; the tested preview does that. The optional subscription-scope restriction is outside this change.Type of change
Checklist
go build ./...passesgofmt -l .is emptygo vet ./...passesgo test ./...passesSHOULDERS.mdCONTRIBUTING.mdTesting notes
The baseline accepted the eleventh completed connect/close attempt; that regression fails before the change and passes now. Tests cover failed-handshake accounting, recovery using the Go test clock, slot release, simultaneous connects, both caps independently, trusted/untrusted client identity, IPv6 grouping, 1013 before hello and established-client ping/pong after shedding. Focused Windows race checks pass.
The retained preview access sample has no completed WebSocket requests and cannot establish a production reconnect rate. The default follows the issue's suggested value. The web source already tests the 1013 backoff path; no client change is required.
AI tools assisted implementation and validation under the contributor's standing approval for this effort.
Native Pi 5 formatting, build, vet and full Go tests pass for the standalone candidate and the combined preview. The private PostgreSQL recovery regression ran without skipping; the combined build also preserves and passes the #138/#139 database regressions and #140 HTTP tests. No schema change is required.
GitHub Actions CI (including vulnerability and Swagger freshness checks) and CodeQL pass for
cd719be2. The Pi preview serves combined revision9fcb461e. Live synthetic-client checks verified five active connections, five accepted-then-1013 closes before hello, a 429/60-second retry header on attempt 11, spoof resistance, a separate client's access, established-client ping/pong and quota recovery after the windows elapsed. REST reads and ordinary browsing stayed available, and both feeds advanced. Ordinary visitor budgets were not used for the limit probes.