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

socketcluster compliance #595

Open
SSANSH opened this issue Dec 7, 2023 · 3 comments
Open

socketcluster compliance #595

SSANSH opened this issue Dec 7, 2023 · 3 comments

Comments

@SSANSH
Copy link

SSANSH commented Dec 7, 2023

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.

@jondubois
Copy link
Member

jondubois commented Dec 7, 2023

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?

@SSANSH
Copy link
Author

SSANSH commented Dec 7, 2023

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

@jondubois
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants