Repository navigation
Detect a game host that dies silently (proxy TCP keepalive + client idle probe) - #459
Merged
Merged
Conversation
The proxy dialled the game with net.connect/tls.connect and never turned on TCP keepalive, so a peer that vanished without closing (crash, power loss, partition, NAT drop) left the socket half-open until the next write. The browser's WebSocket stayed healthy, nothing disconnected or reconnected, and the player's next command was written into the dead socket and lost. - proxy: dial the game through connectGame() (proxy/gameSocket.ts), which arms keepalive with a 60 s idle delay on both the plain and TLS paths. Node probes 10 times at 1 s, so a dead peer raises ETIMEDOUT about 70 s after the last traffic; the proxy already turns that into a WebSocket close, which the client reports and auto-reconnects from. - client: after 120 s of game silence, MudClient sends IAC NOP (repeating while the silence lasts), only to servers that have spoken telnet. That covers direct ws(s):// endpoints and proxies without keepalive: a peer that came back answers with a reset, one that is gone fails retransmission, and either way the socket closes through the normal disconnect path. The delay is past the bundled proxy's keepalive window so a probe never holds that off by leaving data unacknowledged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015dkC1X3wo4Y9wi9RoCrQpU
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Moving tls.connect({ rejectUnauthorized: false }) into gameSocket.ts made
CodeQL report the existing, deliberate setting (the cert is checked by hand
in certProblems) as a new high alert. Leave the dial where it was and arm
keepalive on the socket instead.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Um3ZSHRbGj1Rbf55rAQzdM
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
3 participants
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.
Fixes #454.
When the game host vanished without closing the connection (crash, power loss, network partition, NAT dropping the connection), the web client never noticed. There was no reconnect, and the next command was lost.
Changes
sysDisconnectionEvent/ auto-reconnect path runs.proxy/gameSocket.ts(connectGame/armGameKeepAlive), used by both dial paths.MudClientsendsIAC NOP, and repeats while the silence lasts.ws(s)://endpoint is never probed.disconnect()and redial.tests/mud/connection/idleProbe.test.tsandtests/proxy/gameKeepAlive.test.ts.Known gaps
worker/index.jsuses the Workersconnect(), which can't set keepalive. Worker users rely only on the client probe. That detects a dead host only after TCP stops retrying, which takes minutes after the first probe.Testing
yarn typecheckpasses, andproxy/type-checks.yarn check:vendoredpasses.yarn test: 2 failures, neither caused by this branch:serverEncoding.test.ts(GBK euro sign) also fails on master.processingOrder384.test.ts(MCCP boundaries) fails only under load. It reproduces with master'sMudClient.tsand passes when run alone.🤖 Generated with Claude Code
https://claude.ai/code/session_01Um3ZSHRbGj1Rbf55rAQzdM
Generated by Claude Code