feat(backend-app-api): attribute registration errors to plugins/modules (#33029)

* feat(backend-app-api): attribute registration errors to plugins/modules

Signed-off-by: Carl-Erik Bergström <cbergstrom@spotify.com>

.changeset/spicy-grapes-share.md

@@ -0,0 +1,5 @@
+---
+'@backstage/backend-app-api': minor
+---
+
+Registration errors should be forwarded as BackendStartupResult

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<string>({ 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'",
     );
   });
 

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<string, BackendRegisterInit>();
-    const moduleInits = new Map<string, Map<string, BackendRegisterInit>>();
-
-    // Enumerate all registrations
-    for (const feature of this.#registrations) {
-      for (const r of feature.getRegistrations()) {
-        const provides = new Set<ExtensionPoint<unknown>>();
-
-        if (r.type === 'plugin' || r.type === 'module') {
-          // Handle v1 format: Array<readonly [ExtensionPoint<unknown>, 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<ExtensionPointRegistration>
-          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<typeof createInitializationResultCollector>,
+  ): {
+    pluginInits: Map<string, BackendRegisterInit>;
+    moduleInits: Map<string, Map<string, BackendRegisterInit>>;
+  } {
+    const pluginInits = new Map<string, BackendRegisterInit>();
+    const moduleInits = new Map<string, Map<string, BackendRegisterInit>>();
+
+    for (const r of allRegistrations) {
+      const addedExtensionPointIds: string[] = [];
+      try {
+        const provides = new Set<ExtensionPoint<unknown>>();
+
+        if (r.type === 'plugin' || r.type === 'module') {
+          // Handle v1 format: Array<readonly [ExtensionPoint<unknown>, 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<ExtensionPointRegistration>
+          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<void> {
     instanceRegistry.unregister(this);