From 4ee5a98a7445f2a4b728c479ef60994668bce040 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Sat, 13 Jun 2026 21:25:01 +0200 Subject: [PATCH 1/3] feat(appkit): add global uncaughtException/unhandledRejection handlers Unhandled async errors previously crashed the Node process silently in production. Register process-level uncaughtException/unhandledRejection handlers in the server plugin, in the same place SIGTERM/SIGINT are wired. uncaughtException logs and runs the existing graceful-shutdown path then exits non-zero; unhandledRejection logs at error level. A module-wide guard registers the handlers once per process so repeated start() calls don't stack listeners. Co-authored-by: Isaac Signed-off-by: MarioCadenas --- packages/appkit/src/plugins/server/index.ts | 49 ++++++++++++++++++--- 1 file changed, 44 insertions(+), 5 deletions(-) diff --git a/packages/appkit/src/plugins/server/index.ts b/packages/appkit/src/plugins/server/index.ts index e66abf5ad..8ffd2e3f2 100644 --- a/packages/appkit/src/plugins/server/index.ts +++ b/packages/appkit/src/plugins/server/index.ts @@ -23,6 +23,13 @@ dotenv.config({ path: path.resolve(process.cwd(), "./.env") }); const logger = createLogger("server"); +/** + * Guards process-level lifecycle handlers (signals + global error handlers) + * to a single registration per process, regardless of how many server + * instances start. Avoids stacking listeners on the shared `process` object. + */ +let processHandlersRegistered = false; + /** Dev-only: try `requested` then consecutive ports (see `get-port` `portNumbers`). */ const devListenPortSpan = 100; @@ -160,8 +167,7 @@ export class ServerPlugin extends Plugin { // attach server to remote tunnel controller this.remoteTunnelController.setServer(server); - process.on("SIGTERM", () => this._gracefulShutdown()); - process.on("SIGINT", () => this._gracefulShutdown()); + this._registerProcessHandlers(); if (process.env.NODE_ENV === "development") { const allRoutes = getRoutes(this.serverApplication._router.stack); @@ -398,7 +404,40 @@ export class ServerPlugin extends Plugin { } } - private async _gracefulShutdown() { + /** + * Register process-level lifecycle handlers exactly once. + * + * Beyond SIGTERM/SIGINT, this wires up global error handlers so an + * unhandled async error no longer crashes the Node process silently in + * production: + * - `uncaughtException` — the process is in an undefined state, so we log + * the error and run the same graceful shutdown the signal handlers use, + * then exit non-zero (handled inside `_gracefulShutdown`). + * - `unhandledRejection` — log at error level for visibility without + * forcing an exit, matching how the codebase isolates non-fatal errors. + * + * Guarded module-wide so repeated `start()` calls (e.g. in tests, or + * multiple server instances) don't stack listeners on the shared + * `process` object. + */ + private _registerProcessHandlers() { + if (processHandlersRegistered) return; + processHandlersRegistered = true; + + process.on("SIGTERM", () => this._gracefulShutdown()); + process.on("SIGINT", () => this._gracefulShutdown()); + + process.on("uncaughtException", (error: Error, origin) => { + logger.error("Uncaught exception (%s), shutting down: %O", origin, error); + this._gracefulShutdown(1); + }); + + process.on("unhandledRejection", (reason) => { + logger.error("Unhandled promise rejection: %O", reason); + }); + } + + private async _gracefulShutdown(exitCode = 0) { logger.info("Starting graceful shutdown..."); if (this.viteDevServer) { @@ -433,7 +472,7 @@ export class ServerPlugin extends Plugin { if (this.server) { this.server.close(() => { logger.debug("Server closed gracefully"); - process.exit(0); + process.exit(exitCode); }); // 3. timeout to force shutdown after 15 seconds @@ -442,7 +481,7 @@ export class ServerPlugin extends Plugin { process.exit(1); }, 15000); } else { - process.exit(0); + process.exit(exitCode); } } From 303982d82a7723d8e9804316e2f1813e5c066d95 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Sat, 13 Jun 2026 21:44:22 +0200 Subject: [PATCH 2/3] refactor(appkit): derive process-handler install latch from started-servers set Replaces the module-level boolean guard with a static startedServers Set whose emptiness is the install-once latch: registration stays lazy (importing the module attaches nothing) and needs no separate mutable flag. Handlers now shut down every started server rather than only the first instance to start (the boolean closed over the first instance), and the per-instance method becomes a static installer. Co-authored-by: Isaac Signed-off-by: MarioCadenas --- packages/appkit/src/plugins/server/index.ts | 44 ++++++++++++--------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/packages/appkit/src/plugins/server/index.ts b/packages/appkit/src/plugins/server/index.ts index 8ffd2e3f2..11791e2eb 100644 --- a/packages/appkit/src/plugins/server/index.ts +++ b/packages/appkit/src/plugins/server/index.ts @@ -23,13 +23,6 @@ dotenv.config({ path: path.resolve(process.cwd(), "./.env") }); const logger = createLogger("server"); -/** - * Guards process-level lifecycle handlers (signals + global error handlers) - * to a single registration per process, regardless of how many server - * instances start. Avoids stacking listeners on the shared `process` object. - */ -let processHandlersRegistered = false; - /** Dev-only: try `requested` then consecutive ports (see `get-port` `portNumbers`). */ const devListenPortSpan = 100; @@ -167,7 +160,12 @@ export class ServerPlugin extends Plugin { // attach server to remote tunnel controller this.remoteTunnelController.setServer(server); - this._registerProcessHandlers(); + // The first server to start installs the shared process-level handlers; + // every started server then participates in shutdown. + if (ServerPlugin.startedServers.size === 0) { + ServerPlugin._installProcessHandlers(); + } + ServerPlugin.startedServers.add(this); if (process.env.NODE_ENV === "development") { const allRoutes = getRoutes(this.serverApplication._router.stack); @@ -405,7 +403,15 @@ export class ServerPlugin extends Plugin { } /** - * Register process-level lifecycle handlers exactly once. + * Servers that have started and participate in process-level lifecycle. + * The set being empty is the "install once" latch: the first `start()` + * installs the shared `process` handlers, and registration stays lazy + * (importing this module attaches nothing). Avoids a module-level flag. + */ + private static readonly startedServers = new Set(); + + /** + * Install process-level lifecycle handlers exactly once per process. * * Beyond SIGTERM/SIGINT, this wires up global error handlers so an * unhandled async error no longer crashes the Node process silently in @@ -416,20 +422,22 @@ export class ServerPlugin extends Plugin { * - `unhandledRejection` — log at error level for visibility without * forcing an exit, matching how the codebase isolates non-fatal errors. * - * Guarded module-wide so repeated `start()` calls (e.g. in tests, or - * multiple server instances) don't stack listeners on the shared - * `process` object. + * Handlers shut down every started server, not just the first — they live + * on the shared `process` object, so the single registration covers all. */ - private _registerProcessHandlers() { - if (processHandlersRegistered) return; - processHandlersRegistered = true; + private static _installProcessHandlers() { + const shutdownAll = (exitCode?: number) => { + for (const server of ServerPlugin.startedServers) { + void server._gracefulShutdown(exitCode); + } + }; - process.on("SIGTERM", () => this._gracefulShutdown()); - process.on("SIGINT", () => this._gracefulShutdown()); + process.on("SIGTERM", () => shutdownAll()); + process.on("SIGINT", () => shutdownAll()); process.on("uncaughtException", (error: Error, origin) => { logger.error("Uncaught exception (%s), shutting down: %O", origin, error); - this._gracefulShutdown(1); + shutdownAll(1); }); process.on("unhandledRejection", (reason) => { From 5e75cc573cabea0d2f86080fa029f0d9c6d65562 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Sat, 13 Jun 2026 21:56:50 +0200 Subject: [PATCH 3/3] refactor(appkit): keep process-handler guard simple Reverts the started-servers Set over-engineering. A single server runs per process, so a plain once-flag is sufficient; it only exists to stop repeated start() in tests from stacking listeners. Also trims verbose comments. Co-authored-by: Isaac Signed-off-by: MarioCadenas --- packages/appkit/src/plugins/server/index.ts | 51 +++++---------------- 1 file changed, 11 insertions(+), 40 deletions(-) diff --git a/packages/appkit/src/plugins/server/index.ts b/packages/appkit/src/plugins/server/index.ts index 11791e2eb..2ac43de85 100644 --- a/packages/appkit/src/plugins/server/index.ts +++ b/packages/appkit/src/plugins/server/index.ts @@ -23,6 +23,10 @@ dotenv.config({ path: path.resolve(process.cwd(), "./.env") }); const logger = createLogger("server"); +// Registered once per process; repeated start() calls (e.g. in tests) must not +// stack handlers on the shared `process`. +let processHandlersRegistered = false; + /** Dev-only: try `requested` then consecutive ports (see `get-port` `portNumbers`). */ const devListenPortSpan = 100; @@ -160,12 +164,7 @@ export class ServerPlugin extends Plugin { // attach server to remote tunnel controller this.remoteTunnelController.setServer(server); - // The first server to start installs the shared process-level handlers; - // every started server then participates in shutdown. - if (ServerPlugin.startedServers.size === 0) { - ServerPlugin._installProcessHandlers(); - } - ServerPlugin.startedServers.add(this); + this._registerProcessHandlers(); if (process.env.NODE_ENV === "development") { const allRoutes = getRoutes(this.serverApplication._router.stack); @@ -402,44 +401,16 @@ export class ServerPlugin extends Plugin { } } - /** - * Servers that have started and participate in process-level lifecycle. - * The set being empty is the "install once" latch: the first `start()` - * installs the shared `process` handlers, and registration stays lazy - * (importing this module attaches nothing). Avoids a module-level flag. - */ - private static readonly startedServers = new Set(); - - /** - * Install process-level lifecycle handlers exactly once per process. - * - * Beyond SIGTERM/SIGINT, this wires up global error handlers so an - * unhandled async error no longer crashes the Node process silently in - * production: - * - `uncaughtException` — the process is in an undefined state, so we log - * the error and run the same graceful shutdown the signal handlers use, - * then exit non-zero (handled inside `_gracefulShutdown`). - * - `unhandledRejection` — log at error level for visibility without - * forcing an exit, matching how the codebase isolates non-fatal errors. - * - * Handlers shut down every started server, not just the first — they live - * on the shared `process` object, so the single registration covers all. - */ - private static _installProcessHandlers() { - const shutdownAll = (exitCode?: number) => { - for (const server of ServerPlugin.startedServers) { - void server._gracefulShutdown(exitCode); - } - }; - - process.on("SIGTERM", () => shutdownAll()); - process.on("SIGINT", () => shutdownAll()); + private _registerProcessHandlers() { + if (processHandlersRegistered) return; + processHandlersRegistered = true; + process.on("SIGTERM", () => this._gracefulShutdown()); + process.on("SIGINT", () => this._gracefulShutdown()); process.on("uncaughtException", (error: Error, origin) => { logger.error("Uncaught exception (%s), shutting down: %O", origin, error); - shutdownAll(1); + this._gracefulShutdown(1); }); - process.on("unhandledRejection", (reason) => { logger.error("Unhandled promise rejection: %O", reason); });