On the ping interval handling

[delvedor]

Hi! :wave:

Is the ping interval handling mandatory? Use setInterval is not a very good idea, does not scale well and maybe the users do not need it.

Maybe expose a setKeepAlive function and let the user decide how handle this?

[gj]

Hey @delvedor !

Good point on the scaling re: a bunch of server-side setInterval()s. How about removing the existing code and adding an example / instructions on how to set up a client-side keep-alive ping? The spec cautions against initiating pings from the client side "to aid the server," but keeping the connection alive strikes me as a joint client- and server-side concern.

[delvedor]

I'd like to listen also the opinion of @mcollina here :)

[mcollina]

IMHO pings should not be sent. If a connection dies for inactivity, then the client will reconnect. It is unsafe to assume that a connection will never drop.

[gj]

Removed the keepAlive logic: https://github.com/gj/fastify-ws/commit/15d235fc0772bdcaf3d14e600f5389cab4b5d3ed

[delvedor]

Awesome! One last thing. I wrote here to avoid open another issue. I'll rename the wsServer name space in ws, is more handy :) What do you think?

fastify.ws.on('message', () => {})

[gj]

Sounds great @delvedor !

While I have you here, mind taking a look at why I had to process.exit() to prevent the test suite from timing out? I know it's because there's a connection keeping everything alive, but I tried killing everything here and here.

[gj]

Went ahead and simplified the API: https://github.com/gj/fastify-ws/commit/43a95af153977ca44f8498b635364460a63fd0f2

[delvedor]

Regarding the need of process.exit() try add the following snippet to your code:

fastify.addHook('onClose', (fastify, done) => {
  fastify.ws.close(done)
})

[gj]

Worked like a charm: https://github.com/gj/fastify-ws/commit/5b54c7a31bf10dff0dceed67b025cdc7cefdf1d4

Thanks @delvedor !

[gj]

Gonna close this now — thanks @delvedor and @mcollina !

[delvedor]

Before I didn't see it, you should use the onClose hook inside the plugin code, not inside the test. In this way who will use this plugin will not have to register the closing hook, because is done automatically by the plugin :)

[gj]

Ah, good catch @delvedor !

All good? https://github.com/gj/fastify-ws/commit/a18e3bf3cabc429ae5110f069b894c0c8298a509

[delvedor]

Perfect :)