Merge pull request #13499 from backstage/rugvip/handling

backend-app-api: improve error messaging

.changeset/tiny-rocks-flash.md

@@ -0,0 +1,5 @@
+---
+'@backstage/backend-app-api': patch
+---
+
+Improved error messaging when failing to instantiate services.

packages/backend-app-api/package.json

@@ -36,6 +36,7 @@
     "@backstage/backend-common": "^0.15.1-next.1",
     "@backstage/backend-plugin-api": "^0.1.2-next.0",
     "@backstage/backend-tasks": "^0.3.5-next.0",
+    "@backstage/errors": "^1.1.0",
     "@backstage/plugin-permission-node": "^0.6.5-next.1",
     "express": "^4.17.1",
     "express-promise-router": "^4.1.0",

packages/backend-app-api/src/wiring/ServiceRegistry.test.ts

@@ -220,4 +220,103 @@ describe('ServiceRegistry', () => {
     expect(innerFactory).toHaveBeenCalledWith('catalog');
     expect(innerFactory).toHaveBeenCalledWith('scaffolder');
   });
+
+  it('should throw if dependencies are not available', async () => {
+    const myFactory = createServiceFactory({
+      service: ref1,
+      deps: { dep: ref2 },
+      async factory() {
+        throw new Error('ignored');
+      },
+    });
+
+    const registry = new ServiceRegistry([myFactory]);
+    const factory = registry.get(ref1)!;
+
+    await expect(factory('catalog')).rejects.toThrow(
+      "Failed to instantiate service '1' for 'catalog' because the following dependent services are missing: '2'",
+    );
+  });
+
+  it('should throw if dependencies are not available 2', async () => {
+    const refA = createServiceRef<string>({ id: 'a' });
+    const refB = createServiceRef<string>({ id: 'b' });
+    const refC = createServiceRef<string>({ id: 'c' });
+    const refD = createServiceRef<string>({ id: 'd' });
+
+    const factoryA = createServiceFactory({
+      service: refA,
+      deps: { b: refB },
+      async factory({ b }) {
+        return async pluginId => b(pluginId);
+      },
+    });
+
+    const factoryB = createServiceFactory({
+      service: refB,
+      deps: { c: refC, d: refD },
+      async factory() {
+        throw new Error('ignored');
+      },
+    });
+
+    const registry = new ServiceRegistry([factoryA, factoryB]);
+    const factory = registry.get(refA)!;
+
+    await expect(factory('catalog')).rejects.toThrow(
+      "Failed to instantiate service 'a' for 'catalog' because the factory function threw an error, Error: Failed to instantiate service 'b' for 'catalog' because the following dependent services are missing: 'c', 'd'",
+    );
+  });
+
+  it('should decorate error messages thrown by the top-level factory function', async () => {
+    const myFactory = createServiceFactory({
+      service: ref1,
+      deps: {},
+      factory() {
+        throw new Error('top-level error');
+      },
+    });
+
+    const registry = new ServiceRegistry([myFactory]);
+    const factory = registry.get(ref1)!;
+
+    await expect(factory('catalog')).rejects.toThrow(
+      "Failed to instantiate service '1' because the top-level factory function threw an error, Error: top-level error",
+    );
+  });
+
+  it('should decorate error messages thrown by the plugin-level factory function', async () => {
+    const myFactory = createServiceFactory({
+      service: ref1,
+      deps: {},
+      async factory() {
+        return pluginId => {
+          throw new Error(`error in plugin ${pluginId}`);
+        };
+      },
+    });
+
+    const registry = new ServiceRegistry([myFactory]);
+    const factory = registry.get(ref1)!;
+
+    await expect(factory('catalog')).rejects.toThrow(
+      "Failed to instantiate service '1' for 'catalog' because the factory function threw an error, Error: error in plugin catalog",
+    );
+  });
+
+  it('should decorate error messages thrown by default factory loaders', async () => {
+    const ref = createServiceRef<string>({
+      id: '1',
+      defaultFactory() {
+        throw new Error('default factory error');
+      },
+    });
+
+    const registry = new ServiceRegistry([]);
+    const factory = registry.get(ref)!;
+
+    await expect(factory('catalog')).rejects.toThrow(
+      "Failed to instantiate service '1' because the default factory loader threw an error, Error: default factory error",
+    );
+  });
 });

