Close adapter-node server gracefully on SIGINT and SIGTERM

[vekunz]

Describe the problem

Currently, when building a SvelteKit app with the adapter-node, the production build does not handle SIGINT or SIGTERM signals. The result of this is that the SvelteKit app cannot be gracefully stopped, e.g. in a dockerized environment (Kubernetes in our case). The environment has to kill the app in order to shut it down. In a continuous deployment scenario this results in many broken HTTP requests.

Describe the proposed solution

Since this is a problem for anyone using the node adapter, this should be "fixed" generally in adapter-node.

The index.js file of adapter-node can be extended with this snippet (and the same for SIGTERM also):

process.on('SIGINT', () => {
	console.log('Got SIGINT. Starting graceful shutdown.');
	server.server?.close(() => {
		console.log('Server closed');
		process.exit(0);
	});
	if (
		// This function only exists since Node 18
		server.server !== undefined &&
		'closeIdleConnections' in server.server &&
		typeof server.server.closeIdleConnections === 'function'
	) {
		server.server.closeIdleConnections();
	}
});

Alternatives considered

Alternatively, anyone has to build the same custom logic for this.

Importance

would make my life easier

Additional Information

If the SvelteKit team says that such logic should be implemented in adapter-node, I can create a Pull Request for this (actually I already wrote the code for this in a fork and just need to create the Pull Request for it, but I wanted to ask first).

[dummdidumm]

As pointed out in the docs, such shutdown logic is highly dependent on the environment you're running in. Even if we would add the code you proposed, it would only work for some of the people because

• a) this only exists since Node 18, as pointed out in the code comment
• b) this might be insufficient for other use cases. Worse, if it was implemented and others need more cleanup logic before calling closeIdelConnections, they would no longer have execution order under their control

Therefore closing.

[benmccann]

I'm going to reopen this issue. We now require Node 18, so that's no longer an issue. Also, I think it's fine to have environment-specific logic as that's the point of adapters. Rich currently has a PR out for reading files which introduces a concept of an adapter specifying what features are supported on a platform. We could use such a concept here if needed

I used to work at Google and on each deploy, we'd take instances out of the load balancer, wait for all requests to drain, and then shut down the instance before deploying the new instance. It seems like a really important feature to me

[mustafa0x]

Agreed; graceful shutdown is table stakes. In python and ruby it's a given that the web server waits for requests to terminate before shutting down.

Related project, that is be used by nuxt/nitro: https://github.com/sebhildebrandt/http-graceful-shutdown#explanation

The readme has quite a nice visualization.

[vekunz]

We now require Node 18, so that's no longer an issue.

What do you mean by this? Does Node 18 handle a server shutdown differently?

We use Node 18 also, and use this snippet in a server/index.js file (so we start SvelteKit not with node build, but with node server).

import { server } from '../build/index.js';

process.on('SIGINT', () => {
  console.log('Got SIGINT. Starting graceful shutdown.');
  shutdownServer();
});

process.on('SIGTERM', () => {
  console.log('Got SIGTERM. Starting graceful shutdown.');
  shutdownServer();
});

function shutdownServer() {
  server.server?.close(() => {
    console.log('Server closed');
    process.exit(0);
  });
  server.server?.closeIdleConnections();
  setInterval(() => server.server?.closeIdleConnections(), 1_000);
  setTimeout(() => server.server?.closeAllConnections(), 20_000);
}

[karimfromjordan]

@vekunz The minimu version required by SvelteKit is Node.js 18 today so if we implement this feature we wouldn't have to check if closeIdleConnections is supported.

Could you explain a bit more why you call closeIdleConnections three times in your code snippet?

[vekunz]

Could you explain a bit more why you call closeIdleConnections three times in your code snippet?

server.server?.closeIdleConnections();
setInterval(() => server.server?.closeIdleConnections(), 1_000);

This code is there, because closeIdleConnections() only closes connections, that are idle in the moment the function is called. If a connection is not idle currently, this function will not do anything with it. Therefore we added the setInterval so that all connections are closed after they went idle.

setTimeout(() => server.server?.closeAllConnections(), 20_000);

This is a fallback, if there are connections that "don't want" to go idle. closeAllConnections aborts and closes all remaining connections. We set the setTimeout to 20 seconds since in our use cases this should never happen.

[karimfromjordan]

Ah I just realized you also call closeAllConnections. Yeah that makes sense. I went through all the issues on the Node.js, Fastify and Apollo Server repos and this is similar to what they do too. I want to run a few simple tests and then I could create a PR.

[karimfromjordan]

I did some more research and want to share my results. Many libraries still support Node.js 16 which is why it's difficult to find a recent example of how to gracefully shutdown the Node HTTP server. What those that still use Node.js v16 do, is track the requests and connections, call server.close() and also close any idle connections manually every time a request completes. With closeIdleConnections in Node.js 18 this has become a lot easier so we don't need to track any connections but we still need to track the requests if we want to close connections as soon as they become idle. The following code is an example of what I landed on.

import http from 'node:http';

const port = process.env.PORT ?? 3000;
const shutdown_timeout = parseInt(process.env.SHUTDOWN_TIMEOUT) || 30;

