Skip to content

Emit a packetParseFailure diagnostic event for malformed packets - #1551

Open
u9g wants to merge 1 commit into
masterfrom
claude/agree-start-an-agent-that-opens-an-nmp-pr-adding-a-packetparsefa-shs6jx
Open

u9g wants to merge 1 commit into
masterfrom
claude/agree-start-an-agent-that-opens-an-nmp-pr-adding-a-packetparsefa-shs6jx

Conversation

@u9g

@u9g u9g commented Oct 27, 2026

Copy link
Copy Markdown
Member

Summary

Adds a packetParseFailure client event so that consumers can diagnose incoming packets that could not be parsed, without reaching into the deserializer.

This is the NMP side of PrismarineJS/mineflayer#4164 ("Dump packets that fail to parse to a local file"). That PR currently monkey-patches client.deserializer.parsePacketBuffer on every state change to catch the three failure kinds; @rom1504 asked in PrismarineJS/mineflayer#4164 (comment) for NMP to own the diagnostic so Mineflayer only consumes it. Once this is released, mineflayer#4164 will be reworked to listen to this event instead.

The event

client.on('packetParseFailure', failure => ...) with:

{
  type: 'ReadError' | 'PartialReadError' | 'SizeMismatch',
  state,       // protocol state the deserializer was built for
  direction,   // 'toClient' on a client, 'toServer' on a server-side client
  buffer,      // the raw (decompressed) packet buffer, including the packet id
  packetId,    // leading varint of buffer, or null
  packetName,  // from this version's protocol data for state/direction, or null
  error,       // the Error for ReadError / PartialReadError, else null
  bytesRead,   // metadata.size for SizeMismatch, else null
  partialData  // parsed data for SizeMismatch, else null
}
  • ReadError: emitted from the existing deserializer error handler, before the existing error event. Existing behaviour is unchanged.
  • PartialReadError: listens to the partialReadError event FullPacketParser emits since FullPacketParser: emit partialReadError with the chunk it could not read ProtoDef-io/node-protodef#178 (protodef 1.20.0, merged but not yet published to npm). Still not turned into an error event.
  • SizeMismatch: in the data handler, when metadata.size !== fullBuffer.length; the packet is then emitted as before.

Nothing is computed unless there is a listener (listenerCount), and the diagnostic is wrapped so it can never throw into the stream pipeline. No stack-frame parsing: error.field / error.stack are passed through as-is.

Also adds the event to src/index.d.ts and documents it in docs/API.md.

Tests

test/packetParseFailureTest.js spins up an NMP server per supported version, connects a client, and has the server writeRaw:

  • the keep_alive id with no body → PartialReadError with state: 'play', direction: 'toClient', packetName: 'keep_alive' and the sent buffer. This case is marked pending until protodef >= 1.20.0 is installed, since 1.19.0 swallows partial reads silently.
  • a valid keep_alive with trailing bytes → SizeMismatch with bytesRead < buffer.length, and the keep_alive packet is still emitted.
  • a well-formed keep_alive → no event.

A ReadError case is not covered: an unknown packet id parses as a 1-byte packet (a SizeMismatch) rather than throwing, so there is no easy raw-bytes way to trigger one.

Local results (protodef 1.19.0, Java 21): lint clean; packetTest, serverTest, cyclePacketTest, compressionTest, pingTest, docTest and the new test pass for 26.1, 1.12.2 and 1.8.8, plus the vanilla-server clientTest for 1.8.8 (26.1's vanilla server needs Java 25). With protodef master installed, the new PartialReadError case passes for all 30 versions.


Generated by Claude Code

Add a client event that reports incoming packets the deserializer could
not parse (ReadError), could only partially read (PartialReadError, via
protodef's partialReadError event) or did not fully consume
(SizeMismatch), with the raw buffer, packet id and name resolved from
the version's protocol data. Nothing is computed unless something
listens for the event, and the diagnostic can never throw into the
stream pipeline. Existing error behaviour is unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VkmDtSAEEP841Htah1acp8
u9g added a commit to u9g/mineflayer that referenced this pull request Oct 28, 2026
Instead of wrapping the deserializer's parsePacketBuffer (and rewrapping
it on every state change), listen to the client's packetParseFailure
event added in PrismarineJS/node-minecraft-protocol#1551, which already
reports ReadError, PartialReadError and SizeMismatch with the packet id,
name and raw buffer. This drops mineflayer's own varint and packet-name
lookup.

The test now covers SizeMismatch (trailing bytes), which works with the
released protodef, and keeps the PartialReadError case, skipped on
protodef < 1.20.0 since only 1.20.0 emits partialReadError.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant