FullPacketParser: emit partialReadError with the chunk it could not read - #178
Open
u9g wants to merge 1 commit into
Open
FullPacketParser: emit partialReadError with the chunk it could not read#178u9g wants to merge 1 commit into
u9g wants to merge 1 commit into
Conversation
A chunk the definitions cannot read is dropped, and the only trace was console.log(e.stack), which noErrorLogging removes. The bytes themselves were never available to the consumer. Attach the chunk as error.buffer and emit it as 'partialReadError' before the log. It is not emitted as 'error', which would end the stream; the drop-and-continue behaviour is unchanged, and Parser is untouched since a partial read there means the packet is not complete yet.
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.
FullPacketParser._transformdrops a chunk it cannot read and moves on, which is the right behaviour for a stream of whole packets. But it discards the chunk on the way: the only trace isconsole.log(e.stack), and even that goes away undernoErrorLogging. A consumer cannot find out which bytes failed to parse.That matters when a server's registry disagrees with the client's definitions. A 1.21.3 server sending particle id 47 (
item_slime, no payload) to a 1.21.4 client, whose definitions read 47 astrail(avec3f64and more), throwsPartialReadErroron every particle packet. One bot logged 14,814 stacks, none of them carrying the packet.This attaches the chunk to the error as
error.bufferand emits it as'partialReadError'before the existing log, so a consumer can record the bytes, diff them against the schema, or count them. The event is not'error': emitting that on a Transform ends the stream, which is exactly what the drop-and-continue path exists to avoid.Nothing else changes. The log line and the drop are as before, and
noErrorLoggingstill controls only the log.Parseris untouched: a partial read there means the rest of the packet has not arrived yet, not that the bytes are wrong.The plain
Parseralready setse.bufferon the errors it forwards, so the field name follows it.Typed in
index.d.tsand documented indoc/api.md, which had noFullPacketParsersection. Test covers the interpreted and compiled parsers: the event fires once with the chunk, and the next packet still parses.