Graceful exit - onClose hook is never called when connections are active

[franky47]

🐛 Bug Report

I'm trying to setup a graceful exit strategy by closing connections to other services (eg: database, redis cache) in an onClose hook, using fastify-graceful-shutdown.

However, when there are active websocket connections, the onClose hooks are never called, even though fastify.close() was called upon reception of the exit signal by fastify-graceful-shutdown.

This is probably linked to the fact that graceful shutdown waits for requests to complete (while refusing new ones, which works ok), and websocket requests never actually complete on their own..

To Reproduce

Using this example on CodeSandbox:

1. The server should start automatically
2. Run the client code locally (see below), the client stays connected.
3. Restart the server in CodeSandbox using the Server Control Panel > Control Container > Restart Server button.
4. Observe the logs: the onClose log line is not printed out.

Server code:

const Fastify = require("fastify");

const server = Fastify({ logger: true });

// The order does not seem to matter here:
server.register(require("fastify-graceful-shutdown"));
server.register(require("fastify-websocket"));

server.get("/foo", { websocket: true }, (connection, req) => {
  const socketID = req.headers["sec-websocket-key"];
  server.log.info({
    msg: "Client connected",
    socket: socketID
  });
  connection.socket.on("message", message => {
    server.log.info({
      msg: message,
      socket: socketID
    });
  });
  connection.socket.on("close", () => {
    server.log.info({
      msg: "Client disconnected",
      socket: socketID
    });
  });
});

server.addHook("onClose", (server, done) => {
  server.log.info("in the onClose hook");
  done();
});

server.listen({ port: 8080 });

Test client code:

const WebSocket = require('isomorphic-ws')

const client = new WebSocket('wss://uvgpw.sse.codesandbox.io/foo')

client.on('open', () => {
  console.log('Connected')
  client.send('foo')
})

client.on('close', () => {
  console.log('Disconnected')
})

Expected behavior

The onClose hook should be called when fastify.close() is called.

Closing the open websocket connections could be done manually here using whatever strategy makes sense for the app (sending a batch of data, a special message to the client before closing the connection...).

Your Environment

• node version: 13.7.0
• fastify version: 2.11.0
• os: macOS
• fastify-graceful-shutdown: 2.0.1
• fastify-websocket: 1.1.0

[mcollina]

This requires some work. Would you like to send a PR?

I would like to see a change that call the websocket server clients property to destroy/close all the connections, and an API to possibly send a broadcast before doing so.

[franky47]

I can open a PR, I need to investigate a few things first:

• why the onClose hook is not called (this should be the entry point of any exit cleanup strategy), and how to get it to run
• how to access the websocket connections in the onClose hook (or other parts of the application for that matter), while preserving their context (what route they are connected to: root handler vs route handlers), it could probably be achieved with trackClients: true.

[Eomm]

why the onClose hook is not called

Is codesandbox sending a SIGINT before restart?

how to access the websocket connections

This `fastify.websocketServer.clients' should works

[franky47]

I realised CodeSandbox is probably not the best environment for this kind of test, so here's a reproduction repository.

@Eomm yes, the signal sent is SIGINT.

[franky47]

From what I can tell, this line in fastify never calls the done callback when there are open connections, which is the reason why onClose is never called.

It seems the only solution is to register another SIGINT handler dedicated to closing the open websocket connections, which would trigger the instance.server.close(done) to call the done callback after all connections have been cleanly closed, and then the onClose hook could run, cleaning up the rest of the application. I'm going to try this out and will report back.

[franky47]

Here is a solution that works:

1. Register fastify-websocket and fastify-graceful-shutdown (whatever the order)
2. Register a custom signal handler using fastify-graceful-shutdown's gracefulShutdown method, that will close the websocket connections
3. Add an onClose hook that will cleanup the rest of the application once all the websocket connections have been closed.

const Fastify = require('fastify')

const server = Fastify({ logger: true })

// The order does not seem to matter here:
server.register(require('fastify-graceful-shutdown'))
server.register(require('fastify-websocket'))

server.register((server, _, next) => {
  // We need to call this in a plugin to let
  // fastify-graceful-shutdown decorate the instance
  server.gracefulShutdown((signal, done) => {
    server.log.info('Closing websocket connections')
    server.websocketServer.clients.forEach(client => {
      // Gracefully close client connection here
      client.close(1001, 'bye')
    })
    done()
  })
  next()
})

server.get('/foo', { websocket: true }, (connection, req) => {
  const socketID = req.headers['sec-websocket-key']
  server.log.info({
    msg: 'Client connected',
    socket: socketID
  })
  connection.socket.on('message', message => {
    server.log.info({
      msg: message,
      socket: socketID
    })
  })
  connection.socket.on('close', () => {
    server.log.info({
      msg: 'Client disconnected',
      socket: socketID
    })
  })
})

server.addHook('onClose', (server, done) => {
  // Will run only after all websocket connections have been closed
  server.log.info('in the onClose hook')
  done()
})

server.listen({ port: 3000 })

This solution being closely coupled with fastify-graceful-shutdown, I don't think a code change is necessary in this repository, however I can add a documentation PR for this caveat.

[franky47]

One more thing: here I'm closing the connections with client.close, which waits for a handshake acknowledging the client's disconnection. This potentially could take longer (up to 30 seconds) than the suicide timeout defined by fastify-graceful-shutdown (10 seconds), and would lead to the onClose hook not being called again.

The simple solution here would be to make the suicide timeout longer, another one would be to add a shorter timeout in the signal handler that kills a hanging connection before the suicide timeout runs out, giving the rest of application cleanup a chance to run.

[monteiz]

I may be overlooking a detail, but could you clarify the issue with employing this simple approach?

process.on("SIGINT", async function () {
    await yourCleanUpFunction();
    console.log("Clean, shutdown can proceed.");
    process.exit(0);
});

This implementation functions seamlessly, even when managed by pm2.