From 5171d5c742d140b0965ad452a34b64f3d51e6be6 Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Tue, 3 Dec 2024 13:36:18 +0100 Subject: [PATCH] refactor: create pre shutdown lifecycle Signed-off-by: Camila Belo --- .../src/wiring/BackendInitializer.ts | 42 +++++++------------ .../wiring/createSpecializedBackend.test.ts | 2 + .../src/CreateBackend.test.ts | 2 + .../rootHealthServiceFactory.test.ts | 24 ++--------- .../rootHealth/rootHealthServiceFactory.ts | 5 +-- .../rootHttpRouterServiceFactory.ts | 2 +- .../rootLifecycleServiceFactory.ts | 31 ++++++++++++++ .../lib/PluginTaskSchedulerImpl.test.ts | 6 ++- .../definitions/RootLifecycleService.ts | 4 +- .../src/next/services/mockServices.ts | 1 + 10 files changed, 66 insertions(+), 53 deletions(-) diff --git a/packages/backend-app-api/src/wiring/BackendInitializer.ts b/packages/backend-app-api/src/wiring/BackendInitializer.ts index 101c9abeb7..86a9aa6405 100644 --- a/packages/backend-app-api/src/wiring/BackendInitializer.ts +++ b/packages/backend-app-api/src/wiring/BackendInitializer.ts @@ -65,18 +65,9 @@ const instanceRegistry = new (class InstanceRegistry { if (!this.#registered) { this.#registered = true; - process.addListener('SIGTERM', () => { - this.#exitHandler(); - }); - process.addListener('SIGINT', () => { - this.#exitHandler(); - }); - process.addListener('SIGQUIT', () => { - process.exit(0); - }); - process.addListener('beforeExit', () => { - this.#exitHandler(); - }); + process.addListener('SIGTERM', this.#exitHandler); + process.addListener('SIGINT', this.#exitHandler); + process.addListener('beforeExit', this.#exitHandler); } this.#instances.add(instance); @@ -88,18 +79,12 @@ const instanceRegistry = new (class InstanceRegistry { #exitHandler = async () => { try { - // This signals to the healthcheck service that the process is shutting down - process.exitCode = 0; - - const results = await Promise.race([ - // Give the backend 30 seconds to shut down, then force exit - new Promise(resolve => setTimeout(resolve, 30000)), - Promise.allSettled(Array.from(this.#instances).map(b => b.stop())), - ]); - - const errors = Array.isArray(results) - ? results.flatMap(r => (r.status === 'rejected' ? [r.reason] : [])) - : []; + const results = await Promise.allSettled( + Array.from(this.#instances).map(b => b.stop()), + ); + const errors = results.flatMap(r => + r.status === 'rejected' ? [r.reason] : [], + ); if (errors.length > 0) { for (const error of errors) { @@ -464,6 +449,11 @@ export class BackendInitializer { // The startup failed, but we may still want to do cleanup so we continue silently } + const rootLifecycleService = await this.#getRootLifecycleImpl(); + + // Root services like the health one need to immediatelly be notified of the shutdown + await rootLifecycleService.preShutdown(); + // Get all plugins. const allPlugins = new Set(); for (const feature of this.#registrations) { @@ -483,14 +473,14 @@ export class BackendInitializer { ); // Once all plugin shutdown hooks are done, run root shutdown hooks. - const lifecycleService = await this.#getRootLifecycleImpl(); - await lifecycleService.shutdown(); + await rootLifecycleService.shutdown(); } // Bit of a hacky way to grab the lifecycle services, potentially find a nicer way to do this async #getRootLifecycleImpl(): Promise< RootLifecycleService & { startup(): Promise; + preShutdown(): Promise; shutdown(): Promise; } > { diff --git a/packages/backend-app-api/src/wiring/createSpecializedBackend.test.ts b/packages/backend-app-api/src/wiring/createSpecializedBackend.test.ts index 1fd7fcbbec..32c1fbcd0e 100644 --- a/packages/backend-app-api/src/wiring/createSpecializedBackend.test.ts +++ b/packages/backend-app-api/src/wiring/createSpecializedBackend.test.ts @@ -36,6 +36,7 @@ describe('createSpecializedBackend', () => { deps: {}, factory: async () => ({ addStartupHook: () => {}, + addBeforeShutdownHook: () => {}, addShutdownHook: () => {}, }), }), @@ -44,6 +45,7 @@ describe('createSpecializedBackend', () => { deps: {}, factory: async () => ({ addStartupHook: () => {}, + addBeforeShutdownHook: () => {}, addShutdownHook: () => {}, }), }), diff --git a/packages/backend-defaults/src/CreateBackend.test.ts b/packages/backend-defaults/src/CreateBackend.test.ts index 55fdbe5a6a..57977e9647 100644 --- a/packages/backend-defaults/src/CreateBackend.test.ts +++ b/packages/backend-defaults/src/CreateBackend.test.ts @@ -47,6 +47,7 @@ describe('createBackend', () => { deps: {}, factory: async () => ({ addStartupHook: () => {}, + addBeforeShutdownHook: () => {}, addShutdownHook: () => {}, }), }), @@ -57,6 +58,7 @@ describe('createBackend', () => { deps: {}, factory: async () => ({ addStartupHook: () => {}, + addBeforeShutdownHook: () => {}, addShutdownHook: () => {}, }), }), diff --git a/packages/backend-defaults/src/entrypoints/rootHealth/rootHealthServiceFactory.test.ts b/packages/backend-defaults/src/entrypoints/rootHealth/rootHealthServiceFactory.test.ts index c84c225fac..32fc10a77f 100644 --- a/packages/backend-defaults/src/entrypoints/rootHealth/rootHealthServiceFactory.test.ts +++ b/packages/backend-defaults/src/entrypoints/rootHealth/rootHealthServiceFactory.test.ts @@ -51,31 +51,13 @@ describe('DefaultRootHealthService', () => { }); }); - it(`should return a 503 response if the server is stopping`, async () => { - const service = new DefaultRootHealthService({ - lifecycle: mockServices.rootLifecycle.mock(), - }); - - process.exitCode = 1; - - await expect(service.getReadiness()).resolves.toEqual({ - status: 503, - payload: { - message: 'Backend has not started yet', - status: 'error', - }, - }); - - process.exitCode = undefined; // Reset exitCode for other tests - }); - it(`should return a 500 response if the server has stopped`, async () => { let mockServerStartedFn = () => {}; - let mockServerStoppedFn = () => {}; + let mockServerBeforeStoppedFn = () => {}; const lifecycle = mockServices.rootLifecycle.mock({ addStartupHook: jest.fn(fn => (mockServerStartedFn = fn)), - addShutdownHook: jest.fn(fn => (mockServerStoppedFn = fn)), + addBeforeShutdownHook: jest.fn(fn => (mockServerBeforeStoppedFn = fn)), }); const service = new DefaultRootHealthService({ @@ -83,7 +65,7 @@ describe('DefaultRootHealthService', () => { }); mockServerStartedFn(); - mockServerStoppedFn(); + mockServerBeforeStoppedFn(); await expect(service.getReadiness()).resolves.toEqual({ status: 503, payload: { diff --git a/packages/backend-defaults/src/entrypoints/rootHealth/rootHealthServiceFactory.ts b/packages/backend-defaults/src/entrypoints/rootHealth/rootHealthServiceFactory.ts index b37a6e589f..83efc5a055 100644 --- a/packages/backend-defaults/src/entrypoints/rootHealth/rootHealthServiceFactory.ts +++ b/packages/backend-defaults/src/entrypoints/rootHealth/rootHealthServiceFactory.ts @@ -29,7 +29,7 @@ export class DefaultRootHealthService implements RootHealthService { options.lifecycle.addStartupHook(() => { this.#isRunning = true; }); - options.lifecycle.addShutdownHook(() => { + options.lifecycle.addPreShutdownHook(() => { this.#isRunning = false; }); } @@ -39,8 +39,7 @@ export class DefaultRootHealthService implements RootHealthService { } async getReadiness(): Promise<{ status: number; payload?: any }> { - // If the process has been told to exit, or if the backend has not started yet - if (process.exitCode !== undefined || !this.#isRunning) { + if (!this.#isRunning) { return { status: 503, payload: { message: 'Backend has not started yet', status: 'error' }, diff --git a/packages/backend-defaults/src/entrypoints/rootHttpRouter/rootHttpRouterServiceFactory.ts b/packages/backend-defaults/src/entrypoints/rootHttpRouter/rootHttpRouterServiceFactory.ts index 4e3483d67e..3a5747057f 100644 --- a/packages/backend-defaults/src/entrypoints/rootHttpRouter/rootHttpRouterServiceFactory.ts +++ b/packages/backend-defaults/src/entrypoints/rootHttpRouter/rootHttpRouterServiceFactory.ts @@ -120,7 +120,7 @@ const rootHttpRouterServiceFactoryWithOptions = ( }, }); - lifecycle.addShutdownHook(() => server.stop()); + lifecycle.addPreShutdownHook(() => server.stop()); await server.start(); diff --git a/packages/backend-defaults/src/entrypoints/rootLifecycle/rootLifecycleServiceFactory.ts b/packages/backend-defaults/src/entrypoints/rootLifecycle/rootLifecycleServiceFactory.ts index e724c2f5cb..9711bd36fd 100644 --- a/packages/backend-defaults/src/entrypoints/rootLifecycle/rootLifecycleServiceFactory.ts +++ b/packages/backend-defaults/src/entrypoints/rootLifecycle/rootLifecycleServiceFactory.ts @@ -65,6 +65,37 @@ export class BackendLifecycleImpl implements RootLifecycleService { ); } + #hasPreShutdown = false; + #preShutdownTasks: Array<{ hook: () => void }> = []; + + addPreShutdownHook(hook: () => void): void { + if (this.#hasPreShutdown) { + throw new Error('Attempted to add pre shutdown hook after pre shutdown'); + } + this.#preShutdownTasks.push({ hook }); + } + + async preShutdown(): Promise { + if (this.#hasPreShutdown) { + return; + } + this.#hasPreShutdown = true; + + this.logger.debug( + `Running ${this.#preShutdownTasks.length} pre shutdown tasks...`, + ); + await Promise.all( + this.#preShutdownTasks.map(async ({ hook }) => { + try { + await hook(); + this.logger.debug(`Pre shutdown hook succeeded`); + } catch (error) { + this.logger.error(`Pre shutdown hook failed, ${error}`); + } + }), + ); + } + #hasShutdown = false; #shutdownTasks: Array<{ hook: LifecycleServiceShutdownHook; diff --git a/packages/backend-defaults/src/entrypoints/scheduler/lib/PluginTaskSchedulerImpl.test.ts b/packages/backend-defaults/src/entrypoints/scheduler/lib/PluginTaskSchedulerImpl.test.ts index 3c641feadf..1ce1401829 100644 --- a/packages/backend-defaults/src/entrypoints/scheduler/lib/PluginTaskSchedulerImpl.test.ts +++ b/packages/backend-defaults/src/entrypoints/scheduler/lib/PluginTaskSchedulerImpl.test.ts @@ -55,7 +55,11 @@ describe('PluginTaskManagerImpl', () => { const manager = new PluginTaskSchedulerImpl( async () => knex, mockServices.logger.mock(), - { addShutdownHook, addStartupHook: jest.fn() }, + { + addShutdownHook, + addBeforeShutdownHook: jest.fn(), + addStartupHook: jest.fn(), + }, ); return { knex, manager }; } diff --git a/packages/backend-plugin-api/src/services/definitions/RootLifecycleService.ts b/packages/backend-plugin-api/src/services/definitions/RootLifecycleService.ts index b962fa68ba..15b744e35b 100644 --- a/packages/backend-plugin-api/src/services/definitions/RootLifecycleService.ts +++ b/packages/backend-plugin-api/src/services/definitions/RootLifecycleService.ts @@ -23,4 +23,6 @@ import { LifecycleService } from './LifecycleService'; * * @public */ -export interface RootLifecycleService extends LifecycleService {} +export interface RootLifecycleService extends LifecycleService { + addPreShutdownHook(hook: () => void): void; +} diff --git a/packages/backend-test-utils/src/next/services/mockServices.ts b/packages/backend-test-utils/src/next/services/mockServices.ts index a2c3b34ead..b9bca62767 100644 --- a/packages/backend-test-utils/src/next/services/mockServices.ts +++ b/packages/backend-test-utils/src/next/services/mockServices.ts @@ -472,6 +472,7 @@ export namespace mockServices { export const factory = () => rootLifecycleServiceFactory; export const mock = simpleMock(coreServices.rootLifecycle, () => ({ addShutdownHook: jest.fn(), + addBeforeShutdownHook: jest.fn(), addStartupHook: jest.fn(), })); }