-
-
Notifications
You must be signed in to change notification settings - Fork 780
fix: restore runtime close hook and implement graceful shutdown env vars
#4017
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
base: main
Are you sure you want to change the base?
Changes from all commits
cd7850a
4218d87
3482064
853f67f
ea4235a
63e2ebd
77f90e4
70627b2
6261cd6
8e1f996
5ebc5f9
ad08c19
b25ed2f
8769db6
2127dc3
4b68824
8fc90dd
12a6cac
9c5aee1
6330afe
0a7dc65
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -18,6 +18,16 @@ NITRO_PRESET=deno_server npm run build | |||||
| deno run --unstable --allow-net --allow-read --allow-env .output/server/index.ts | ||||||
| ``` | ||||||
|
|
||||||
| ### Environment Variables | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same heading-level skip as bun.md β should be The file uses π Suggested fix-### Environment Variables
+## Environment Variablesπ Committable suggestion
Suggested change
π§° Toolsπͺ markdownlint-cli2 (0.20.0)[warning] 21-21: Heading levels should only increment by one level at a time (MD001, heading-increment) π€ Prompt for AI Agents |
||||||
|
|
||||||
| You can customize server behavior using following environment variables: | ||||||
|
|
||||||
| - `NITRO_PORT` or `PORT` (defaults to `3000`) | ||||||
| - `NITRO_HOST` or `HOST` | ||||||
| - `NITRO_SSL_CERT` and `NITRO_SSL_KEY` - if both are present, this will launch the server in HTTPS mode. In the vast majority of cases, this should not be used other than for testing, and the Nitro server should be run behind a reverse proxy like nginx or Cloudflare which terminates SSL. | ||||||
| - `NITRO_SHUTDOWN_DISABLED` - Disables the graceful shutdown feature when set to `'true'`. Defaults to `'false'`. | ||||||
| - `NITRO_SHUTDOWN_TIMEOUT` - Sets the amount of time (in milliseconds) before a forced shutdown occurs. Defaults to `5000` milliseconds. | ||||||
|
|
||||||
| ## Deno Deploy | ||||||
|
|
||||||
| :read-more{to="/deploy/providers/deno-deploy"} | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ import wsAdapter from "crossws/adapters/node"; | |
| import { useNitroApp } from "nitro/app"; | ||
| import { startScheduleRunner } from "#nitro/runtime/task"; | ||
| import { trapUnhandledErrors } from "#nitro/runtime/error/hooks"; | ||
| import { resolveGracefulShutdownConfig, setupShutdownHooks } from "#nitro/runtime/shutdown"; | ||
| import { resolveWebsocketHooks } from "#nitro/runtime/app"; | ||
|
|
||
| const _parsedPort = Number.parseInt(process.env.NITRO_PORT ?? process.env.PORT ?? ""); | ||
|
|
@@ -13,7 +14,8 @@ const port = Number.isNaN(_parsedPort) ? 3000 : _parsedPort; | |
| const host = process.env.NITRO_HOST || process.env.HOST; | ||
| const cert = process.env.NITRO_SSL_CERT; | ||
| const key = process.env.NITRO_SSL_KEY; | ||
| // const socketPath = process.env.NITRO_UNIX_SOCKET; // TODO | ||
| const socketPath = process.env.NITRO_UNIX_SOCKET; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could introduce this also in srvx directly via an env without |
||
| const gracefulShutdown = resolveGracefulShutdownConfig(); | ||
|
|
||
| const nitroApp = useNitroApp(); | ||
|
|
||
|
|
@@ -22,6 +24,8 @@ const server = serve({ | |
| hostname: host, | ||
| tls: cert && key ? { cert, key } : undefined, | ||
| fetch: nitroApp.fetch, | ||
| gracefulShutdown, | ||
| node: socketPath ? { path: socketPath } : undefined, | ||
| }); | ||
|
|
||
| if (import.meta._websocket) { | ||
|
|
@@ -38,6 +42,7 @@ if (import.meta._websocket) { | |
| } | ||
|
|
||
| trapUnhandledErrors(); | ||
| setupShutdownHooks(); | ||
|
|
||
| // Scheduled tasks | ||
| if (import.meta._tasks) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| import { useNitroApp } from "../app.ts"; | ||
| import type { ServerOptions } from "srvx"; | ||
|
|
||
| export function resolveGracefulShutdownConfig(): ServerOptions["gracefulShutdown"] { | ||
| if (process.env.NITRO_SHUTDOWN_DISABLED === "true") { | ||
| return false; | ||
| } | ||
wadefletch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| const timeoutMs = Number.parseInt(process.env.NITRO_SHUTDOWN_TIMEOUT ?? "", 10); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it is best that we don't introduce new |
||
|
|
||
| if (timeoutMs > 0) { | ||
| // srvx expects timeout in seconds | ||
| return { gracefulTimeout: timeoutMs / 1000 }; | ||
| } | ||
|
|
||
| return undefined; | ||
| } | ||
|
|
||
| function _onShutdownSignal() { | ||
| useNitroApp().hooks?.callHook("close"); | ||
| } | ||
|
|
||
| export function setupShutdownHooks() { | ||
| process.on("SIGTERM", _onShutdownSignal); | ||
| process.on("SIGINT", _onShutdownSignal); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; | ||
|
|
||
| const callHook = vi.fn().mockResolvedValue(undefined); | ||
|
|
||
| vi.mock("../../src/runtime/internal/app.ts", () => ({ | ||
| useNitroApp: () => ({ | ||
| hooks: { callHook }, | ||
| }), | ||
| })); | ||
|
|
||
| import { | ||
| resolveGracefulShutdownConfig, | ||
| setupShutdownHooks, | ||
| } from "../../src/runtime/internal/shutdown.ts"; | ||
| import type { ProcessEventMap } from "node:process"; | ||
|
|
||
| describe("resolveGracefulShutdownConfig", () => { | ||
| const env = process.env; | ||
|
|
||
| afterEach(() => { | ||
| process.env = env; | ||
| }); | ||
|
|
||
| it("returns undefined by default", () => { | ||
| process.env = { ...env }; | ||
| delete process.env.NITRO_SHUTDOWN_DISABLED; | ||
| delete process.env.NITRO_SHUTDOWN_TIMEOUT; | ||
| expect(resolveGracefulShutdownConfig()).toBeUndefined(); | ||
| }); | ||
|
|
||
| it.each([ | ||
| { value: "true", expected: false }, | ||
| { value: "false", expected: undefined }, | ||
| { value: "", expected: undefined }, | ||
| { value: "1", expected: undefined }, | ||
| { value: "yes", expected: undefined }, | ||
| ])("NITRO_SHUTDOWN_DISABLED=$value returns $expected", ({ value, expected }) => { | ||
| process.env = { ...env, NITRO_SHUTDOWN_DISABLED: value }; | ||
| delete process.env.NITRO_SHUTDOWN_TIMEOUT; | ||
| expect(resolveGracefulShutdownConfig()).toBe(expected); | ||
| }); | ||
|
|
||
| it("returns gracefulTimeout in seconds from NITRO_SHUTDOWN_TIMEOUT ms", () => { | ||
| process.env = { ...env, NITRO_SHUTDOWN_TIMEOUT: "10000" }; | ||
| delete process.env.NITRO_SHUTDOWN_DISABLED; | ||
| expect(resolveGracefulShutdownConfig()).toEqual({ gracefulTimeout: 10 }); | ||
| }); | ||
|
|
||
| it("disabled takes priority over timeout", () => { | ||
| process.env = { | ||
| ...env, | ||
| NITRO_SHUTDOWN_DISABLED: "true", | ||
| NITRO_SHUTDOWN_TIMEOUT: "10000", | ||
| }; | ||
| expect(resolveGracefulShutdownConfig()).toBe(false); | ||
| }); | ||
|
|
||
| it("ignores non-numeric timeout", () => { | ||
| process.env = { ...env, NITRO_SHUTDOWN_TIMEOUT: "abc" }; | ||
| delete process.env.NITRO_SHUTDOWN_DISABLED; | ||
| expect(resolveGracefulShutdownConfig()).toBeUndefined(); | ||
| }); | ||
| }); | ||
|
|
||
| describe("setupShutdownHooks", () => { | ||
| let signals: (keyof ProcessEventMap)[] = ["SIGTERM", "SIGINT"]; | ||
| let priors: Record<string, ((...args: any) => void)[]> = Object.fromEntries( | ||
| signals.map((s) => [s, []]) | ||
| ); | ||
|
|
||
| beforeEach(() => { | ||
| callHook.mockClear(); | ||
|
|
||
| for (const signal of signals) { | ||
| priors[signal] = process.listeners(signal).slice(); | ||
| } | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| for (const signal of signals) { | ||
| process.removeAllListeners(signal); | ||
| for (const fn of priors[signal]) { | ||
| process.on(signal, fn); | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| it.each(["SIGTERM", "SIGINT"])("calls close hook on %s", (signal) => { | ||
| setupShutdownHooks(); | ||
| process.emit(signal, true); | ||
| expect(callHook).toHaveBeenCalledOnce(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heading level skips from h1 to h3.
There's no h2 between
# Bun(line 5) and### Environment Variables(line 21). This should be##to comply with Markdown heading hierarchy.π Suggested fix
π€ Prompt for AI Agents