let requests = 0;
let shutdown_timeout_id;

function shutdown() {
	if (shutdown_timeout_id) return;

	server.closeIdleConnections();
	server.close(() => clearTimeout(shutdown_timeout_id));
	shutdown_timeout_id = setTimeout(() => server.closeAllConnections(), shutdown_timeout * 1000);
}

const server = http.createServer(async (req, res) => {
	res.writeHead(200).end();
});

server.on('request', (req) => {
	requests++;

	req.on('close', () => {
		requests--;

		if (shutdown_timeout_id) {
			server.closeIdleConnections();
		}
	});
});

server.listen({ port }, () => console.log(`Listening on ${port}`));

process.on('SIGTERM', shutdown);
process.on('SIGINT', shutdown);

1. It calls server.close which drains fullfills any remaining requests and rejects new requests.
2. It closes connections as soon as they become idle using closeIdleConnections.
3. If there are still open connections after a timeout for what ever reason they are closed forcefully using closeAllConnections. The timeout can be controlled using an envirnment variable.
4. If a shutdown has already been scheduled because the user sends a SIGINT more than once by pressing Ctrl + C multiple times in the terminal the shutdown function simply returns.

This is essentially what everyone else does too and it works as expected in my tests. Because closeIdleConnections is called every time a request completes and not in a setTimeout the server shuts down as quickly as possible. In #11653 we already need to be aware of the number of requests at any give moment so I will include the code for a graceful shutdown there.

[weepy]

OK - sad news

So I was having trouble closing down a project built using node-adapter. It took me a while but I found out that it's because it has a MongoDb connection. When I removed it - it would shut down properly.

export const { handle, signIn, signOut } = SvelteKitAuth({
    trustHost: true,
    secret: AUTH_SECRET,
    adapter: MongoDBAdapter(connection), <---- this guy

I found that even if I manually set the shutdown_timeout to 1 - nothing would happen and it would just sit there. Obviously annoying when deploying !

Perhaps there's a way to tell the Mongo Adapter to shutdown ? But it seems bad to force the user to always shutdown gracefully IMHO.

Node 20 here ...

[karimfromjordan]

I found that even if I manually set the shutdown_timeout to 1 - nothing would happen and it would just sit there.

This is expected. adapter-node gracefully shuts down the HTTP server — not your app. It drains all requests and then closes the server. If then there is no work for Node.js left to do your app will automatically terminate. If there is, because you have something else running besides the HTTP server, then you need to manually close that.

Perhaps there's a way to tell the Mongo Adapter to shutdown ?

You need to clean up your resources — either by using a process.on('exit') handler, or if it is asynchronous work, in a process.on('SIGTERM/SIGINT') handler. It is mentioned in the docs but maybe it's not clear enough.

But it seems bad to force the user to always shutdown gracefully IMHO.

Before adapter-node v5 you were forced to just kill your app. I personally can't think of a use case where you would want your HTTP requests, connections, DB etc. to get abruptly closed without waiting for them to finish their work and returning a response to the user. But even if there is, I would argue that the better default in the vast majority of cases is to clean up your resources before shutting down. If for whatever reason you don't want that you can use the custom server feature.

[weepy]

Hm yes that makes a lot of sense in production in particular.
Maybe there's a best of both worlds where SIGINT closes gracefully and SIGTERM kills ?

[weepy]

So now I did the following:

function closeMongoConnection() {
    client.close();
}

process.on('SIGINT', closeMongoConnection);
process.on('SIGTERM', closeMongoConnection);

This works if I run node build/index.js.

But now I can't Ctrl+C out of 'npm run dev' ! It's somehow reversed it!

[weepy]

FWIW it's easy enough to conditionally bind dependent on some ENV variable that's set in the package.json dev script, but it's a bit fiddly.

[weepy]

So need a bit of best practice advice.... I have a Sveltekit project that uses a mongo server using adapter-node

I need to add the following to make the project exit when I Ctrl+C.

function closeMongoConnection() {
    client.close()
}
process.on('SIGINT', closeMongoConnection);
process.on('SIGTERM', closeMongoConnection);

But then in npm run dev mode it won't exit using this method (after a db query). The only way I could get it to work was :

function closeMongoConnection() {
    client.close()
    setTimeout(() => {
        process.exit(0)
    }, 100)
}

process.on('SIGINT', closeMongoConnection);
process.on('SIGTERM', closeMongoConnection);

But this doesn't feel like a great way to do it .

Can anyone suggest best practice here ?

[karimfromjordan]

Adapters don't run in development, only in production. The code is correct although I would set the timeout equal to SHUTDOWN_TIMEOUT. Otherwise you may close the database connection before the server has finished processing all requests.

[karimfromjordan]

I'm looking into whether we can add a simple hook to make this easier.

[weepy]

Nice ! Something like that would be good - appreciate there's a lot get it all working smoothly .

RN it doesn't feel like a great DX for what should be a very common use case of connecting to a database in production.

[karimfromjordan]

@weepy the hook has just been added to adapter-node v5.1.0. You should now be able to simply do this instead:

process.on('sveltekit:shutdown', reason => {
  db.close();
}