-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Switchable server side PONG #2186
Comments
No thanks, I don't see how disabling automatic pong replies is useful. In a test environment you can ignore the |
Can you describe how you can make a ws server not pong automatically using the solution you proposed? Client is not necessarily using this ws library nor a tester necessarily has access to the device fw. Furthermore, modifying the client fw does not help validation of the real fw for a production environment. |
You can't (it's a spec requirement), but if you build something that relies on these frames, you can ignore the frames and it would be as if the frame was not sent in the first place. |
Exactly my point. You can't! I am not building anything, I am validating that third party build embedded devices do what they are supposed to do, using their production intented fw that I have no control of. Suggestions to modify the production intended fw on the client is not acceptable, especially on strict environments where even a minor change on the fw as simple as deleting a space character requires re-validation. |
I understand but I don't want to add an option or an env variable for an edge case like this. You can write a dummy server that does not reply to pings for testing or use another WebSocket library that allows you to do that. |
Here is an example: const crypto = require('crypto');
const http = require('http');
const GUID = '258EAFA5-E914-47DA-95CA-C5AB0DC85B11';
const server = http.createServer();
server.on('upgrade', function (req, socket) {
const key = crypto
.createHash('sha1')
.update(req.headers['sec-websocket-key'] + GUID)
.digest('base64');
socket.write(
[
'HTTP/1.1 101 Switching Protocols',
'Upgrade: websocket',
'Connection: Upgrade',
`Sec-WebSocket-Accept: ${key}`,
'\r\n'
].join('\r\n')
);
socket.resume();
});
server.listen(8080); This a "black hole" WebSocket server. It establishes a connection and ignores everything else. |
Thank you for your suggestions anyway, well it's your choice really. Can't use a black hole unfortunately, as I want to trigger the loss on demand in various stages. It's a huge test really that test many ways comms loss. This edge case actually caused thousands of devices in prod going off-line repeatedly over the span of three months, and set fire on vendors trying to fix it. In any case, I'll keep modding your library because it's the best and I've tried them all, believe me 😅 Can't have it all though, can you? |
Another hack that can help you is this websocket._receiver.removeAllListeners('ping');
websocket._receiver.on('ping', function (data) {
websocket.emit('ping', data);
}) In this way pongs are not sent. |
We could even move this line Line 1215 in 527ec97
to a default listener of the |
My thought really is, I appreciate this library and always update my modules to latest, and sometimes install them on demand last moment before use via script. Modding it means I need to keep modding your latest version manually, whereas if there was an official way, I can modify my app to use it and always be up to date. I don't mind the way it will be done, as long as it is consistent throughout the new releases. I've demonstrated how I did it quick and dirty by using env variable but would happily bite the bullet if it's done differently by you. That's why I did this ticket really. To me this lib has it all, and this pong automation is kinda limiting for test/validation use. Some examples of tests I do are: And many more during other tests that I don't remember off the bat right now. |
Using #2186 (comment) works until one day the |
Cheers, I'll give it a go! |
Is there an existing issue for this?
Description
Hello, I know that as per RFC a WS server/client running on this library will automatically pong when a ping is received. While this is great and makes everyone's life easier, sometimes this is not ideal.
A good example is troubleshooting a device/system internet recovery strategy on demand.
As such, I wanted to see what my device will do if a WS ping was not responded and test that my code will try to re-init the LTE modem if it discovers that a certain number of WS pings were not getting a response.
To give some context, it was found that due to some problems the mobile operator randomly is facing, although the connection status remains "connected" and the data channel seems UP, data packets are actually getting lost unless the client disconnects completely from the base station and re-connects.
Although testing this on an ethernet/wifi connected device is easy, just unplug/turn off wifi or the likes, deactivating/re-activating a simcard to simulate network down usually takes time and makes troubleshooting these types of failures a painful time consuming process.
A quick solution was to manually edit the library and add a single if(process.env.BLOCK_PONGS!="on") { on line 1218 of websocket.js and this little hack did the trick for me as I was able to switch auto ponging on an off on demand from my app, or keep it off and handle socket.on('ping' and socket.on ('pong') events and manually .ping() and .pong().
I was thinking that making the automatic pong of this library officially switchable and on by default gives more control to the devs to decide what to do and perhaps chose to manually handle it in the code in some edge cases like mine.
ws version
all
Node.js Version
No response
System
No response
Expected result
No response
Actual result
No response
Attachments
No response
The text was updated successfully, but these errors were encountered: