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

Forward early websocket client errors to client future, not the websocket bean #8300

Merged
merged 2 commits into from Nov 23, 2022

Conversation

yawkat
Copy link
Member

@yawkat yawkat commented Nov 7, 2022

When the websocket is closed, but the session has not yet been initialized (and OnOpen not been called), change handleCloseReason to instead emit an error on the publisher that is returned by the WebSocketClient.connect method. This means the publisher won't get stuck, and the OnClose method won't be called without a session being available.

Additionally, I refactored AbstractNettyWebSocketHandler and the subclasses a bit. The bodyArgument/pongArgument code, and the old callOpenMethod, were only used for the server handler.

FYI: there is a merge conflict with 4.0.x, where I did a similar change in channelActive. The code from this PR is correct for 4.0.x as well.

Hopefully fixes #7921

…cket bean

When the websocket is closed, but the session has not yet been initialized (and OnOpen not been called), change handleCloseReason to instead emit an error on the publisher that is returned by the `WebSocketClient.connect` method. This means the publisher won't get stuck, and the OnClose method won't be called without a session being available.

Additionally, I refactored `AbstractNettyWebSocketHandler` and the subclasses a bit. The bodyArgument/pongArgument code, and the old callOpenMethod, were only used for the server handler.

Hopefully fixes #7921
@graemerocher
Copy link
Contributor

once this is merged can you deal with merging into 4.0.x since the location of these classes changed there.

@yawkat
Copy link
Member Author

yawkat commented Nov 7, 2022

how do i do that, just create a PR that merges 3.8.x into 4.0.x?

@graemerocher
Copy link
Contributor

That will do

@sonarcloud
Copy link

sonarcloud bot commented Nov 7, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 4 Code Smells

59.8% 59.8% Coverage
0.0% 0.0% Duplication

@graemerocher graemerocher added this to the 3.8.0 milestone Nov 8, 2022
@yawkat
Copy link
Member Author

yawkat commented Nov 23, 2022

ping @timyates

@yawkat yawkat merged commit e45b5ea into 3.8.x Nov 23, 2022
@yawkat yawkat deleted the close-ws-client branch November 23, 2022 11:10
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.

None yet

3 participants