Record the first unreadable packet to a file instead of logging every one - #38
Open
u9g wants to merge 1 commit into
Open
Record the first unreadable packet to a file instead of logging every one#38u9g wants to merge 1 commit into
u9g wants to merge 1 commit into
Conversation
… one
protodef swallows a PartialReadError inside FullPacketParser._transform:
it prints the stack and drops the packet without emitting anything, so
bot.on('error') never sees it and the raw bytes are never attached. On a
server whose particle registry does not match the client's, that printed
14,814 stacks into one bot's log.
The daemon now wraps each deserializer's parsePacketBuffer, where the
packet buffer is still in hand, and writes the first failure to
bad-packet.json in the working directory: the exact bytes, their sha256,
the packet id and name, the protocol state, and the error. Trailing bytes
after a successful read are recorded the same way, since a payload the
definitions think is shorter than it is fails silently otherwise.
At most one file is written: a process flag stops retries, and wx leaves
an existing file alone across restarts. noErrorLogging is set on the
deserializer instance rather than through hideErrors, which would also
silence mineflayer's own warnings. nmp builds a new deserializer per
protocol state, so the wrap is reapplied on the state event.
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.
On a server whose particle registry does not match the client's (Jartex's lobby sends 1.21.3 ids to a 1.21.4 client, so
item_slimearrives astrailwith no payload), every such packet printed a fullPartialReadErrorstack. One bot logged 14,814 of them, most of a 30 MB log, and none of them carried the bytes.Why the bytes were never available
protodef's
FullPacketParser._transformswallows partial reads:console.log(e.stack), thencb()with nothing emitted and no buffer attached.bot.on('error')never sees these. The only place the packet buffer is still in hand isparsePacketBuffer, so the daemon wraps that on each deserializer.What it does now
The first unreadable packet is written to
bad-packet.jsonin the daemon's working directory, then nothing else is written or printed:{ "bot": { "name", "host", "port", "username", "version" }, "state": "play", "packet": { "id": 42, "name": "world_particles" }, "problem": "partial-read", "length": 51, "sha256": "...", "bytes": "<hex of the whole packet>", "error": { "name": "PartialReadError", "message", "stack" } }problemispartial-readwhen the definitions expect more bytes than arrived, ortrailing-bytes(withread) when they stop short of the end. The second case parses "successfully" and is otherwise invisible; it is the mirror image of the same registry mismatch.At most one file: a process flag stops retries, and the file is opened with
wx, so a restart in the same directory leaves the first one in place.noErrorLoggingis set on the deserializer instance rather than viahideErrors, which would also mute mineflayer's_warn. nmp builds a fresh deserializer per protocol state, so the wrap is reapplied on'state'.Testing
Against the same
minecraft-protocol/protodef@1.19.0/minecraft-data 3.116.0-allprsbuild the affected bot ran, with aworld_particles/trailpacket truncated to the Jartex shape:problem: partial-read,packet: world_particles 0x2a, the exact 51 bytes and matching sha256'state'event: the replacement deserializer is wrappedtrailing-byteswithread: 77of80npx tsc --noEmitreports only the pre-existingcreateHumanerror from this checkout's stalenode_modules.