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

Prevent Websocket send from hanging if connection fails. #280

Merged
merged 1 commit into from
Jan 18, 2023
Merged

Prevent Websocket send from hanging if connection fails. #280

merged 1 commit into from
Jan 18, 2023

Conversation

vincev
Copy link
Contributor

@vincev vincev commented Dec 28, 2022

I have just found out that the following code hangs if no server is running on port 9321:

let ws = WebSocket::open("ws://localhost:9321").unwrap();
let (mut sender, mut receiver) = ws.split();
let _ = sender.send(Message::Text("test".into())).await;

this is because if the server is not running the open_callback is not called and the waker is not waked causing poll_ready to always return Pending.

This PR adds a wake() call on the error_callback so that poll_ready will move to Ready also if an error occurs.

@vincev vincev changed the title Prevent send from hanging if connection fails. Prevent Websocket send from hanging if connection fails. Dec 28, 2022
@vincev
Copy link
Contributor Author

vincev commented Dec 28, 2022

Checked the lint errors they are from another crate not affected by this PR, please let me know if you would like them fixed with this PR.

Copy link
Collaborator

@futursolo futursolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

I have created #287 to fix clippy.

@futursolo futursolo merged commit f97869a into rustwasm:master Jan 18, 2023
@vincev vincev deleted the wake_on_error branch January 18, 2023 13:53
@ranile
Copy link
Collaborator

ranile commented Jan 21, 2023

Released in gloo-net 0.2.6 🎉

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