ducktors / fastify-socket.io

Fastify plugin for Socket.io
MIT License
95 stars 17 forks source link

Unable to close fastify server with open connections #88

Open giovanni-bertoncelli opened 7 months ago

giovanni-bertoncelli commented 7 months ago

I'm reporting a bug discussed here: https://github.com/fastify/help/issues/988. It seems that fastify-socket.io is missing some logic like the one illustrated here with fastify-websocket: https://github.com/fastify/fastify-websocket/blob/431cba461f0bdd133d607deee6cfc56b69ea379a/index.js#L155-L172.

Description

When a fastify instance gets closed the fastify-socket.io plugin never closes its underlying websocket connections so the server hangs indefinitely. There should be some logic that forces the plugin to close at least the local sockets in order to close correctly the underlying HTTP server.

Reproduction

In order to reproduce the issue you can use this example https://codesandbox.io/p/devbox/socketio-close-stuck-jlcglp, and following these steps:

Proposal

I'd like to have some forceClose parameter in order to close the local websockets by using socket.io API:

fastify.io.local.disconnectSockets(true);

Or, since fastify has already a parameter for this check whenever forcecloseconnections is true and force the disconnection.

Another option can be some preClose parameter with a design similar to the mentioned fastify-websocket.

giovanni-bertoncelli commented 7 months ago

@alemagio @matteovivona any thoughts on this?

alemagio commented 7 months ago

Hi @giovanni-bertoncelli, the plugin adds an hook to make sure that the socket is closed when the fastify instance is closed. I'll have a look if there is a bug.

giovanni-bertoncelli commented 7 months ago

Yes, I saw that.. but onClose hook gets never called since the server never closes when there are connections hanging.. You can reproduce that very easily with the codesandbox I've linked.

giovanni-bertoncelli commented 7 months ago

I can make a PR if you want

alemagio commented 7 months ago

Sure, it would be great 😄