hunterloftis / stoppable

Node's `server.close` the way you expected it to work.
MIT License
404 stars 25 forks source link

Using with express app #31

Open sujeet-agrahari opened 4 years ago

sujeet-agrahari commented 4 years ago

This is the express, how can I wrap it around stoppable?

const app = require('./src/app');

const port = process.env.PORT || 3000;
app.listen(port, () => {
  console.log(`Listening on port ${port}...`);
});
mstaicu commented 3 years ago

Try the following: (I've thrown in terminus for health checking and graceful shutdown)

import { createTerminus } from '@godaddy/terminus';
import stoppable from 'stoppable';

import app from './src/app';

const server = stoppable(
  app.listen(process.env.PORT, () => {
    console.log(`Express API running on port ${process.env.PORT}`);
  }),
);

const onSignal = () => {
  console.log('Received SIGTERM, closing down...');

  return new Promise((resolve, reject) => {
    server.stop(err => {
      if (err) {
        reject(err);
      }
      resolve(0);
    });
  });
};

const onHealthCheck = () => Promise.resolve();

createTerminus(server, {
  healthChecks: {
    '/healthz': onHealthCheck,
  },
  signals: ['SIGINT', 'SIGTERM'],
  onSignal,
});
maxbarbul commented 3 years ago

@mstaicu actually, godaddy/terminus is using stoppable under the hood. You don't need to use stoppable in your example.

It's probably late to answer :) however:

So, to use stoppable with express, you need to run something like this:

const app = express();
const httpServer = app.listen(...);
stoppable(httpServer, graceTimeout);
mstaicu commented 3 years ago

Not at all. Nice! Didn't know that @maxbarbul, TIL. As for my solution, I decided to temporarily go with the following approach of keeping track of connections and manually handling the node process termination

import type { Socket } from 'net';

import app from './app';

const server = app.listen(process.env.PORT, () => {
  console.log(`Running on port ${process.env.PORT}`);
});

const sockets = new Set<Socket>();

server.on('connection', socket => {
  sockets.add(socket);
  server.once('close', () => {
    sockets.delete(socket);
  });
});

/**
 * need this in docker container to properly exit since node doesn't handle SIGINT/SIGTERM
 * this also won't work on using npm start since:
 *
 * https://github.com/npm/npm/issues/4603
 * https://github.com/npm/npm/pull/10868
 * https://github.com/RisingStack/kubernetes-graceful-shutdown-example/blob/master/src/index.js
 */

// Quit on CTRL - C when running Docker in Terminal
process.on('SIGINT', () => {
  console.info('Received SIGINT, gracefully shutting down');
  shutdown();
});

// Quit on docker stop command
process.on('SIGTERM', () => {
  console.info('Received SIGTERM, gracefully shutting down');
  shutdown();
});

const shutdown = () => {
  /**
   * The server is finally closed when all connections are ended and the server emits a 'close' event.
   * waitForSocketsToClose will give existing connections 10 seconds to terminate before destroying the sockets
   */
  waitForSocketsToClose(10);

  /**
   * https://nodejs.org/api/net.html#net_server_close_callback
   *
   * server.close([callback])
   *
   * callback <Function> Called when the server is closed.
   * Returns: <net.Server>
   *
   * Stops the server from accepting new connections and keeps existing connections
   * This function is asynchronous, the server is finally closed when all connections are ended and the server emits a 'close' event.
   * The optional callback will be called once the 'close' event occurs. Unlike that event, it will be called with an Error as its only argument if the server was not open when it was closed.
   */

  server.close(err => {
    if (err) {
      console.error(err);
      process.exitCode = 1;
    }

    process.exit();
  });
};

const waitForSocketsToClose = (counter: number) => {
  if (counter > 0) {
    console.log(
      `Waiting ${counter} more ${
        counter !== 1 ? 'seconds' : 'second'
      } for all connections to close...`,
    );

    return setTimeout(waitForSocketsToClose, 1000, counter - 1);
  }

  console.log('Forcing all connections to close now');

  for (const socket of sockets) {
    socket.destroy();
    sockets.delete(socket);
  }
};

export { server };
sujeet-agrahari commented 3 years ago

@mstaicu @maxbarbul This is my server.js file,

/* eslint-disable no-console */
const stoppable = require('stoppable');

const app = require('./src/app');

const normalizePort = require('./src/utils/normalize-port');

const gracefulShutdown = require('./src/utils/graceful-shutdown');

/**
 * Get port from environment and store in Express.
 */

const port = normalizePort(process.env.PORT || '3000');

/**
 * Create HTTP server.
 */

const server = stoppable(app.listen(port));

server.on('error', onError);
server.on('listening', onListening);

/**
 * Event listener for HTTP server "error" event.
 */

function onError(error) {
  if (error.syscall !== 'listen') {
    throw error;
  }

  const bind = typeof port === 'string'
    ? `Pipe ${port}`
    : `Port ${port}`;

  // handle specific listen errors with friendly messages
  switch (error.code) {
    case 'EACCES':
      console.error(`${bind} requires elevated privileges`);
      process.exit(1);
      break;
    case 'EADDRINUSE':
      console.error(`${bind} is already in use`);
      process.exit(1);
      break;
    default:
      throw error;
  }
}

/**
 * Event listener for HTTP server "listening" event.
 */

function onListening() {
  const addr = server.address();
  const bind = typeof addr === 'string'
    ? `pipe ${addr}`
    : `port ${addr.port}`;
  console.info(`Listening on ${bind} in ${process.env.NODE_ENV} environment...`);
}

// quit on ctrl+c when running docker in terminal
process.on('SIGINT', async () => {
  console.info('Got SIGINT (aka ctrl+c in docker). Graceful shutdown initiated!', new Date().toISOString());
  await gracefulShutdown(server);
  console.log('here');
});

// quit properly on docker stop
process.on('SIGTERM', async () => {
  console.log('Got SIGTERM (docker container stop). \n Graceful shutdown initiated!', new Date().toISOString());
  await gracefulShutdown(server);
});

This is graceful-shtudown.js

const { sequelize } = require('../models');

module.exports = async (server) => new Promise((resolve, reject) => {
  sequelize.connectionManager.close()
    .then(() => console.log('Closed database connection!'))
    .catch(() => console.log('Error occurred while closing database connection!'));
  server.stop((err) => {
    if (err) {
      reject(err);
    }
    console.log('Graceful shutdown completed');
    resolve(0);
  });
});

I never get messages for closing database connection or shutdown completion.

I also tried using async/await syntax.

const { sequelize } = require('../models');

module.exports = async (server) => {
  try {
    await sequelize.connectionManager.close();
    console.log('Closed database connection!');
    await server.stop();
    console.log('Graceful shutdown completed!');
    process.exit();
  } catch (error) {
    console.log(error.message);
    process.exit(1);
  }
};

I don't any message from 'graceful-shutdown.js` either in this case.

Can you please look into what's wrong?

mstaicu commented 3 years ago

It's interesting that you say this because it is also something that I've experience, either from misusing the library or improperly configuring it. What I experienced was that whenever I called server.stop, the monkey patched method provided by stoppable (which by the way you don't need to await it, it's not a Promise), and I was providing a callback to the .stop method, which would get called once all the sockets have been closed, I would get an error saying that node cannot stop the server because it wasn't running. So this:

server.stop(err => {
  if (err) console.error(err);
  console.log('Closed');
});

I would start by adding a breakpoint to this line ( and starting the server with the debugger enabled, i.e. node --inspect entry-file ) and see why your callback doesn't get called