From 4408bdb0d3715f31cdff7d1f6b2ee70ae6c64850 Mon Sep 17 00:00:00 2001 From: Carlos Valente Date: Fri, 13 Mar 2026 22:52:18 +0100 Subject: [PATCH] refactor: improve shutdown cleanup --- apps/server/src/adapters/IAdapter.ts | 2 +- apps/server/src/adapters/OscAdapter.ts | 11 +- apps/server/src/adapters/WebsocketAdapter.ts | 21 +++- apps/server/src/app.ts | 116 +++++++++++++++---- apps/server/src/utils/withTimeout.ts | 15 +++ 5 files changed, 135 insertions(+), 30 deletions(-) create mode 100644 apps/server/src/utils/withTimeout.ts diff --git a/apps/server/src/adapters/IAdapter.ts b/apps/server/src/adapters/IAdapter.ts index 2b58f4edf..ce1858ca0 100644 --- a/apps/server/src/adapters/IAdapter.ts +++ b/apps/server/src/adapters/IAdapter.ts @@ -1,3 +1,3 @@ export interface IAdapter { - shutdown: () => void; + shutdown: () => Promise; } diff --git a/apps/server/src/adapters/OscAdapter.ts b/apps/server/src/adapters/OscAdapter.ts index 06ce82863..c728c4de5 100644 --- a/apps/server/src/adapters/OscAdapter.ts +++ b/apps/server/src/adapters/OscAdapter.ts @@ -72,10 +72,17 @@ class OscServer implements IAdapter { }); this.udpSocket.bind(port); } - shutdown() { + shutdown(): Promise { logger.info(LogOrigin.Rx, 'OSC: Closing server'); - this.udpSocket?.close(); + const socket = this.udpSocket; this.udpSocket = null; + if (!socket) { + return Promise.resolve(); + } + + return new Promise((resolve) => { + socket.close(() => resolve()); + }); } } diff --git a/apps/server/src/adapters/WebsocketAdapter.ts b/apps/server/src/adapters/WebsocketAdapter.ts index 531429c17..57622493c 100644 --- a/apps/server/src/adapters/WebsocketAdapter.ts +++ b/apps/server/src/adapters/WebsocketAdapter.ts @@ -235,8 +235,25 @@ class SocketServer implements IAdapter { } } - shutdown() { - this.wss?.close(); + shutdown(): Promise { + const wss = this.wss; + if (!wss) { + return Promise.resolve(); + } + + return new Promise((resolve) => { + // Notify clients first so they can reconnect gracefully + for (const client of wss.clients) { + if (client.readyState === WebSocket.OPEN || client.readyState === WebSocket.CONNECTING) { + client.close(1001, 'Server shutting down'); + } + } + + wss.close(() => { + this.wss = null; + resolve(); + }); + }); } } diff --git a/apps/server/src/app.ts b/apps/server/src/app.ts index 4ff999c70..f331b2eae 100644 --- a/apps/server/src/app.ts +++ b/apps/server/src/app.ts @@ -42,6 +42,7 @@ import { consoleError, consoleHighlight, consoleSuccess } from './utils/console. import { generateCrashReport } from './utils/generateCrashReport.js'; import { getNetworkInterfaces } from './utils/network.js'; import { clearUploadfolder } from './utils/upload.js'; +import { withTimeout } from './utils/withTimeout.js'; console.log('\n'); consoleHighlight(`Starting Ontime version ${ONTIME_VERSION}`); @@ -64,6 +65,7 @@ const prefix = updateRouterPrefix(); // Create express APP const app = express(); +let isShuttingDown = false; if (!isProduction) { // log server timings to requests app.use(serverTiming()); @@ -85,6 +87,15 @@ app.get(`${prefix}/health`, (_req, res) => { res.status(200).send('OK'); }); +// readiness probe route for orchestrators (e.g. kubernetes) +app.get(`${prefix}/ready`, (_req, res) => { + if (isShuttingDown) { + res.status(503).send('SHUTTING_DOWN'); + return; + } + res.status(200).send('READY'); +}); + // Implement route endpoints app.use(`${prefix}/login`, loginRouter); // router for login flow app.use(`${prefix}/data`, authenticate, appRouter); // router for application data @@ -136,6 +147,7 @@ enum OntimeStartOrder { let step = OntimeStartOrder.InitAssets; let expressServer: Server | null = null; +let shutdownPromise: Promise | null = null; const checkStart = (currentState: OntimeStartOrder) => { if (step !== currentState) { @@ -250,34 +262,88 @@ export const startIntegrations = async () => { }; /** - * @description clean shutdown app services - * @param {number} exitCode - * @return {Promise} + * Clean shutdown app services + * - it avoid concurrency issues with deduplication of request to shutdown + * - extracts exit code to modify cleanup behaviour */ -export const shutdown = async (exitCode = 0) => { - consoleHighlight(`Ontime shutting down with code ${exitCode}`); - - await flushPendingWrites().catch((_error) => { - /** nothing do to here */ - }); - - // clear the restore file if it was a normal exit - // 0 means it was a SIGNAL - // 1 means crash -> keep the file - // 2 means dev crash -> do nothing - // 3 means container shutdown -> keep the file - // 99 means there was a shutdown request from the UI - if (exitCode === 0 || exitCode === 99) { - await restoreService.clear(); - await portManager.shutdown(); +export async function shutdown(exitCode = 0): Promise { + if (shutdownPromise) { + return shutdownPromise; } - expressServer?.close(); - runtimeService.shutdown(); - logger.shutdown(); - oscServer.shutdown(); - socket.shutdown(); - process.exit(exitCode); + shutdownPromise = performShutdown(exitCode); + return shutdownPromise; +}; + +const closeHttpServer = async (server: Server | null): Promise => { + if (!server) return; + + const closePromise = new Promise((resolve, reject) => { + server.close((error) => { + if (error) { + if ((error as NodeJS.ErrnoException).code === 'ERR_SERVER_NOT_RUNNING') { + resolve(); + return; + } + reject(error); + return; + } + resolve(); + }); + }); + + server.closeIdleConnections(); + server.closeAllConnections(); + + await closePromise; +}; + +const shutdownGlobalTimeout = 10_000; // 10 seconds +const shutdownTimeout = 4_000; // 4 seconds + +async function performShutdown(exitCode: number): Promise { + isShuttingDown = true; + consoleHighlight(`Ontime shutting down with code ${exitCode}`); + + // if shutdown takes longer than 10 seconds, force exit to avoid hanging processes + const forceExitTimer = setTimeout(() => { + consoleError('Forced shutdown after timeout'); + process.exit(exitCode); + }, shutdownGlobalTimeout); + + try { + runtimeService.shutdown(); + + // Block for at most 4 seconds on each shutdown segment + await withTimeout( + flushPendingWrites().catch((_error) => { + /** nothing do to here */ + }), + shutdownTimeout, + ); + + // clear the restore file if it was a normal exit + // 0 means it was a SIGNAL + // 1 means crash -> keep the file + // 2 means dev crash -> do nothing + // 3 means container shutdown -> keep the file + // 99 means there was a shutdown request from the UI + if (exitCode === 0 || exitCode === 99) { + await withTimeout(restoreService.clear(), shutdownTimeout); + await withTimeout(portManager.shutdown(), shutdownTimeout); + } + + await withTimeout( + Promise.all([closeHttpServer(expressServer), socket.shutdown(), oscServer.shutdown()]), + shutdownTimeout, + ); + } catch (error) { + logger.error(LogOrigin.Server, `Shutdown error: ${error}`, false); + } finally { + clearTimeout(forceExitTimer); + logger.shutdown(); + process.exit(exitCode); + } }; process.on('exit', (code) => consoleHighlight(`Ontime shutdown with code: ${code}`)); diff --git a/apps/server/src/utils/withTimeout.ts b/apps/server/src/utils/withTimeout.ts new file mode 100644 index 000000000..1e2fed04b --- /dev/null +++ b/apps/server/src/utils/withTimeout.ts @@ -0,0 +1,15 @@ +/** + * Resolves or rejects with the provided promise, but fails if it does not settle within `timeoutMs`. + */ +export const withTimeout = (promise: Promise, timeoutMs: number): Promise => { + let timer: NodeJS.Timeout | null = null; + const timeout = new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error('Operation timed out')), timeoutMs); + }); + + return Promise.race([promise, timeout]).finally(() => { + if (timer) { + clearTimeout(timer); + } + }); +};