-
-
Notifications
You must be signed in to change notification settings - Fork 55
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
Custom parser support #231
Conversation
fd973ab
to
db42afa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clippy found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
This is a good idea, can you create a corresponding tracking issue for dynamic parsers please :). |
Also can you explain more in detail your issue with message ordering and web socket frames? Maybe provide an example? I don't completely understand your issue and in what way a msgpack parser may solve it. |
This is my question, as well. If message ordering is not sequential (because async), is it not upto the client to re-order them based on different criteria (like timestamp)? |
I'll try to explain the issue I have been experiencing with socketioxide + socket.io-client. When sending two independent packets in a very tiny timeframe, they both arrive pretty much the same time at the server. This seems to be working, all packets arrive at the server and are handled just fine, but my guess is that the ack or any other packets sent back to the client may get messed up in order because they are sent by two parallel-running tokio threads. I'll try to visualize. This is socketioxide: And this is Socket.io: Note that the packets are sent back exactly the same as they were received. (At least I have not had any issues with my previous Socket.io server in this regard) Still, I am just guessing that this causes the problem. I did not yet do extensive research on this problem. The solution with msgpack is also described on the official Socket.io documentation here. This way, it does not really matter in what order the packets are sent and received, as one packet is equal one ethernet frame and neither side needs to wait until all packets have arrived, ultimately solving the underlying issue (although not the preferred way in most cases).
|
Okay so now I understand better. It is due to the fact that socket.io-client expect binary packets to be adjacent to the first string data frame. Therefore, this issue is fixed with msgpack because :
I'll create a tracking issue for this bug. I think it would be better to separate the custom parser support feature from this bug that should not happen even with the default parser. I'll try to propose a solution as simple as possible for this bug during the week. Thanks for your great explanations and schemas :). |
Motivation
I am developing a high performance realtime collaboration server and noticed a problem when sending / receiving multiple packets from and to the server with the default packet parsing implementation. Because of the way socketioxide handles messages asynchronously, websocket frames may get messed up resulting in the wrong order, sometimes crashing the client or the server (although that needs more testing).
Solution
The obvious option would be to fix the underlying issue of ensuring all packets are received in the correct order, but I had to find a quicker and more performant fix to be able to use socketioxide as my primary server implementation. With this PR, I chose to implement custom packet parsing and enabling developers to implement their own protocols or improve packet size / performance by using msgpack.
Open Todos
SocketData::partial_bin_packet
, and rather directly inside the parser.decode_msg
function returns aResult
and thedecode_bin
returns anOption
which is inconvenient.