Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Propagate handshake failures to Handshake future #3058

Merged
merged 1 commit into from
Nov 30, 2024

Conversation

mp911de
Copy link
Collaborator

@mp911de mp911de commented Nov 21, 2024

We now complete the handshake future exceptionally if handshake settings do not match our expectations. This can happen if e.g. the connection id is being sent as String instead of an integer.

See also:

We now complete the handshake future exceptionally if handshake settings do not match our expectations.
This can happen if e.g. the connection id is being sent as String instead of an integer.
@tishun
Copy link
Collaborator

tishun commented Nov 30, 2024

Hey @mp911de , do you think we should backport this or is it enough to have it in 6.6 and 6.5.1 ?

@tishun tishun added this to the 6.5.1.RELEASE milestone Nov 30, 2024
@tishun tishun merged commit 26fbdbb into redis:main Nov 30, 2024
5 checks passed
tishun pushed a commit that referenced this pull request Dec 1, 2024
We now complete the handshake future exceptionally if handshake settings do not match our expectations.
This can happen if e.g. the connection id is being sent as String instead of an integer.
@mp911de mp911de deleted the handshake-failure-propagation branch December 2, 2024 07:29
@mp911de
Copy link
Collaborator Author

mp911de commented Dec 2, 2024

Guess the next minor is fine. It's not a problem with Redis, only with other implementations that deviate from Redis' behavior.

@tishun
Copy link
Collaborator

tishun commented Dec 2, 2024

Guess the next minor is fine. It's not a problem with Redis, only with other implementations that deviate from Redis' behavior.

Sounds fair, thanks!

(Already released with 6.5.1.RELEASE)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants