kafka-protocol-rs icon indicating copy to clipboard operation
kafka-protocol-rs copied to clipboard

Add helper method for decoding the header without having to pass the header version

Open aovestdipaperino opened this issue 4 months ago • 1 comments

I am currently doing this:

            let api_key = ApiKey::try_from(buf.peek_bytes(0..2).get_i16())
                .map_err(|_| anyhow::Error::msg("Unknown API key"))?;
            let api_version = buf.peek_bytes(2..4).get_i16();
            let header_version = api_key.request_header_version(api_version);
            let correlation_id = RequestHeader::decode(&mut buf, header_version)?.correlation_id;
            let request = RequestKind::decode(api_key, &mut buf, api_version)?;

But it feels un-natural to peek from the buffer when the RequestHeader::decode has access to the same information itself. I would think the correct signature should be

RequestHeader::decode(&mut buf) -> Result<>

If there is agreement I can create a simple PR for this.

aovestdipaperino avatar Aug 04 '25 08:08 aovestdipaperino

I created a pull request once I convinced myself that implementing Decodable for RequestHeader was a misfit. While all other "message chunks" being part of the payload require the version that's retrieved from the header, this is definitely not true for the header itself being unique in its purpose. Indeed the buffer::peek was highly suspicious. This little oddity has caused quite a bit of frustration when writing the frame parser due to unintentionally specifying the wrong version. Kudos if it gets approved, version bumped and everything. -e

aovestdipaperino avatar Aug 04 '25 09:08 aovestdipaperino