Benedicht / BestHTTP-Issues

Issue tracking repo for the Best HTTP and all related Unity plugins.
https://assetstore.unity.com/publishers/4137
11 stars 1 forks source link

Socket.IO questionable null check #198

Closed FlaShG closed 3 months ago

FlaShG commented 3 months ago

I'm not sure whether this is a bug because I'm still figuring out the Socket class's event handling, but...

In TypedEventTable:127, you have this null check:

object[] args = packet.DecodedArg != null ? new object[] { packet.DecodedArg } : packet.DecodedArgs;

I am not sure whether we have faulty states that leads to this, but sometimes, both DecodedArg and DecodedArgs are null. As a result, args is null.

Without understanding too much of the context yet, I feel like this code should look more like this:

object[] args = packet.DecodedArgs ?? new object[] { packet.DecodedArg };

...judging plainly from my assumption that this line is supposed to prevent args from being null, as this leads to an exception further down the line.

Changing the line like this has caused the exceptions to disappear. If there should be some sort of alert when both properties of the packet are null, and the current line is correct as it is, I would like to suggest a more verbose message here.

FlaShG commented 3 months ago

Okay, I have investigated further and found DefaultJsonParser:139-145. So it seems to be intended that args is null when the parser finds no arguments?

It's a bit confusing that DecodedArg only exists to extract the only element of a 1-sized array, and then it gets put back into a 1-size array during Call.

Benedicht commented 3 months ago

Closing as we discussed it on discord (socket.io server was a v2.x one).