socketcluster icon indicating copy to clipboard operation
socketcluster copied to clipboard

socketcluster compliance

Open SSANSH opened this issue 1 year ago • 3 comments

I work with a server in version [email protected] (why its not available on tags of repository). And a client on version [email protected]

I have issue with following code :

node_modules/socketcluster-server/serversocket.js

` // Receive incoming raw messages this.socket.on('message', async (messageBuffer, isBinary) => { let message = isBinary ? messageBuffer : messageBuffer.toString(); this.inboundReceivedMessageCount++;

let isPong = message === pongMessage;

`

problem is that isBinary function is not implemented on ws link to the version of the client [email protected] Thats means when pong arrive isBinary is set to false and we cannot validate the condition let isPong = message === pongMessage;

What to you think to change this code by :

node_modules/socketcluster-server/serversocket.js

` // Receive incoming raw messages this.socket.on('message', async (messageBuffer) => { let message = Buffer.from(messageBuffer).toString(); this.inboundReceivedMessageCount++;

let isPong = message === pongMessage;

` This will work on all case with and without isBinary.

or other option is to specify an option to keep the compliance with old client by running this code conditionnaly.

I know the best options is to update server and client, I will do this however this will be more easy to keep compliance during migration.

SSANSH avatar Dec 07 '23 12:12 SSANSH

Thanks for this thorough description. Looking at the suggested change, the functionality appears to be a bit different because it will always convert to a string. Currently, it will sometimes keep the message as a buffer (binary) and not a string. It's important that SC continue to support raw binary (even though that's an unusual use case).

About your problem. Since you are using different versions of the SC client and server, I would expect your package manager (e.g. npm or yarn) to point to two different versions of the ws module for the client and for the server. I would suggest looking into why the client and server are sharing the same version of the ws module/dependency. Normally npm can handle this case without issue. Maybe something to do with bundling step de-duplicating the dependency?

jondubois avatar Dec 07 '23 13:12 jondubois

Thanks too for your quick answer my ws dependency is setup like this.

` ├─┬ [email protected] │ └── [email protected]

├─┬ [email protected] └── [email protected] `

Client and server actualy not share the same version of ws.

note my issue is only on ping pong process.

I understand the raw thing maybe you can restrict the change like this

serversocket.js 
134d133
<     let messagePong = Buffer.from(messageBuffer).toString();
137c136
<     let isPong = messagePong === pongMessage;
---
>     let isPong = message === pongMessage;

in order to identify each time the ping correctly

SSANSH avatar Dec 07 '23 14:12 SSANSH

@SSANSH OK. You could consider either downgrading socketcluster-client to an older version which uses an older ws module or upgrading it. Also, did you set the protocolVersion to 1 on the server? Like this: https://github.com/SocketCluster/socketcluster#compatibility-mode

You need to do this if using an older client with a new version of the server as the new server versions default to protocolVersion 2 which old clients don't fully understand (the difference is related to the ping format so could be related to your issue).

jondubois avatar Dec 08 '23 10:12 jondubois