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

Websocketclient disconnected from websocketserver #44

Closed
elC0mpa opened this issue Oct 10, 2019 · 11 comments
Closed

Websocketclient disconnected from websocketserver #44

elC0mpa opened this issue Oct 10, 2019 · 11 comments
Assignees
Labels
possible-bug Something isn't working, and I think it is a bug

Comments

@elC0mpa
Copy link

elC0mpa commented Oct 10, 2019

Describe the bug
Websocketclient is disconnected from websocketserver after client get a ping from server

To Reproduce
Library : 0.4.11
Board : NodeMCU
Steps to reproduce the behavior.

Expected behavior
I just hope the client doesn't disconnect from server

Additional context
When this first happens, I thought it was because of a bug of the library, but i realized that the websocketserver makes a ping to the client when it receives a connection, so I decided to supress this behavior. After doing this, the websocketclient worked as expected, but i have no idea of what am i doing wrong. I suppose that i should modify the code when the client receives a Ping, something like answer with a pong, but i am not sure

@gilmaimon
Copy link
Owner

Steps to reproduce the behavior.

I'm sorry but there isn't enough info for me to reproduce or solve anything here. As you can guess pings should and are working fine for most people who use the library.

What is the server you are using? (software)
ESP debug logs can also be super useful.

@elC0mpa
Copy link
Author

elC0mpa commented Oct 10, 2019

Ok @gilmaimon, I understand you. Later i will post the code. The server I am using is from the Asyncwebserver library. I just want to know something, isn't supposed that each time the websocket receives a ping, it should respond with a pong? In that case, how can i do it, I mean, a pong. Have you think about sending the pong automatically?

@adelin-mcbsoft
Copy link
Contributor

As far as I tested, the library does handle the ping/pong situation. It does reply automatically with a pong every time receives a ping from the remote socket.

The server implementation you are using should reply with a pong to every ping request that is received from the micro-controller as well, if one is sent.

There's nothing that can be done on the library side, as it works accordingly.
See the RFCs here for details about how ping/pong frame is handled.
Perhaps you just have to configure your websocket server accordingly.

@gilmaimon
Copy link
Owner

@adelin-mcbsoft is absolutely right, the library will response with a pong every time a ping is received. The pong content will be the same as the content passed in the ping, as the RFC specifies.

Thanks for helping @adelin-mcbsoft.

The server I am using is from the Asyncwebserver library

Cool, I'll look into it tomorrow. Hopefully I will understand what is the issue (either in my or their impl). If I won't be able to reproduce using a Asyncwebserver server, I will let you know so we can continue to try and find the issue.

@gilmaimon gilmaimon added bug Something isn't working possible-bug Something isn't working, and I think it is a bug and removed bug Something isn't working labels Oct 10, 2019
@elC0mpa
Copy link
Author

elC0mpa commented Oct 11, 2019

You both are entirely right @gilmaimon and @adelin-mcbsoft, the library sends automatically a pong when it receives a ping. But there is a problem, when server receives the pong from the websocketclient wich is written using this library, it crashes. In order to detect the problem i programmed to log everything that is happening through serial. The problem is that, after receiving the pong answer from the client, it receives a binary message : 6e. In order to detect if the problem is related to the server, I did the same but using this library https://github.com/Links2004/arduinoWebSockets. The thing is that in this case, the server doesn't crash. So there should be a difference between your pong implementation and theirs. I hope this could Help you improving your library if i am right, I would like to keep using your library. Thanks in advance

gilmaimon added a commit that referenced this issue Oct 11, 2019
…agmented. This should solve issue #44. Also advanced to patch 0.4.12
@gilmaimon
Copy link
Owner

Hi @elC0mpa,

I think the issue is fixed now. It seems like Asyncwebserver expects the whole websockets frame to come as one chunck, and I separate it for convince. This is not a "client bug" per se, but because changing it won't harm the correctness of the impl and will only increase compatibility with other versions and implementations of websockets - I'm happy to make it.

The changes are in master, can you clone the repo (or pull the changes) and see if the issue is fixed for you as well?

Gil.

@gilmaimon
Copy link
Owner

I understand everything works well? @elC0mpa

@elC0mpa
Copy link
Author

elC0mpa commented Oct 11, 2019

Thanks for your answer @gilmaimon, I really appreciate your Help by changing the pong functionality of this library, I am busy right now but later i will clone the master branch and i will tell you if everything works as expected.

@gilmaimon
Copy link
Owner

Great, I'm happy to help. I'll wait for your updates.

Actually not only pongs crash, any kind of message sent to the server used to crash it. So the change was essentially to how the library sends any kind of websockets message.

Gil.

@elC0mpa
Copy link
Author

elC0mpa commented Oct 12, 2019

It worked as expected @gilmaimon, now the websocketclient doesn't disconnect from server because the server doesn't crash. Thanks for your support, I really appreciate it, and i think this issue has helped to improve the compatibility of the library, so it has been good for you too. Thanks again

@gilmaimon
Copy link
Owner

Great, I'll release it as a patch right now.

I agree, it is a great change and I'm glad you surfaced this issue.

Thanks,
Gil.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
possible-bug Something isn't working, and I think it is a bug
Projects
None yet
Development

No branches or pull requests

3 participants