mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-27 12:13:30 +00:00
🧯 ci: Disarm the Graceful-Shutdown Force-Exit Timer on Test Reset (#14972)
* fix(shutdown): disarm the force-exit timer when test state is reset `shutdown()` arms a 60s timer that calls `process.exit(1)` as a safety net for drains that never finish. It is cleared only when the drain runs to completion, so a drain that never settles — an HTTP server whose close callback never fires, a task that hangs — leaves it armed. `__resetShutdownStateForTests()` clears the task list, the shutting-down flag and the server reference, but not that timer. A suite that triggers a signal therefore leaves a live self-destruct behind: `unref` keeps it from holding the process open, but it still fires if anything else keeps the process alive to the timeout, and `process.exit(1)` then takes down whatever is running a minute later. Jest reports that as a bare `process.exit called with "1"` with no failing test, because the run dies before it can print a summary. Track the timer at module scope, clear it from the reset helper, and clear it from a `finally` so a throwing drain step cannot leak it either. The new test fails without the reset change: it starts a drain that never settles, resets state, advances 120s, and asserts the process was not exited. * Scope the force-exit timer to the shutdown that armed it Hoisting the timer to module scope introduced an aliasing hazard: a drain that settles late runs its `finally` against whatever `forceExitTimer` points at by then. If state was reset and a second shutdown armed its own timer in the meantime, the late `finally` cleared the second shutdown's safety net instead of its own. Keep a local handle per shutdown, always clear that, and null the module reference only while it still identifies the same timer. The added test fails without this: it starts a drain whose close callback is withheld, resets state, starts a second shutdown, then releases the first callback and asserts the second net still force-exits.
This commit is contained in:
parent
0ab3414c01
commit
6daafda86f
2 changed files with 89 additions and 8 deletions
|
|
@ -347,4 +347,63 @@ describe('setupGracefulShutdown', () => {
|
|||
expect(calls).toEqual(['async-done']);
|
||||
expect(exitSpy).toHaveBeenCalledWith(0);
|
||||
});
|
||||
it('disarms the force-exit timer when shutdown state is reset', async () => {
|
||||
jest.useFakeTimers();
|
||||
try {
|
||||
// A drain that never settles: the server close callback is never invoked,
|
||||
// so `shutdown` stays awaiting and never reaches its own `clearTimeout`.
|
||||
jest.spyOn(server, 'close').mockImplementation(() => server);
|
||||
setupGracefulShutdown(server);
|
||||
triggerSignal('SIGTERM');
|
||||
await Promise.resolve();
|
||||
|
||||
// The safety net is armed and would exit the process on its own.
|
||||
__resetShutdownStateForTests();
|
||||
jest.advanceTimersByTime(120_000);
|
||||
|
||||
// Without the reset clearing it, this timer fires long after the suite
|
||||
// that armed it has finished, killing the run with code 1.
|
||||
expect(exitSpy).not.toHaveBeenCalledWith(1);
|
||||
} finally {
|
||||
jest.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps a later shutdown's safety net when an earlier drain settles late", async () => {
|
||||
// `setImmediate` stays real so the first shutdown's continuation can actually
|
||||
// reach its `finally`; only the force-exit timer is faked.
|
||||
jest.useFakeTimers({ doNotFake: ['setImmediate'] });
|
||||
try {
|
||||
let releaseFirstClose: (() => void) | undefined;
|
||||
jest.spyOn(server, 'close').mockImplementation((cb?: (err?: Error) => void) => {
|
||||
if (cb) {
|
||||
releaseFirstClose = () => cb();
|
||||
}
|
||||
return server;
|
||||
});
|
||||
setupGracefulShutdown(server);
|
||||
triggerSignal('SIGTERM');
|
||||
await flush();
|
||||
|
||||
// A second shutdown arms its own net after the first is reset away.
|
||||
__resetShutdownStateForTests();
|
||||
const secondServer = http.createServer();
|
||||
Object.defineProperty(secondServer, 'listening', { value: true, configurable: true });
|
||||
jest.spyOn(secondServer, 'close').mockImplementation(() => secondServer);
|
||||
setupGracefulShutdown(secondServer);
|
||||
triggerSignal('SIGTERM');
|
||||
await flush();
|
||||
|
||||
// The first drain settles only now; its `finally` must not disarm the second.
|
||||
releaseFirstClose?.();
|
||||
await flush();
|
||||
await flush();
|
||||
await flush();
|
||||
|
||||
jest.advanceTimersByTime(120_000);
|
||||
expect(exitSpy).toHaveBeenCalledWith(1);
|
||||
} finally {
|
||||
jest.useRealTimers();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ const tasks: ShutdownTask[] = [];
|
|||
let nextRegistrationOrder = 0;
|
||||
let isShuttingDown = false;
|
||||
let httpServer: Server | null = null;
|
||||
let forceExitTimer: NodeJS.Timeout | null = null;
|
||||
|
||||
/**
|
||||
* Register a cleanup task for graceful shutdown. Post-drain is the default phase.
|
||||
|
|
@ -73,6 +74,11 @@ export function __resetShutdownStateForTests(): void {
|
|||
nextRegistrationOrder = 0;
|
||||
isShuttingDown = false;
|
||||
httpServer = null;
|
||||
/** A drain that never settles leaves this armed. It is `unref`'d, so it does
|
||||
* not hold the process open — but it does fire if anything else keeps the
|
||||
* process alive past the timeout, exiting a suite that had long since moved
|
||||
* on with code 1 and no attributable failure. */
|
||||
clearForceExitTimer();
|
||||
}
|
||||
|
||||
async function runShutdownTasks(phase: ShutdownPhase): Promise<void> {
|
||||
|
|
@ -93,6 +99,13 @@ async function runShutdownTasks(phase: ShutdownPhase): Promise<void> {
|
|||
}
|
||||
}
|
||||
|
||||
function clearForceExitTimer(): void {
|
||||
if (forceExitTimer) {
|
||||
clearTimeout(forceExitTimer);
|
||||
forceExitTimer = null;
|
||||
}
|
||||
}
|
||||
|
||||
async function shutdown(signal: NodeJS.Signals): Promise<void> {
|
||||
if (isShuttingDown) {
|
||||
return;
|
||||
|
|
@ -100,24 +113,33 @@ async function shutdown(signal: NodeJS.Signals): Promise<void> {
|
|||
isShuttingDown = true;
|
||||
logger.info(`Received ${signal}, draining HTTP server...`);
|
||||
|
||||
/** Owned locally so a late `finally` from a superseded drain cannot clear the
|
||||
* safety net belonging to a shutdown that started after it. */
|
||||
const forceExit = setTimeout(() => {
|
||||
logger.warn(`Graceful shutdown exceeded ${SHUTDOWN_TIMEOUT_MS}ms, forcing exit`);
|
||||
process.exit(1);
|
||||
}, SHUTDOWN_TIMEOUT_MS);
|
||||
forceExit.unref();
|
||||
forceExitTimer = forceExit;
|
||||
|
||||
let exitCode = 0;
|
||||
|
||||
const serverClosePromise = closeHttpServer().catch((err) => {
|
||||
logger.error('Error closing HTTP server during graceful shutdown:', err);
|
||||
exitCode = 1;
|
||||
});
|
||||
try {
|
||||
const serverClosePromise = closeHttpServer().catch((err) => {
|
||||
logger.error('Error closing HTTP server during graceful shutdown:', err);
|
||||
exitCode = 1;
|
||||
});
|
||||
|
||||
await runShutdownTasks('pre-drain');
|
||||
await serverClosePromise;
|
||||
await runShutdownTasks('post-drain');
|
||||
await runShutdownTasks('pre-drain');
|
||||
await serverClosePromise;
|
||||
await runShutdownTasks('post-drain');
|
||||
} finally {
|
||||
clearTimeout(forceExit);
|
||||
if (forceExitTimer === forceExit) {
|
||||
forceExitTimer = null;
|
||||
}
|
||||
}
|
||||
|
||||
clearTimeout(forceExit);
|
||||
logger.info('Graceful shutdown complete, exiting');
|
||||
process.exit(exitCode);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue