fix: sync response header version to clamped request header#3223
Merged
fix: sync response header version to clamped request header#3223
Conversation
Smallish refactor changing the internal structure of responsePromise: instead of storing a (potentially stale) response header version, it now stores a pointer to the response's protocolBody (if any). This allows sendInternal to easily update the response's Version after version negotiation. (request and response versions must match; header versions instead don't map 1-1, meaning we can't rely on the request's header version for decoding responses). Signed-off-by: Giorgio Pellero <[email protected]>
Collaborator
|
@trapped historically in the functional tests we pin the Sarama config version to match the remote Kafka version we are testing against, but it might be worth updating that here to just always set MaxVersion and UseApiVersions and that should shake out this sort of issue too (where Sarama supports a newer version than the remote) |
trapped
force-pushed
the
fix/sync-response-header-ver
branch
from
e48a683 to
80eff2d
Compare
Contributor
Author
|
@dnwe thanks for the suggestion, that makes a lot of sense. I've just pushed a new commit with that change. Let's see if CI breaks. |
dnwe
force-pushed
the
fix/sync-response-header-ver
branch
3 times, most recently
from
454b4e5 to
8f1f8de
Compare
dnwe
changed the title
dnwe
force-pushed
the
fix/sync-response-header-ver
branch
from
5532671 to
38ed256
Compare
Collaborator
|
Cool, that testing eventually worked with apiversions by default. I've taken those commits off again so we can merge this PR with the single focus and then I'll raise a separate PR to move to "enable apiversions by default" as a followup |
dnwe
approved these changes
Aug 3, 2025
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
2 participants
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.
Smallish refactor changing the internal structure of
responsePromise: instead of storing a (potentially stale) response header version, it now stores a pointer to the response'sprotocolBody(if any).This allows
sendInternalto easily update the response'sVersionafter version negotiation. (request and response versions must match; header versions instead don't map 1-1, meaning we can't rely on the request's header version for decoding responses).Fixes the issue from this comment.