test: detect and reconnect when the server stops reading the bot socket - #4018
Closed
u9g wants to merge 4 commits into
Closed
test: detect and reconnect when the server stops reading the bot socket#4018u9g wants to merge 4 commits into
u9g wants to merge 4 commits into
Conversation
u9g
force-pushed
the
test/login-stall-detector
branch
from
August 27, 2026 06:30
e3edf99 to
15925d1
Compare
Member
|
CI fails |
u9g
force-pushed
the
test/login-stall-detector
branch
from
August 29, 2026 14:11
15925d1 to
993e937
Compare
Member
|
Ok I don't understand the one, will come back to it |
rom1504
reviewed
Aug 30, 2026
| // commands still reach that bot, so this echo is the only proof of life. | ||
| async function serverReads (username, timeoutMs = 3000) { | ||
| const deadline = Date.now() + timeoutMs | ||
| while (Date.now() < deadline) { |
Member
There was a problem hiding this comment.
I don't like this active wait loop. Can we use an event instead?
u9g
force-pushed
the
test/login-stall-detector
branch
from
September 4, 2026 18:18
1e21ccb to
332d031
Compare
The vanilla server (1.7 through at least 1.21.8) can lose netty read interest on a connection during the login burst: Connection.sendPacket calls setAutoRead(false) on the main thread for every PLAY packet sent while the channel still says LOGIN and queues setAutoRead(true) on the IO thread. Netty only re-arms OP_READ on a 0->1 transition but drops it unconditionally on the queued clearReadPending, so with two or more doSendPacket tasks pending when the main thread flips the flag the socket ends up with autoRead=true and OP_READ off. Nothing in the PLAY state ever calls read() again; the server reads nothing from the bot until ReadTimeoutHandler(30) kills the connection. Reproduced locally at ~1% of logins under CPU load (10% with Connection DEBUG logging on). Console commands still reach the bot, so `op` succeeds and the whole version then fails with "Event move did not fire" from resetState after the 30s kick. The server echoing our skinParts (127) in our own entity_metadata is its first reaction to anything the bot sent; it lands ~160ms after login (max 1.45s under load) and is never sent on a stalled connection. Wait for it once `op` has succeeded and reconnect if it never comes.
…socket The stalls hit in CI were not flatbot's: the inventory example child on 1.17.1 and spawnbot on 1.19.3 each logged in, never had a byte read, and burned a 90s mocha timeout before the server dropped them. The before-hook detector only guards flatbot. Poll the joined bot's entity_metadata for the skinParts echo (127) that the server only sends after reading that socket. runExample kills a stalled child and relaunches it, which makes the server kick the dead session; spawnEvent ends and recreates spawnbot. Three attempts each.
spawnbot logs in at the world spawn, which on <=1.17 superflat worlds is hundreds of blocks from flatbot, so flatbot never sees its entity and the metadata gate reported a stall on every attempt. Teleporting it into view first is not reliable either: the 1.8 entity tracker did not surface the teleported player within the window. A chat line from spawnbot reaching flatbot needs neither and is only broadcast once its socket was read.
u9g
force-pushed
the
test/login-stall-detector
branch
from
September 4, 2026 21:11
332d031 to
39a136a
Compare
Contributor
|
Tests more than 1.5x slower than master (durations are noisy, so this is informational): |
Member
Author
|
Let’s just close this for now |
Member
Author
|
I’m not saying it’s fixed, but if it becomes serious we can use this fix, as of now it’s a rare situation |
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.
CI occasionally fails a whole version with 40+
Event move did not fireerrors right after thebeforehook (e.g. 1.19.3 in https://github.com/PrismarineJS/mineflayer/actions/runs/33027267332). The server log for those runs shows zero[flatbot: ...]command feedback andflatbot lost connection: Timed outexactly 30s after login.Root cause (vanilla server ≤1.20.1, not mineflayer):
Connection.sendPacketcallschannel.config().setAutoRead(false)on the main thread for each PLAY packet sent while the channel attr still says LOGIN (the wholeplaceNewPlayerburst), and queuesdoSendPacket → setProtocol → setAutoRead(true)on the netty IO thread. Netty re-armsOP_READonly on a 0→1 transition of the autoRead flag, but the queuedclearReadPendingdrops it unconditionally. With ≥2doSendPackettasks pending when the main thread flips the flag, the channel ends up autoRead=true withOP_READoff, nothing in PLAY ever callsread()again, andReadTimeoutHandler(30)kills the connection. Reproduced locally at ~1% of logins under CPU load (10% withConnectionDEBUG logging); the bot's bytes sit unread in the server's kernel Recv-Q. 1.20.2+ moved the protocol switch onto the IO thread (MC-265209), so only ≤1.20.1 is affected. Nothing the client sends can re-arm the read, so the only fix is to detect it and reconnect.Detection: console commands still reach the bot, so
opsucceeds. The server's first reaction to anything the bot sent is echoing itssettingsskinParts (127) in that bot's ownentity_metadata, ~160ms after login (max 1.45s under load), never on a stalled connection.Where it's applied — every bot the suite creates, since the stalls actually hit in CI were not flatbot's (the
inventoryexample child on 1.17.1,spawnboton 1.19.3), each burning a 90s mocha timeout:beforehook: afteropsucceeds, require flatbot's own echo (3s grace), otherwisedeop, end the bot and log in again.bot.test.serverReads(username)intestCommon.jspolls the joined bot's entity metadata for the echo.runExampleuses it after the child has been teleported into view; a stalled child is killed and relaunched under the same name, which makes the server kick the dead session.spawnEvent:spawnbotlogs in at the world spawn, which on ≤1.17 superflat worlds is hundreds of blocks from flatbot, so its entity (and metadata) is never in view. It sends a chat line instead, which the server only broadcasts once it has read that socket; if flatbot doesn't get it within 3s the bot is ended and recreated. Three attempts each.Zero cost on healthy logins (the echo normally lands before the
opmessage / the child's arrival the tests already wait on).Validation
settingsdeliberately dropped on the first login, the hook reconnected and the suite passed 72/72.exampleBee,exampleInventoryon 1.16.5 and 1.21.8: pass, no gate trips.spawnEventon 1.8.8, 1.16.5, 1.19.3, 1.21.8 (spawnbot logging in 100–400 blocks away on the first two): pass, single login each.spawnbotchat suppressed on attempt 1 → gate tripped at 3s, attempt 2 passed;runExamplewith a forced stall on attempt 1 → child killed, relaunched,exampleInventorypassed.