From 545557a928a4e49c1a75691e5578506601b40d1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carl-Erik=20Bergstr=C3=B6m?= Date: Tue, 3 Mar 2026 19:12:44 +0100 Subject: [PATCH] feat(backend-app-api): attribute registration errors to plugins/modules (#33029) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(backend-app-api): attribute registration errors to plugins/modules Signed-off-by: Carl-Erik Bergström --- .changeset/spicy-grapes-share.md | 5 + .../src/wiring/BackendInitializer.test.ts | 215 +++++++++++++++++- .../src/wiring/BackendInitializer.ts | 190 ++++++++++------ 3 files changed, 332 insertions(+), 78 deletions(-) create mode 100644 .changeset/spicy-grapes-share.md diff --git a/.changeset/spicy-grapes-share.md b/.changeset/spicy-grapes-share.md new file mode 100644 index 0000000000..c3d7702155 --- /dev/null +++ b/.changeset/spicy-grapes-share.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-app-api': minor +--- + +Registration errors should be forwarded as BackendStartupResult diff --git a/packages/backend-app-api/src/wiring/BackendInitializer.test.ts b/packages/backend-app-api/src/wiring/BackendInitializer.test.ts index 0491faa96f..f782295e71 100644 --- a/packages/backend-app-api/src/wiring/BackendInitializer.test.ts +++ b/packages/backend-app-api/src/wiring/BackendInitializer.test.ts @@ -899,7 +899,7 @@ describe('BackendInitializer', () => { }); it('should reject duplicate plugins', async () => { - const init = new BackendInitializer([]); + const init = new BackendInitializer(baseFactories); init.add( createBackendPlugin({ pluginId: 'test', @@ -922,13 +922,24 @@ describe('BackendInitializer', () => { }, }), ); - await expect(init.start()).rejects.toThrow( + + const err = await init.start().then( + () => { + throw new Error('Expected BackendStartupError to be thrown'); + }, + (e: BackendStartupError) => e, + ); + + expect(err).toBeInstanceOf(BackendStartupError); + const plugin = err?.result.plugins.find(p => p.pluginId === 'test'); + expect(plugin?.failure?.error.message).toBe( "Plugin 'test' is already registered", ); + expect(plugin?.failure?.allowed).toBe(false); }); it('should reject duplicate modules', async () => { - const init = new BackendInitializer([]); + const init = new BackendInitializer(baseFactories); init.add(testPlugin); init.add( createBackendModule({ @@ -954,8 +965,202 @@ describe('BackendInitializer', () => { }, }), ); - await expect(init.start()).rejects.toThrow( - "Module 'mod' for plugin 'test' is already registered", + + const err = await init.start().then( + () => { + throw new Error('Expected BackendStartupError to be thrown'); + }, + (e: BackendStartupError) => e, + ); + + expect(err).toBeInstanceOf(BackendStartupError); + const plugin = err?.result.plugins.find(p => p.pluginId === 'test'); + const modResult = plugin?.modules.find( + m => + m.failure?.error.message === + "Module 'mod' for plugin 'test' is already registered", + ); + expect(modResult).toBeDefined(); + expect(modResult?.failure?.allowed).toBe(false); + }); + + it('should allow other plugins to continue when one has a registration error', async () => { + const pluginAInit = jest.fn(async () => {}); + const init = new BackendInitializer(baseFactories); + init.add( + createBackendPlugin({ + pluginId: 'plugin-a', + register(reg) { + reg.registerInit({ + deps: {}, + init: pluginAInit, + }); + }, + }), + ); + init.add( + createBackendPlugin({ + pluginId: 'plugin-b', + register(reg) { + reg.registerInit({ + deps: {}, + async init() {}, + }); + }, + }), + ); + init.add( + createBackendPlugin({ + pluginId: 'plugin-b', + register(reg) { + reg.registerInit({ + deps: {}, + async init() {}, + }); + }, + }), + ); + + const err = await init.start().then( + () => { + throw new Error('Expected BackendStartupError to be thrown'); + }, + (e: BackendStartupError) => e, + ); + + expect(err).toBeInstanceOf(BackendStartupError); + // plugin-a should have started successfully + expect(pluginAInit).toHaveBeenCalled(); + const pluginA = err?.result.plugins.find(p => p.pluginId === 'plugin-a'); + expect(pluginA?.failure).toBeUndefined(); + // plugin-b should have a registration failure + const pluginB = err?.result.plugins.find(p => p.pluginId === 'plugin-b'); + expect(pluginB?.failure?.error.message).toBe( + "Plugin 'plugin-b' is already registered", + ); + }); + + it('should permit registration errors for plugins with onPluginBootFailure: continue', async () => { + const init = new BackendInitializer([ + ...baseFactories, + mockServices.rootConfig.factory({ + data: { + backend: { + startup: { + plugins: { test: { onPluginBootFailure: 'continue' } }, + }, + }, + }, + }), + ]); + init.add( + createBackendPlugin({ + pluginId: 'test', + register(reg) { + reg.registerInit({ + deps: {}, + async init() {}, + }); + }, + }), + ); + init.add( + createBackendPlugin({ + pluginId: 'test', + register(reg) { + reg.registerInit({ + deps: {}, + async init() {}, + }); + }, + }), + ); + + const { result } = await init.start(); + const plugin = result.plugins.find(p => p.pluginId === 'test'); + expect(plugin?.failure?.error.message).toBe( + "Plugin 'test' is already registered", + ); + expect(plugin?.failure?.allowed).toBe(true); + }); + + it('should attribute duplicate extension point errors to the correct plugin', async () => { + const extensionPoint = createExtensionPoint({ id: 'shared-ext' }); + const init = new BackendInitializer(baseFactories); + init.add( + createBackendPlugin({ + pluginId: 'plugin-a', + register(reg) { + reg.registerExtensionPoint(extensionPoint, 'a'); + reg.registerInit({ + deps: {}, + async init() {}, + }); + }, + }), + ); + init.add( + createBackendPlugin({ + pluginId: 'plugin-b', + register(reg) { + reg.registerExtensionPoint(extensionPoint, 'b'); + reg.registerInit({ + deps: {}, + async init() {}, + }); + }, + }), + ); + + const err = await init.start().then( + () => { + throw new Error('Expected BackendStartupError to be thrown'); + }, + (e: BackendStartupError) => e, + ); + + expect(err).toBeInstanceOf(BackendStartupError); + // plugin-a should succeed (registered first) + const pluginA = err?.result.plugins.find(p => p.pluginId === 'plugin-a'); + expect(pluginA?.failure).toBeUndefined(); + // plugin-b should fail due to duplicate extension point + const pluginB = err?.result.plugins.find(p => p.pluginId === 'plugin-b'); + expect(pluginB?.failure?.error.message).toBe( + "ExtensionPoint with ID 'shared-ext' is already registered", + ); + }); + + it('should attribute invalid registration type errors to plugin when pluginId is available', async () => { + const init = new BackendInitializer(baseFactories); + // Create a fake registration with an invalid type but valid pluginId + const fakeFeature = { + $$type: '@backstage/BackendFeature' as const, + version: 'v1' as const, + featureType: 'registrations' as const, + getRegistrations: () => [ + { + type: 'invalid-type', + pluginId: 'broken-plugin', + init: { deps: {}, func: async () => {} }, + extensionPoints: [], + }, + ], + }; + init.add(fakeFeature as any); + + const err = await init.start().then( + () => { + throw new Error('Expected BackendStartupError to be thrown'); + }, + (e: BackendStartupError) => e, + ); + + expect(err).toBeInstanceOf(BackendStartupError); + const plugin = err?.result.plugins.find( + p => p.pluginId === 'broken-plugin', + ); + expect(plugin?.failure?.error.message).toBe( + "Invalid registration type 'invalid-type'", ); }); diff --git a/packages/backend-app-api/src/wiring/BackendInitializer.ts b/packages/backend-app-api/src/wiring/BackendInitializer.ts index d51befb644..8ebab723da 100644 --- a/packages/backend-app-api/src/wiring/BackendInitializer.ts +++ b/packages/backend-app-api/src/wiring/BackendInitializer.ts @@ -310,77 +310,6 @@ export class BackendInitializer { // Initialize all root scoped services await this.#serviceRegistry.initializeEagerServicesWithScope('root'); - const pluginInits = new Map(); - const moduleInits = new Map>(); - - // Enumerate all registrations - for (const feature of this.#registrations) { - for (const r of feature.getRegistrations()) { - const provides = new Set>(); - - if (r.type === 'plugin' || r.type === 'module') { - // Handle v1 format: Array, unknown]> - for (const [extRef, extImpl] of r.extensionPoints) { - if (this.#extensionPoints.has(extRef.id)) { - throw new Error( - `ExtensionPoint with ID '${extRef.id}' is already registered`, - ); - } - this.#extensionPoints.set(extRef.id, { - pluginId: r.pluginId, - factory: () => extImpl, - }); - provides.add(extRef); - } - } else if (r.type === 'plugin-v1.1' || r.type === 'module-v1.1') { - // Handle v1.1 format: Array - for (const extReg of r.extensionPoints) { - if (this.#extensionPoints.has(extReg.extensionPoint.id)) { - throw new Error( - `ExtensionPoint with ID '${extReg.extensionPoint.id}' is already registered`, - ); - } - this.#extensionPoints.set(extReg.extensionPoint.id, { - pluginId: r.pluginId, - factory: extReg.factory, - }); - provides.add(extReg.extensionPoint); - } - } - - if (r.type === 'plugin' || r.type === 'plugin-v1.1') { - if (pluginInits.has(r.pluginId)) { - throw new Error(`Plugin '${r.pluginId}' is already registered`); - } - pluginInits.set(r.pluginId, { - provides, - consumes: new Set(Object.values(r.init.deps)), - init: r.init, - }); - } else if (r.type === 'module' || r.type === 'module-v1.1') { - let modules = moduleInits.get(r.pluginId); - if (!modules) { - modules = new Map(); - moduleInits.set(r.pluginId, modules); - } - if (modules.has(r.moduleId)) { - throw new Error( - `Module '${r.moduleId}' for plugin '${r.pluginId}' is already registered`, - ); - } - modules.set(r.moduleId, { - provides, - consumes: new Set(Object.values(r.init.deps)), - init: r.init, - }); - } else { - throw new Error(`Invalid registration type '${(r as any).type}'`); - } - } - } - - const pluginIds = [...pluginInits.keys()]; - const rootConfig = await this.#serviceRegistry.get( coreServices.rootConfig, 'root', @@ -390,15 +319,32 @@ export class BackendInitializer { 'root', ); + const allRegistrations = this.#registrations.flatMap(f => + f.getRegistrations(), + ); + + const allPluginIds = [ + ...new Set( + allRegistrations.flatMap(r => + 'pluginId' in r && typeof r.pluginId === 'string' ? [r.pluginId] : [], + ), + ), + ]; + const resultCollector = createInitializationResultCollector({ - pluginIds, + pluginIds: allPluginIds, logger: rootLogger, allowBootFailurePredicate: createAllowBootFailurePredicate(rootConfig), }); + const { pluginInits, moduleInits } = this.#enumerateRegistrations( + allRegistrations, + resultCollector, + ); + // All plugins are initialized in parallel await Promise.all( - pluginIds.map(async pluginId => { + [...pluginInits.keys()].map(async pluginId => { try { // Initialize all eager services await this.#serviceRegistry.initializeEagerServicesWithScope( @@ -491,6 +437,104 @@ export class BackendInitializer { return { result }; } + #enumerateRegistrations( + allRegistrations: ReturnType< + InternalBackendRegistrations['getRegistrations'] + >, + resultCollector: ReturnType, + ): { + pluginInits: Map; + moduleInits: Map>; + } { + const pluginInits = new Map(); + const moduleInits = new Map>(); + + for (const r of allRegistrations) { + const addedExtensionPointIds: string[] = []; + try { + const provides = new Set>(); + + if (r.type === 'plugin' || r.type === 'module') { + // Handle v1 format: Array, unknown]> + for (const [extRef, extImpl] of r.extensionPoints) { + if (this.#extensionPoints.has(extRef.id)) { + throw new Error( + `ExtensionPoint with ID '${extRef.id}' is already registered`, + ); + } + this.#extensionPoints.set(extRef.id, { + pluginId: r.pluginId, + factory: () => extImpl, + }); + addedExtensionPointIds.push(extRef.id); + provides.add(extRef); + } + } else if (r.type === 'plugin-v1.1' || r.type === 'module-v1.1') { + // Handle v1.1 format: Array + for (const extReg of r.extensionPoints) { + if (this.#extensionPoints.has(extReg.extensionPoint.id)) { + throw new Error( + `ExtensionPoint with ID '${extReg.extensionPoint.id}' is already registered`, + ); + } + this.#extensionPoints.set(extReg.extensionPoint.id, { + pluginId: r.pluginId, + factory: extReg.factory, + }); + addedExtensionPointIds.push(extReg.extensionPoint.id); + provides.add(extReg.extensionPoint); + } + } + + if (r.type === 'plugin' || r.type === 'plugin-v1.1') { + if (pluginInits.has(r.pluginId)) { + throw new Error(`Plugin '${r.pluginId}' is already registered`); + } + pluginInits.set(r.pluginId, { + provides, + consumes: new Set(Object.values(r.init.deps)), + init: r.init, + }); + } else if (r.type === 'module' || r.type === 'module-v1.1') { + let modules = moduleInits.get(r.pluginId); + if (!modules) { + modules = new Map(); + moduleInits.set(r.pluginId, modules); + } + if (modules.has(r.moduleId)) { + throw new Error( + `Module '${r.moduleId}' for plugin '${r.pluginId}' is already registered`, + ); + } + modules.set(r.moduleId, { + provides, + consumes: new Set(Object.values(r.init.deps)), + init: r.init, + }); + } else { + throw new Error(`Invalid registration type '${(r as any).type}'`); + } + } catch (error: unknown) { + assertError(error); + // Clean up partially registered extension points + for (const id of addedExtensionPointIds) { + this.#extensionPoints.delete(id); + } + if ('pluginId' in r && 'moduleId' in r) { + resultCollector.onPluginModuleResult(r.pluginId, r.moduleId, error); + } else if ('pluginId' in r) { + pluginInits.delete(r.pluginId); + moduleInits.delete(r.pluginId); + resultCollector.onPluginResult(r.pluginId, error); + } else { + throw error; + } + } + } + + return { pluginInits, moduleInits }; + } + // It's fine to call .stop() multiple times, which for example can happen with manual stop + process exit async stop(): Promise { instanceRegistry.unregister(this);