Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: adapter-node shutdown event #12153

Merged
merged 14 commits into from
Jun 13, 2024
Merged
5 changes: 5 additions & 0 deletions .changeset/cuddly-otters-repeat.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@sveltejs/adapter-node": minor
---

feat: add shutdown event
32 changes: 16 additions & 16 deletions documentation/docs/25-build-and-deploy/40-adapter-node.md
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,22 @@ By default `adapter-node` gracefully shuts down the HTTP server when a `SIGTERM`

> If you want to customize this behaviour you can use a [custom server](#custom-server).

You can listen to the `sveltekit:shutdown` event which is emitted after the HTTP server has finished shutting down. Unlike Node's `exit` event, the `sveltekit:shutdown` event is always emitted when a shutdown is triggered and also supports handling asynchronous code.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unlike Node's exit event, the sveltekit:shutdown event is always emitted when a shutdown is triggered

What do you mean by this? When isn't exit emitted? I don't see anything about it only happening sometimes in the Node docs

and also supports handling asynchronous code.

maybe we could say "asynchronous operations" instead of "asynchronous code" to match the wording of the Node docs

Copy link
Contributor Author

@karimfromjordan karimfromjordan Jun 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exit is emitted automatically when Node has no work left to do. But there might be the scenario where the SvelteKit server shuts down because we listen for a SIGINT but the Node app still doesn't close because the user has a Mongo DB connection in the background that prevents it. That's what's happening in the comment I linked to at the beginning. In that case, even though the SvelteKit server closed, an exit event is not emitted. And if we manually emit the exit event after the server has closed using process.exit() the user can't run any async cleanup operations because process.exit() forcefully closes the app as quickly as possible.

karimfromjordan marked this conversation as resolved.
Show resolved Hide resolved

```js
process.on('sveltekit:shutdown', async (reason) => {
await db.close();
await jobs.stop();
process.exit();
karimfromjordan marked this conversation as resolved.
Show resolved Hide resolved
});
```

The parameter `reason` has one of the following values:

- `SIGINT` - shutdown was triggered by a `SIGINT` signal
- `SIGTERM` - shutdown was triggered by a `SIGTERM` signal
- `IDLE` - shutdown was triggered by [`IDLE_TIMEOUT`](#environment-variables-idle-timeout)

## Socket activation

Most Linux operating systems today use a modern process manager called systemd to start the server and run and manage services. You can configure your server to allocate a socket and start and scale your app on demand. This is called [socket activation](http://0pointer.de/blog/projects/socket-activated-containers.html). In this case, the OS will pass two environment variables to your app — `LISTEN_PID` and `LISTEN_FDS`. The adapter will then listen on file descriptor 3 which refers to a systemd socket unit that you will have to create.
Expand Down Expand Up @@ -242,19 +258,3 @@ app.listen(3000, () => {
console.log('listening on port 3000');
});
```

## Troubleshooting

### Is there a hook for cleaning up before the app exits?

There's nothing built-in to SvelteKit for this, because such a cleanup hook depends highly on the execution environment you're on. For Node, you can use its built-in `process.on(...)` to implement a callback that runs before the app exits:

```js
// @errors: 2304 2580
function shutdownGracefully() {
// anything you need to clean up manually goes in here
db.shutdown();
}

process.on('exit', shutdownGracefully);
```
12 changes: 9 additions & 3 deletions packages/adapter-node/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,19 +42,25 @@ if (socket_activation) {
});
}

function shutdown() {
/** @param {'SIGINT' | 'SIGTERM' | 'IDLE'} reason */
function shutdown(reason) {
if (shutdown_timeout_id) return;

// @ts-expect-error this was added in 18.2.0 but is not reflected in the types
server.server.closeIdleConnections();
benmccann marked this conversation as resolved.
Show resolved Hide resolved

server.server.close(() => {
server.server.close((error) => {
karimfromjordan marked this conversation as resolved.
Show resolved Hide resolved
if (error) return;
benmccann marked this conversation as resolved.
Show resolved Hide resolved

if (shutdown_timeout_id) {
shutdown_timeout_id = clearTimeout(shutdown_timeout_id);
}
if (idle_timeout_id) {
idle_timeout_id = clearTimeout(idle_timeout_id);
}

// @ts-expect-error Custom events cannot be typed
benmccann marked this conversation as resolved.
Show resolved Hide resolved
process.emit('sveltekit:shutdown', reason);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit torn as to whether this is the right API. It is more convenient to work with but kind of duplicating/aggregating existing events. The alternative would be to have the user do this:

process.on('SIGINT', shutdownHandler)
process.on('SIGTERM', shutdownHandler)
process.on('sveltekit:idle', shutdownHandler)

I think what you're doing is probably nicer, but I thought I'd note it for posterity or in case other reviewers have a different opinion

Copy link
Contributor Author

@karimfromjordan karimfromjordan Jun 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that this doesn't work with the graceful shutdown. If the user listens for the SIGINTand SIGTERM events to run some cleanup code then that cleanup code will run at the same time as the server is shutting down and when some requests might not have finished yet. But ideally you want to close the db connection etc. after the server has closed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, got it. I was a bit thrown by the process.on('SIGTERM', shutdown); line. Do you think we could rename shutdown to graceful_shutdown to make it a bit clearer in the code that it's not shutting down immediately?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah makes sense. I just renamed it.

});

shutdown_timeout_id = setTimeout(
Expand Down Expand Up @@ -83,7 +89,7 @@ server.server.on(
server.server.closeIdleConnections();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm. I wonder if users would want an event to know that all requests have finished processing. if you issue a sveltekit:shutdown when receiving SIGINT and shutdown your database connections then maybe your requests can't actually finish and fail because the database connection is gone, which would kind of defeat the purpose of having a shutdown timeout.

maybe it'd be better to issue an event here rather than in shutdown. if users want to do something earlier they can still listen for SIGTERM or SIGINT individually without us firing an event that simply echoes them (/~https://github.com/sveltejs/kit/pull/12153/files#r1637507349)

Copy link
Contributor Author

@karimfromjordan karimfromjordan Jun 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was the idea. The sveltekit:shutdown event is emitted inside the server.close((error) => { ... }) callback. And that callback runs after all requests/connections have finished and the server closed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I didn't realize that server.close() was graceful. I added suggestion for a comment here: /~https://github.com/sveltejs/kit/pull/12153/files#r1638706765

}
if (requests === 0 && socket_activation && idle_timeout) {
idle_timeout_id = setTimeout(shutdown, idle_timeout * 1000);
idle_timeout_id = setTimeout(() => shutdown('IDLE'), idle_timeout * 1000);
}
});
}
Expand Down
Loading