packages/backend-app-api/src/wiring/ServiceRegistry.ts

@@ -18,6 +18,7 @@ import {
   FactoryFunc,
   ServiceRef,
 } from '@backstage/backend-plugin-api';
+import { stringifyError } from '@backstage/errors';
 
 export class ServiceRegistry {
   readonly #providedFactories: Map<string, ServiceFactory>;
@@ -47,24 +48,56 @@ export class ServiceRegistry {
       if (!factory) {
         let loadedFactory = this.#loadedDefaultFactories.get(defaultFactory!);
         if (!loadedFactory) {
-          loadedFactory = defaultFactory!(ref) as Promise<ServiceFactory>;
+          loadedFactory = Promise.resolve().then(
+            () => defaultFactory!(ref) as Promise<ServiceFactory>,
+          );
           this.#loadedDefaultFactories.set(defaultFactory!, loadedFactory);
         }
         // NOTE: This await is safe as long as #providedFactories is not mutated.
-        factory = await loadedFactory;
+        factory = await loadedFactory.catch(error => {
+          throw new Error(
+            `Failed to instantiate service '${
+              ref.id
+            }' because the default factory loader threw an error, ${stringifyError(
+              error,
+            )}`,
+          );
+        });
       }
 
       let implementation = this.#implementations.get(factory);
       if (!implementation) {
-        const factoryDeps = Object.fromEntries(
-          Object.entries(factory.deps).map(([name, serviceRef]) => [
-            name,
-            this.get(serviceRef)!, // TODO: throw
-          ]),
-        );
+        const missingRefs = new Array<ServiceRef<unknown>>();
+        const factoryDeps: { [name in string]: FactoryFunc<unknown> } = {};
+
+        for (const [name, serviceRef] of Object.entries(factory.deps)) {
+          const target = this.get(serviceRef);
+          if (!target) {
+            missingRefs.push(serviceRef);
+          } else {
+            factoryDeps[name] = target;
+          }
+        }
+
+        if (missingRefs.length) {
+          const missing = missingRefs.map(r => `'${r.id}'`).join(', ');
+          throw new Error(
+            `Failed to instantiate service '${ref.id}' for '${pluginId}' because the following dependent services are missing: ${missing}`,
+          );
+        }
 
         implementation = {
-          factoryFunc: factory.factory(factoryDeps),
+          factoryFunc: Promise.resolve()
+            .then(() => factory!.factory(factoryDeps))
+            .catch(error => {
+              throw new Error(
+                `Failed to instantiate service '${
+                  ref.id
+                }' because the top-level factory function threw an error, ${stringifyError(
+                  error,
+                )}`,
+              );
+            }),
           byPlugin: new Map(),
         };
 
@@ -73,7 +106,19 @@ export class ServiceRegistry {
 
       let result = implementation.byPlugin.get(pluginId) as Promise<any>;
       if (!result) {
-        result = implementation.factoryFunc.then(func => func(pluginId));
+        result = implementation.factoryFunc.then(func =>
+          Promise.resolve()
+            .then(() => func(pluginId))
+            .catch(error => {
+              throw new Error(
+                `Failed to instantiate service '${
+                  ref.id
+                }' for '${pluginId}' because the factory function threw an error, ${stringifyError(
+                  error,
+                )}`,
+              );
+            }),
+        );
 
         implementation.byPlugin.set(pluginId, result);
       }

yarn.lock

@@ -2846,6 +2846,7 @@ __metadata:
     "@backstage/backend-plugin-api": ^0.1.2-next.0
     "@backstage/backend-tasks": ^0.3.5-next.0
     "@backstage/cli": ^0.19.0-next.1
+    "@backstage/errors": ^1.1.0
     "@backstage/plugin-permission-node": ^0.6.5-next.1
     express: ^4.17.1
     express-promise-router: ^4.1.0