fix: gracefulShutdown awaits async cleanup after server.close drain (v0.0.4)

CHANGELOG.md

@@ -0,0 +1,86 @@
+# Changelog
+
+All notable changes to this project will be documented in this file.
+
+The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
+
+## [0.0.4] — Unreleased
+
+### Changed
+
+- **`gracefulShutdown(server, cleanup)` now awaits an async `cleanup`
+  callback after `server.close()` drains in-flight requests.** The
+  `cleanup` parameter type is widened from `() => void` to
+  `() => void | Promise<void>` (backward-compatible at the call site —
+  any function that previously satisfied `() => void` still satisfies
+  the new union). Cleanup errors are caught with
+  `console.error("gracefulShutdown: cleanup error", err)` and do not
+  abort `process.exit(0)`.
+
+  Previously, `cleanup` ran synchronously *before* `server.close()`,
+  which meant any returned `Promise` was dropped on the floor — the
+  process exited before async resource releases (Redis disconnects,
+  database pools, file handles) could complete. The new shape is:
+
+  1. SIGTERM/SIGINT triggers the handler.
+  2. `server.close()` is registered with an async callback that fires
+     once all sockets are closed; in parallel,
+     `server.closeIdleConnections()` runs synchronously to release
+     idle keep-alive sockets so the close callback can fire promptly.
+  3. Inside the close callback, `await cleanup?.()` runs (caught +
+     logged on rejection), then `process.exit(0)`.
+
+  `server.closeIdleConnections()` deliberately leaves connections that
+  are mid-request alone — those drain naturally through `server.close`,
+  preserving the graceful-shutdown contract for in-flight requests.
+  (Note: starting with Node.js 19.0.0, `server.close` reaps idle
+  keep-alive connections on its own; the explicit
+  `closeIdleConnections()` call is harmless on 19+ and necessary on
+  18.x.)
+
+  This is a behavior change. Operators relying on the old fire-and-forget
+  ordering — where `cleanup` started before `server.close()` and ran
+  concurrently with request draining — will now experience strictly
+  sequential, awaited cleanup after the request drain. In practice, the
+  new ordering is what most operators *expected* the old code to do, so
+  the migration impact is typically zero.
+
+### Added
+
+- **`server.closeIdleConnections()` is invoked synchronously after
+  `server.close()` registers its callback** so the close callback can
+  fire promptly on Node 18.x without waiting for keep-alive timeouts.
+  Active in-flight requests are not affected — only idle keep-alive
+  connections are released. Requires Node.js >= 18.2.0; `package.json`
+  now declares `engines.node >= 18.19.0` which already satisfies this.
+- **`engines.node >= 18.19.0`** declared in `package.json` to match the
+  consumer floor (`auth.provider`) and to fail fast for installers on
+  pre-18.2 Node where `closeIdleConnections` is undefined.
+- **Idempotent under repeated signal delivery.** A `shuttingDown` guard
+  in the handler returns early on the second SIGTERM / SIGINT, and the
+  signal listeners are removed on first invocation. Operators that send
+  multiple SIGTERMs (k8s sending repeated TERM before falling back to
+  SIGKILL, or operators pressing Ctrl+C several times) no longer cause
+  duplicated `cleanup()` invocations or duplicate `process.exit(0)`
+  calls.
+
+### Migration
+
+The signature widening from `() => void` to `() => void | Promise<void>`
+is structurally backward-compatible: every previous caller's `cleanup`
+shape still satisfies the new type. No code change is required at call
+sites unless the caller explicitly wants to take advantage of awaited
+async cleanup (e.g., switching from
+`() => { redis.disconnect(); }` to
+`async () => { await redis.disconnect(); }`).
+
+For callers in `o3co/auth.provider` `templates/standalone/src/app.mts`:
+the existing call `gracefulShutdown(server, () => handle.dispose())` was
+already passing a function returning a `Promise` that the old code was
+silently dropping. After this release, `handle.dispose()` is properly
+awaited before `process.exit(0)` runs.
+
+## [0.0.3] — 2026-04-14
+
+- CI / publishing infrastructure: OIDC Trusted Publishing, dependabot,
+  pnpm store cache, idempotent release workflow, badges.

package.json

@@ -3,6 +3,9 @@
   "description": "Shared utilities for auth.* services",
   "version": "0.0.0",
   "license": "Apache-2.0",
