socketioxide icon indicating copy to clipboard operation
socketioxide copied to clipboard

Custom parser support

Open FabianHummel opened this issue 1 year ago • 5 comments

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

  • [ ] Have a parser instance per each client / socket so that collecting binary packets is easier and is not required to do via SocketData::partial_bin_packet, and rather directly inside the parser.
  • [ ] Cleaner API, currently the decode_msg function returns a Result and the decode_bin returns an Option which is inconvenient.
  • [ ] More freedom with encoding and decoding packets, a lot of logic is still hardcoded into the client struct.
  • [ ] Unit tests. I commented them out because I had issues with getting them work.

FabianHummel avatar Jan 16 '24 10:01 FabianHummel

This is a good idea, can you create a corresponding tracking issue for dynamic parsers please :).

Totodore avatar Jan 16 '24 14:01 Totodore

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.

Totodore avatar Jan 16 '24 14:01 Totodore

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)?

tausifcreates avatar Jan 16 '24 14:01 tausifcreates

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:

image

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) image

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).

Pros: packets with binary content are sent as one single WebSocket frame (if the WebSocket connection is established)

FabianHummel avatar Jan 16 '24 19:01 FabianHummel

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 :

packets with binary content are sent as one single WebSocket frame (if the WebSocket connection is established).

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 :).

Totodore avatar Jan 16 '24 19:01 Totodore