uNetworking / uWebSockets.js

μWebSockets for Node.js back-ends :metal:
Apache License 2.0
7.85k stars 569 forks source link

v19 ping #505

Closed Wyzix33 closed 3 years ago

Wyzix33 commented 3 years ago

Hi, I have this setup:

APP = uWS
  .SSLApp({ key_file_name: './privkey.pem', cert_file_name: './fullchain.pem' })
  .ws('/*', {
   compression: uWS.SHARED_COMPRESSOR,
   maxBackpressure: 1 * 1024 * 1024,
   maxPayloadLength: 500 * 1024 * 1024,
   idleTimeout: 30,
   upgrade: (res, req, context) => {
   ...
   }
   open: (ws) => 
   ...
   },
   message: async (ws, message) => {
   ...
   },
   drain: (ws) => console.log('WebSocket backpressure: ' + ws.getBufferedAmount()),
   close: (ws, code) => {
   ...
   })  
  .any('/*', require('./get'))
  .listen(443, (token) => {
   if (token) {
    console.log('Listening to port 443');
   } else {
    console.log('Failed to listen to port 443');
   }
  });
   ...

I used to send a ping every 15 seconds from the client and i see that in v19 the server is supposed to do that, but i get disconnected with

CloseEvent {isTrusted: true, wasClean: false, code: 1006, reason: "", type: "close", …}

every 30 seconds and the client gets not message from the server, i have a function that checks for socket readyState and reconnects it. Can anyone provide an example on how to use this auto ping from the server? and also does the client must respond to it?

Thanks

ghost commented 3 years ago

I can't reproduce this it works for me. Can you provide a minimal reproducer without SSL?

hst-m commented 3 years ago

I install v19 and test some auto ping

require('uWebSockets.js').App().ws('/',{
   idleTimeout: 16,
   open:ws=>console.log(new Date(),'open'),
   close:ws=>console.log(new Date(),'close'),
   pong:ws=>console.log(new Date(),'pong'),
}).listen(3000,()=>{})
ws=new WebSocket('ws://localhost:3000')
ws.onclose=e=>console.log('close')

I don't know what @Wyzix33 issue is, maybe make sure v19 is installed and not running v18

There is issue with idleTimeout value of 16 though it needs fix. For values 8 and 12 it sends ping 4 seconds before timeout, For idleTimeout values 16 and higher it tries to send 16 seconds before timeout, but 16 before 16 is zero, nothing sends and does not close connection. And 16 before 20 is every 4 seconds. Looks like this line is supposed to be margin << 1 https://github.com/uNetworking/uWebSockets/blob/master/src/WebSocketContextData.h#L86

hst-m commented 3 years ago

idleTimeout of 30 seconds rounds down to 28, and 28 is one of the numbers affected by this issue: 16,20,24,28 are supposed to be 8 second margins but code was giving them 16 second margins, possible related to OP issue but maybe not

Wyzix33 commented 3 years ago

you are right, i see in the browser that it reports uWebSockets: 18, but in my package.json i have "uWebSockets.js": "github:uNetworking/uWebSockets.js#v19.0.0" and i deleted node_modules and did a npm install, and it still uses uWebSockets: 18

this is what is inside package.json inside node_modules/uWebSockets.js

{
  "name": "uWebSockets.js",
  "version": "18.14.0",
  "main": "uws.js",
  "types": "./index.d.ts"
}
hst-m commented 3 years ago

Haha yep that is what I figured happened lol

Wyzix33 commented 3 years ago

after a npm uninstall uWebSockets.js --save and npm install uWebSockets.js uWebSockets.js@uNetworking/uWebSockets.js#v19.0.0 --save i see it updated. so the problem is with updating uwebsockets, only new install works, if i set only the package.json deps to

"dependencies": {
    "bcrypt": "^5.0.1",
    "exact-math": "^2.2.3",
    "mongodb": "^4.0.0-beta.1",
    "uWebSockets.js": "github:uNetworking/uWebSockets.js#v19.0.0"
  }

i get 18.14.0 ...

C:\app>ncu
Checking C:\app\package.json
[====================] 4/4 100%

 uWebSockets.js  github:uNetworking/uWebSockets.js#v18.14.0  →  v19.0.0

Run ncu -u to upgrade package.json

C:\app>ncu -u
Upgrading C:\app\package.json
[====================] 4/4 100%

 uWebSockets.js  github:uNetworking/uWebSockets.js#v18.14.0  →  v19.0.0

Run npm install to install new versions.

C:\app>npm i

up to date, audited 74 packages in 1s

6 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

after this the version is still 18.14.0. it only update package.json but not the node_modules,

hst-m commented 3 years ago

Yes this was discussed previously, update issues with npm, that is why it says use yarn instead, I just delete my uws in node_modules and use npm

Wyzix33 commented 3 years ago

also ping works :) Thanks

ghost commented 3 years ago

@hst-m You're right it was changed from this:

        margin *= 2;

To this (wrong):

        margin = (unsigned short) (margin << 2);