diff --git a/.changeset/tiny-rocks-flash.md b/.changeset/tiny-rocks-flash.md new file mode 100644 index 0000000000..32b467a6af --- /dev/null +++ b/.changeset/tiny-rocks-flash.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-app-api': patch +--- + +Improved error messaging when failing to instantiate services. diff --git a/packages/backend-app-api/package.json b/packages/backend-app-api/package.json index 0ae48d4e9c..f6c2b9f3b9 100644 --- a/packages/backend-app-api/package.json +++ b/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", diff --git a/packages/backend-app-api/src/wiring/ServiceRegistry.test.ts b/packages/backend-app-api/src/wiring/ServiceRegistry.test.ts index d5b688c0de..a00f6c0b4b 100644 --- a/packages/backend-app-api/src/wiring/ServiceRegistry.test.ts +++ b/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({ id: 'a' }); + const refB = createServiceRef({ id: 'b' }); + const refC = createServiceRef({ id: 'c' }); + const refD = createServiceRef({ 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({ + 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", + ); + }); }); diff --git a/packages/backend-app-api/src/wiring/ServiceRegistry.ts b/packages/backend-app-api/src/wiring/ServiceRegistry.ts index 9df092db42..5592a0f0eb 100644 --- a/packages/backend-app-api/src/wiring/ServiceRegistry.ts +++ b/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; @@ -47,24 +48,56 @@ export class ServiceRegistry { if (!factory) { let loadedFactory = this.#loadedDefaultFactories.get(defaultFactory!); if (!loadedFactory) { - loadedFactory = defaultFactory!(ref) as Promise; + loadedFactory = Promise.resolve().then( + () => defaultFactory!(ref) as Promise, + ); 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>(); + const factoryDeps: { [name in string]: FactoryFunc } = {}; + + 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; 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); } diff --git a/yarn.lock b/yarn.lock index 86d6360229..b6a63248d8 100644 --- a/yarn.lock +++ b/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