From 04af116eeae9bf0415ea1466e595213b8769b5bb Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 19 Sep 2024 12:03:45 +0200 Subject: [PATCH 1/2] backend-app-api: settle all inits before exiting on error Signed-off-by: Patrik Oldsberg --- .changeset/rich-deers-attend.md | 7 +++ .../src/wiring/BackendInitializer.test.ts | 43 +++++++++++++++++++ .../src/wiring/BackendInitializer.ts | 12 +++++- 3 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 .changeset/rich-deers-attend.md diff --git a/.changeset/rich-deers-attend.md b/.changeset/rich-deers-attend.md new file mode 100644 index 0000000000..1b26636009 --- /dev/null +++ b/.changeset/rich-deers-attend.md @@ -0,0 +1,7 @@ +--- +'@backstage/backend-app-api': patch +--- + +The backend will no longer exit immediately if any plugin or modules fails to initialize. Instead, the backend will wait for all plugins and modules to either start up successfully or throw, and then shut down the backend if there were any initialization errors. + +This fixes an issue where backend initialization errors in adjacent plugins during database schema migration could cause the database migrations to be stuck in a locked state. diff --git a/packages/backend-app-api/src/wiring/BackendInitializer.test.ts b/packages/backend-app-api/src/wiring/BackendInitializer.test.ts index b524301f21..8e92fc7120 100644 --- a/packages/backend-app-api/src/wiring/BackendInitializer.test.ts +++ b/packages/backend-app-api/src/wiring/BackendInitializer.test.ts @@ -418,6 +418,49 @@ describe('BackendInitializer', () => { ); }); + it('should forward errors when multiple plugins fail to start', async () => { + const init = new BackendInitializer([]); + init.add( + createBackendPlugin({ + pluginId: 'test-1', + register(reg) { + reg.registerInit({ + deps: {}, + async init() { + throw new Error('NOPE A'); + }, + }); + }, + }), + ); + init.add( + createBackendPlugin({ + pluginId: 'test-2', + register(reg) { + reg.registerInit({ + deps: {}, + async init() { + throw new Error('NOPE B'); + }, + }); + }, + }), + ); + const result = init.start(); + + await expect(result).rejects.toThrow('Backend startup failed'); + await expect(result).rejects.toMatchObject({ + errors: [ + expect.objectContaining({ + message: "Plugin 'test-1' startup failed; caused by Error: NOPE A", + }), + expect.objectContaining({ + message: "Plugin 'test-2' startup failed; caused by Error: NOPE B", + }), + ], + }); + }); + it('should forward errors when modules fail to start', async () => { const init = new BackendInitializer([]); init.add(testPlugin); diff --git a/packages/backend-app-api/src/wiring/BackendInitializer.ts b/packages/backend-app-api/src/wiring/BackendInitializer.ts index 6b020bb5ac..087f6e7924 100644 --- a/packages/backend-app-api/src/wiring/BackendInitializer.ts +++ b/packages/backend-app-api/src/wiring/BackendInitializer.ts @@ -275,7 +275,7 @@ export class BackendInitializer { ); // All plugins are initialized in parallel - await Promise.all( + const results = await Promise.allSettled( allPluginIds.map(async pluginId => { // Initialize all eager services await this.#serviceRegistry.initializeEagerServicesWithScope( @@ -345,6 +345,16 @@ export class BackendInitializer { }), ); + const initErrors = results.flatMap(r => + r.status === 'rejected' ? [r.reason] : [], + ); + if (initErrors.length === 1) { + throw initErrors[0]; + } else if (initErrors.length > 1) { + // TODO(Rugvip): Seems like there aren't proper types for AggregateError yet + throw new (AggregateError as any)(initErrors, 'Backend startup failed'); + } + // Once all plugins and modules have been initialized, we can signal that the backend has started up successfully const lifecycleService = await this.#getRootLifecycleImpl(); await lifecycleService.startup(); From 1db5312db8db599ce0b51a27236979c01981ab7e Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 19 Sep 2024 15:00:48 +0200 Subject: [PATCH 2/2] backend-app-api: log at the point of plugin startup failure Signed-off-by: Patrik Oldsberg --- .../src/wiring/BackendInitializer.ts | 127 +++++++++--------- .../src/wiring/createInitializationLogger.ts | 11 ++ 2 files changed, 77 insertions(+), 61 deletions(-) diff --git a/packages/backend-app-api/src/wiring/BackendInitializer.ts b/packages/backend-app-api/src/wiring/BackendInitializer.ts index 087f6e7924..86771fbb74 100644 --- a/packages/backend-app-api/src/wiring/BackendInitializer.ts +++ b/packages/backend-app-api/src/wiring/BackendInitializer.ts @@ -277,71 +277,76 @@ export class BackendInitializer { // All plugins are initialized in parallel const results = await Promise.allSettled( allPluginIds.map(async pluginId => { - // Initialize all eager services - await this.#serviceRegistry.initializeEagerServicesWithScope( - 'plugin', - pluginId, - ); - - // Modules are initialized before plugins, so that they can provide extension to the plugin - const modules = moduleInits.get(pluginId); - if (modules) { - const tree = DependencyGraph.fromIterable( - Array.from(modules).map(([moduleId, moduleInit]) => ({ - value: { moduleId, moduleInit }, - // Relationships are reversed at this point since we're only interested in the extension points. - // If a modules provides extension point A we want it to be initialized AFTER all modules - // that depend on extension point A, so that they can provide their extensions. - consumes: Array.from(moduleInit.provides).map(p => p.id), - provides: Array.from(moduleInit.consumes).map(c => c.id), - })), - ); - const circular = tree.detectCircularDependency(); - if (circular) { - throw new ConflictError( - `Circular dependency detected for modules of plugin '${pluginId}', ${circular - .map(({ moduleId }) => `'${moduleId}'`) - .join(' -> ')}`, - ); - } - await tree.parallelTopologicalTraversal( - async ({ moduleId, moduleInit }) => { - const moduleDeps = await this.#getInitDeps( - moduleInit.init.deps, - pluginId, - moduleId, - ); - await moduleInit.init.func(moduleDeps).catch(error => { - throw new ForwardedError( - `Module '${moduleId}' for plugin '${pluginId}' startup failed`, - error, - ); - }); - }, - ); - } - - // Once all modules have been initialized, we can initialize the plugin itself - const pluginInit = pluginInits.get(pluginId); - // We allow modules to be installed without the accompanying plugin, so the plugin may not exist - if (pluginInit) { - const pluginDeps = await this.#getInitDeps( - pluginInit.init.deps, + try { + // Initialize all eager services + await this.#serviceRegistry.initializeEagerServicesWithScope( + 'plugin', pluginId, ); - await pluginInit.init.func(pluginDeps).catch(error => { - throw new ForwardedError( - `Plugin '${pluginId}' startup failed`, - error, + + // Modules are initialized before plugins, so that they can provide extension to the plugin + const modules = moduleInits.get(pluginId); + if (modules) { + const tree = DependencyGraph.fromIterable( + Array.from(modules).map(([moduleId, moduleInit]) => ({ + value: { moduleId, moduleInit }, + // Relationships are reversed at this point since we're only interested in the extension points. + // If a modules provides extension point A we want it to be initialized AFTER all modules + // that depend on extension point A, so that they can provide their extensions. + consumes: Array.from(moduleInit.provides).map(p => p.id), + provides: Array.from(moduleInit.consumes).map(c => c.id), + })), ); - }); + const circular = tree.detectCircularDependency(); + if (circular) { + throw new ConflictError( + `Circular dependency detected for modules of plugin '${pluginId}', ${circular + .map(({ moduleId }) => `'${moduleId}'`) + .join(' -> ')}`, + ); + } + await tree.parallelTopologicalTraversal( + async ({ moduleId, moduleInit }) => { + const moduleDeps = await this.#getInitDeps( + moduleInit.init.deps, + pluginId, + moduleId, + ); + await moduleInit.init.func(moduleDeps).catch(error => { + throw new ForwardedError( + `Module '${moduleId}' for plugin '${pluginId}' startup failed`, + error, + ); + }); + }, + ); + } + + // Once all modules have been initialized, we can initialize the plugin itself + const pluginInit = pluginInits.get(pluginId); + // We allow modules to be installed without the accompanying plugin, so the plugin may not exist + if (pluginInit) { + const pluginDeps = await this.#getInitDeps( + pluginInit.init.deps, + pluginId, + ); + await pluginInit.init.func(pluginDeps).catch(error => { + throw new ForwardedError( + `Plugin '${pluginId}' startup failed`, + error, + ); + }); + } + + initLogger.onPluginStarted(pluginId); + + // Once the plugin and all modules have been initialized, we can signal that the plugin has stared up successfully + const lifecycleService = await this.#getPluginLifecycleImpl(pluginId); + await lifecycleService.startup(); + } catch (error) { + initLogger.onPluginFailed(pluginId); + throw error; } - - initLogger.onPluginStarted(pluginId); - - // Once the plugin and all modules have been initialized, we can signal that the plugin has stared up successfully - const lifecycleService = await this.#getPluginLifecycleImpl(pluginId); - await lifecycleService.startup(); }), ); diff --git a/packages/backend-app-api/src/wiring/createInitializationLogger.ts b/packages/backend-app-api/src/wiring/createInitializationLogger.ts index 566b5507ec..de323f914b 100644 --- a/packages/backend-app-api/src/wiring/createInitializationLogger.ts +++ b/packages/backend-app-api/src/wiring/createInitializationLogger.ts @@ -27,6 +27,7 @@ export function createInitializationLogger( rootLogger?: RootLoggerService, ): { onPluginStarted(pluginId: string): void; + onPluginFailed(pluginId: string): void; onAllStarted(): void; } { const logger = rootLogger?.child({ type: 'initialization' }); @@ -67,6 +68,16 @@ export function createInitializationLogger( starting.delete(pluginId); started.add(pluginId); }, + onPluginFailed(pluginId: string) { + starting.delete(pluginId); + const status = + starting.size > 0 + ? `, waiting for ${starting.size} other plugins to finish before shutting down the process` + : ''; + logger?.error( + `Plugin '${pluginId}' thew an error during startup${status}`, + ); + }, onAllStarted() { logger?.info(`Plugin initialization complete${getInitStatus()}`);