+  "engines": {
+    "node": ">=18.19.0"
+  },
   "repository": {
     "type": "git",
     "url": "https://github.com/o3co/auth.utils.git"

src/__tests__/shutdown.test.mts

@@ -2,6 +2,11 @@ import { createServer } from "node:http";
 import { afterEach, describe, expect, it, vi } from "vitest";
 import { gracefulShutdown } from "../shutdown.mjs";
 
+const flushEventLoop = async (): Promise<void> => {
+	await new Promise<void>((r) => setImmediate(r));
+	await new Promise<void>((r) => setImmediate(r));
+};
+
 describe("gracefulShutdown", () => {
 	const listeners: Map<string, (...args: unknown[]) => void> = new Map();
 
@@ -29,13 +34,146 @@ describe("gracefulShutdown", () => {
 		onSpy.mockRestore();
 	});
 
-	it("calls cleanup function when handler is invoked", () => {
+	it("invokes cleanup inside server.close callback (after in-flight requests drain)", async () => {
+		const order: string[] = [];
+		const cleanup = vi.fn(() => {
+			order.push("cleanup");
+		});
+		const server = createServer();
+		const closeSpy = vi.spyOn(server, "close").mockImplementation((cb) => {
+			order.push("close");
+			cb?.();
+			return server;
+		});
+		const closeIdleSpy = vi.spyOn(server, "closeIdleConnections").mockImplementation(() => server);
+		const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => {
+			order.push("exit");
+			return undefined as never;
+		});
+		const onSpy = vi.spyOn(process, "on");
+
+		gracefulShutdown(server, cleanup);
+
+		const handler = onSpy.mock.calls.find(([event]) => event === "SIGTERM")?.[1] as () => void;
+		listeners.set("SIGTERM", handler);
+		handler();
+		await flushEventLoop();
+
+		expect(order).toEqual(["close", "cleanup", "exit"]);
+
+		closeSpy.mockRestore();
+		closeIdleSpy.mockRestore();
+		exitSpy.mockRestore();
+		onSpy.mockRestore();
+	});
+
+	it("awaits an async cleanup before exiting", async () => {
+		let cleanupResolved = false;
+		const cleanup = vi.fn(async () => {
+			await new Promise<void>((r) => setTimeout(r, 30));
+			cleanupResolved = true;
+		});
+		const server = createServer();
+		const closeSpy = vi.spyOn(server, "close").mockImplementation((cb) => {
+			cb?.();
+			return server;
+		});
+		const closeIdleSpy = vi.spyOn(server, "closeIdleConnections").mockImplementation(() => server);
+		let exitObservedCleanupResolved = false;
+		const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => {
+			exitObservedCleanupResolved = cleanupResolved;
+			return undefined as never;
+		});
+		const onSpy = vi.spyOn(process, "on");
+
+		gracefulShutdown(server, cleanup);
+
+		const handler = onSpy.mock.calls.find(([event]) => event === "SIGTERM")?.[1] as () => void;
+		listeners.set("SIGTERM", handler);
+		handler();
+		await new Promise<void>((r) => setTimeout(r, 60));
+
+		expect(cleanup).toHaveBeenCalledOnce();
+		expect(cleanupResolved).toBe(true);
+		expect(exitSpy).toHaveBeenCalledWith(0);
+		expect(exitObservedCleanupResolved).toBe(true);
+
+		closeSpy.mockRestore();
+		closeIdleSpy.mockRestore();
+		exitSpy.mockRestore();
+		onSpy.mockRestore();
+	});
+
+	it("logs cleanup errors via console.error and still exits", async () => {
+		const cleanupErr = new Error("cleanup boom");
+		const cleanup = vi.fn(async () => {
+			throw cleanupErr;
+		});
+		const server = createServer();
+		const closeSpy = vi.spyOn(server, "close").mockImplementation((cb) => {
+			cb?.();
+			return server;
+		});
+		const closeIdleSpy = vi.spyOn(server, "closeIdleConnections").mockImplementation(() => server);
+		const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => undefined as never);
+		const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
+		const onSpy = vi.spyOn(process, "on");
+
+		gracefulShutdown(server, cleanup);
+
+		const handler = onSpy.mock.calls.find(([event]) => event === "SIGTERM")?.[1] as () => void;
+		listeners.set("SIGTERM", handler);
+		handler();
+		await flushEventLoop();
+
+		expect(errorSpy).toHaveBeenCalledWith("gracefulShutdown: cleanup error", cleanupErr);
+		expect(exitSpy).toHaveBeenCalledWith(0);
+
+		closeSpy.mockRestore();
+		closeIdleSpy.mockRestore();
+		exitSpy.mockRestore();
+		errorSpy.mockRestore();
+		onSpy.mockRestore();
+	});
+
+	it("calls server.closeIdleConnections to drain idle keep-alive sockets without aborting in-flight requests", async () => {
+		const server = createServer();
+		const closeSpy = vi.spyOn(server, "close").mockImplementation((cb) => {
+			cb?.();
+			return server;
+		});
+		const closeIdleSpy = vi.spyOn(server, "closeIdleConnections").mockImplementation(() => server);
+		const closeAllSpy = vi.spyOn(server, "closeAllConnections").mockImplementation(() => server);
+		const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => undefined as never);
+		const onSpy = vi.spyOn(process, "on");
+
+		gracefulShutdown(server);
+
+		const handler = onSpy.mock.calls.find(([event]) => event === "SIGTERM")?.[1] as () => void;
+		listeners.set("SIGTERM", handler);
+		handler();
+		await flushEventLoop();
+
+		expect(closeIdleSpy).toHaveBeenCalledOnce();
+		// Regression defense: closeAllConnections aborts in-flight requests.
+		// Pre-Round 1 spec premise was wrong; never reintroduce.
+		expect(closeAllSpy).not.toHaveBeenCalled();
+
+		closeSpy.mockRestore();
+		closeIdleSpy.mockRestore();
+		closeAllSpy.mockRestore();
+		exitSpy.mockRestore();
+		onSpy.mockRestore();
+	});
+
+	it("works with a synchronous cleanup function", async () => {
 		const cleanup = vi.fn();
 		const server = createServer();
 		const closeSpy = vi.spyOn(server, "close").mockImplementation((cb) => {
 			cb?.();
 			return server;
 		});
+		const closeIdleSpy = vi.spyOn(server, "closeIdleConnections").mockImplementation(() => server);
 		const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => undefined as never);
 		const onSpy = vi.spyOn(process, "on");
 
@@ -46,22 +184,59 @@ describe("gracefulShutdown", () => {
 		listeners.set("SIGTERM", handler);
 
 		handler();
+		await flushEventLoop();
+
+		expect(cleanup).toHaveBeenCalledOnce();
+		expect(closeSpy).toHaveBeenCalledOnce();
+		expect(exitSpy).toHaveBeenCalledWith(0);
+
+		closeSpy.mockRestore();
+		closeIdleSpy.mockRestore();
+		exitSpy.mockRestore();
+		onSpy.mockRestore();
+	});
+
+	it("is idempotent under repeated signal delivery (multi-SIGTERM safe)", async () => {
+		const cleanup = vi.fn();
+		const server = createServer();
+		const closeSpy = vi.spyOn(server, "close").mockImplementation((cb) => {
+			cb?.();
+			return server;
+		});
+		const closeIdleSpy = vi.spyOn(server, "closeIdleConnections").mockImplementation(() => server);
+		const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => undefined as never);
+		const onSpy = vi.spyOn(process, "on");
+
+		gracefulShutdown(server, cleanup);
+
+		const handler = onSpy.mock.calls.find(([event]) => event === "SIGTERM")?.[1] as () => void;
+		listeners.set("SIGTERM", handler);
+		// Simulate three rapid SIGTERM deliveries (k8s will SIGKILL after grace,
+		// but operators may also Ctrl+C several times in a row).
+		handler();
+		handler();
+		handler();
+		await flushEventLoop();
 
 		expect(cleanup).toHaveBeenCalledOnce();
 		expect(closeSpy).toHaveBeenCalledOnce();
+		expect(closeIdleSpy).toHaveBeenCalledOnce();
+		expect(exitSpy).toHaveBeenCalledOnce();
 		expect(exitSpy).toHaveBeenCalledWith(0);
 
 		closeSpy.mockRestore();
+		closeIdleSpy.mockRestore();
 		exitSpy.mockRestore();
 		onSpy.mockRestore();
 	});
 
-	it("works without cleanup function", () => {
+	it("works without cleanup function", async () => {
 		const server = createServer();
 		const closeSpy = vi.spyOn(server, "close").mockImplementation((cb) => {
 			cb?.();
 			return server;
 		});
+		const closeIdleSpy = vi.spyOn(server, "closeIdleConnections").mockImplementation(() => server);
 		const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => undefined as never);
 		const onSpy = vi.spyOn(process, "on");
 
@@ -72,11 +247,13 @@ describe("gracefulShutdown", () => {
 		listeners.set("SIGTERM", handler);
 
 		handler();
+		await flushEventLoop();
 
 		expect(closeSpy).toHaveBeenCalledOnce();
 		expect(exitSpy).toHaveBeenCalledWith(0);
 
 		closeSpy.mockRestore();
+		closeIdleSpy.mockRestore();
 		exitSpy.mockRestore();
 		onSpy.mockRestore();
 	});

src/shutdown.mts

@@ -1,9 +1,21 @@
 import type { Server } from "node:http";
 
-export function gracefulShutdown(server: Server, cleanup?: () => void): void {
+export function gracefulShutdown(server: Server, cleanup?: () => void | Promise<void>): void {
+	let shuttingDown = false;
 	const handler = (): void => {
-		cleanup?.();
-		server.close(() => process.exit(0));
+		if (shuttingDown) return;
+		shuttingDown = true;
+		process.removeListener("SIGTERM", handler);
+		process.removeListener("SIGINT", handler);
+		server.close(async () => {
+			try {
+				await cleanup?.();
+			} catch (err) {
+				console.error("gracefulShutdown: cleanup error", err);
+			}
+			process.exit(0);
+		});
+		server.closeIdleConnections();
 	};
 	process.on("SIGTERM", handler);
 	process.on("SIGINT", handler);