From 69938650471bae695452a051ee26966971e8c84c Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 31 Aug 2025 10:10:36 +0200 Subject: [PATCH] frontend-app-api: initial error collection implementation Signed-off-by: Patrik Oldsberg --- packages/frontend-app-api/report.api.md | 18 + .../extractRouteInfoFromAppNode.test.ts | 16 +- .../src/tree/instantiateAppNodeTree.test.ts | 1169 ++++++++++------- .../src/tree/instantiateAppNodeTree.ts | 204 ++- .../src/tree/resolveAppNodeSpecs.test.ts | 134 +- .../src/tree/resolveAppNodeSpecs.ts | 150 +-- .../src/tree/resolveAppTree.test.ts | 234 ++-- .../src/tree/resolveAppTree.ts | 23 +- .../src/wiring/createErrorCollector.ts | 69 + .../src/wiring/createSpecializedApp.tsx | 43 +- packages/frontend-app-api/src/wiring/index.ts | 1 + 11 files changed, 1314 insertions(+), 747 deletions(-) create mode 100644 packages/frontend-app-api/src/wiring/createErrorCollector.ts diff --git a/packages/frontend-app-api/report.api.md b/packages/frontend-app-api/report.api.md index 039d0bb7b1..1806f47b02 100644 --- a/packages/frontend-app-api/report.api.md +++ b/packages/frontend-app-api/report.api.md @@ -4,16 +4,33 @@ ```ts import { ApiHolder } from '@backstage/core-plugin-api'; +import { AppNode } from '@backstage/frontend-plugin-api'; +import { AppNodeSpec } from '@backstage/frontend-plugin-api'; import { AppTree } from '@backstage/frontend-plugin-api'; import { ConfigApi } from '@backstage/core-plugin-api'; import { ExtensionFactoryMiddleware } from '@backstage/frontend-plugin-api'; import { ExternalRouteRef } from '@backstage/frontend-plugin-api'; import { FrontendFeature } from '@backstage/frontend-plugin-api'; +import { FrontendPlugin } from '@backstage/frontend-plugin-api'; import { FrontendPluginInfo } from '@backstage/frontend-plugin-api'; import { JsonObject } from '@backstage/types'; import { RouteRef } from '@backstage/frontend-plugin-api'; import { SubRouteRef } from '@backstage/frontend-plugin-api'; +// @public (undocumented) +export type AppError = { + code: string; + message: string; + context?: { + node?: AppNode; + spec?: AppNodeSpec; + plugin?: FrontendPlugin; + extensionId?: string; + inputName?: string; + dataRefId?: string; + }; +}; + // @public export type CreateAppRouteBinder = < TExternalRoutes extends { @@ -31,6 +48,7 @@ export type CreateAppRouteBinder = < export function createSpecializedApp(options?: CreateSpecializedAppOptions): { apis: ApiHolder; tree: AppTree; + errors?: AppError[]; }; // @public diff --git a/packages/frontend-app-api/src/routing/extractRouteInfoFromAppNode.test.ts b/packages/frontend-app-api/src/routing/extractRouteInfoFromAppNode.test.ts index bb03b5604b..efcc9ef18c 100644 --- a/packages/frontend-app-api/src/routing/extractRouteInfoFromAppNode.test.ts +++ b/packages/frontend-app-api/src/routing/extractRouteInfoFromAppNode.test.ts @@ -38,6 +38,18 @@ import { Root } from '../extensions/Root'; // eslint-disable-next-line @backstage/no-relative-monorepo-imports import { resolveExtensionDefinition } from '../../../frontend-plugin-api/src/wiring/resolveExtensionDefinition'; import { createRouteAliasResolver } from './RouteAliasResolver'; +import { createErrorCollector } from '../wiring/createErrorCollector'; + +const collector = createErrorCollector(); + +afterEach(() => { + const errors = collector.collectErrors(); + if (errors) { + throw new Error( + `Unexpected errors: ${errors.map(e => e.message).join(', ')}`, + ); + } +}); const ref1 = createRouteRef(); const ref2 = createRouteRef(); @@ -102,10 +114,12 @@ function routeInfoFromExtensions( ], parameters: readAppExtensionsConfig(mockApis.config()), forbidden: new Set(['root']), + collector, }), + collector, ); - instantiateAppNodeTree(tree.root, TestApiRegistry.from()); + instantiateAppNodeTree(tree.root, TestApiRegistry.from(), collector); return extractRouteInfoFromAppNode( tree.root, diff --git a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.test.ts b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.test.ts index 57ec7b23f3..9ed0319045 100644 --- a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.test.ts +++ b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.test.ts @@ -43,6 +43,7 @@ import { // eslint-disable-next-line @backstage/no-relative-monorepo-imports import { createSchemaFromZod } from '../../../frontend-plugin-api/src/schema/createSchemaFromZod'; import { TestApiRegistry, withLogCollector } from '@backstage/test-utils'; +import { createErrorCollector } from '../wiring/createErrorCollector'; const testApis = TestApiRegistry.from(); const testDataRef = createExtensionDataRef().with({ id: 'test' }); @@ -51,6 +52,17 @@ const inputMirrorDataRef = createExtensionDataRef().with({ id: 'mirror', }); +const collector = createErrorCollector(); + +afterEach(() => { + const errors = collector.collectErrors(); + if (errors) { + throw new Error( + `Unexpected errors: ${errors.map(e => e.message).join(', ')}`, + ); + } +}); + function makeSpec( extension: Extension, spec?: Partial, @@ -88,6 +100,7 @@ function makeInstanceWithId( node, attachments: new Map(), apis: testApis, + collector, }), }; } @@ -151,75 +164,37 @@ describe('instantiateAppNodeTree', () => { }); it('should instantiate a single node', () => { - const tree = resolveAppTree('root-node', [ - makeSpec(simpleExtension, { id: 'root-node' }), - ]); + const tree = resolveAppTree( + 'root-node', + [makeSpec(simpleExtension, { id: 'root-node' })], + collector, + ); expect(tree.root.instance).not.toBeDefined(); - instantiateAppNodeTree(tree.root, testApis); + instantiateAppNodeTree(tree.root, testApis, collector); expect(tree.root.instance).toBeDefined(); expect(tree.root.instance?.getData(testDataRef)).toBe('test'); // Multiple calls should have no effect - instantiateAppNodeTree(tree.root, testApis); + instantiateAppNodeTree(tree.root, testApis, collector); expect(tree.root.instance).toBeDefined(); }); it('should not instantiate disabled nodes', () => { - const tree = resolveAppTree('root-node', [ - makeSpec(simpleExtension, { id: 'root-node', disabled: true }), - ]); + const tree = resolveAppTree( + 'root-node', + [makeSpec(simpleExtension, { id: 'root-node', disabled: true })], + collector, + ); expect(tree.root.instance).not.toBeDefined(); - instantiateAppNodeTree(tree.root, testApis); + instantiateAppNodeTree(tree.root, testApis, collector); expect(tree.root.instance).not.toBeDefined(); }); it('should instantiate a node with attachments', () => { - const tree = resolveAppTree('root-node', [ - makeSpec( - createV1Extension({ - namespace: 'root-node', - attachTo: { id: 'ignored', input: 'ignored' }, - inputs: { - test: createV1ExtensionInput({ test: testDataRef }), - }, - output: { - inputMirror: inputMirrorDataRef, - }, - factory({ inputs }) { - return { inputMirror: inputs }; - }, - }), - ), - makeSpec(simpleExtension, { - id: 'child-node', - attachTo: { id: 'root-node', input: 'test' }, - }), - ]); - - const childNode = tree.nodes.get('child-node'); - expect(childNode).toBeDefined(); - - expect(tree.root.instance).not.toBeDefined(); - expect(childNode?.instance).not.toBeDefined(); - instantiateAppNodeTree(tree.root, testApis); - expect(tree.root.instance).toBeDefined(); - expect(childNode?.instance).toBeDefined(); - expect(tree.root.instance?.getData(inputMirrorDataRef)).toMatchObject({ - test: [ - { node: { spec: { id: 'child-node' } }, output: { test: 'test' } }, - ], - }); - - // Multiple calls should have no effect - instantiateAppNodeTree(tree.root, testApis); - expect(tree.root.instance).toBeDefined(); - expect(childNode?.instance).toBeDefined(); - }); - - it('should not instantiate disabled attachments', () => { - const tree = resolveAppTree('root-node', [ - { - ...makeSpec( + const tree = resolveAppTree( + 'root-node', + [ + makeSpec( createV1Extension({ namespace: 'root-node', attachTo: { id: 'ignored', input: 'ignored' }, @@ -234,22 +209,72 @@ describe('instantiateAppNodeTree', () => { }, }), ), - }, - { - ...makeSpec(simpleExtension), - id: 'child-node', - // Using an invalid input should not be an error when disabled - attachTo: { id: 'root-node', input: 'invalid' }, - disabled: true, - }, - ]); + makeSpec(simpleExtension, { + id: 'child-node', + attachTo: { id: 'root-node', input: 'test' }, + }), + ], + collector, + ); const childNode = tree.nodes.get('child-node'); expect(childNode).toBeDefined(); expect(tree.root.instance).not.toBeDefined(); expect(childNode?.instance).not.toBeDefined(); - instantiateAppNodeTree(tree.root, testApis); + instantiateAppNodeTree(tree.root, testApis, collector); + expect(tree.root.instance).toBeDefined(); + expect(childNode?.instance).toBeDefined(); + expect(tree.root.instance?.getData(inputMirrorDataRef)).toMatchObject({ + test: [ + { node: { spec: { id: 'child-node' } }, output: { test: 'test' } }, + ], + }); + + // Multiple calls should have no effect + instantiateAppNodeTree(tree.root, testApis, collector); + expect(tree.root.instance).toBeDefined(); + expect(childNode?.instance).toBeDefined(); + }); + + it('should not instantiate disabled attachments', () => { + const tree = resolveAppTree( + 'root-node', + [ + { + ...makeSpec( + createV1Extension({ + namespace: 'root-node', + attachTo: { id: 'ignored', input: 'ignored' }, + inputs: { + test: createV1ExtensionInput({ test: testDataRef }), + }, + output: { + inputMirror: inputMirrorDataRef, + }, + factory({ inputs }) { + return { inputMirror: inputs }; + }, + }), + ), + }, + { + ...makeSpec(simpleExtension), + id: 'child-node', + // Using an invalid input should not be an error when disabled + attachTo: { id: 'root-node', input: 'invalid' }, + disabled: true, + }, + ], + collector, + ); + + const childNode = tree.nodes.get('child-node'); + expect(childNode).toBeDefined(); + + expect(tree.root.instance).not.toBeDefined(); + expect(childNode?.instance).not.toBeDefined(); + instantiateAppNodeTree(tree.root, testApis, collector); expect(tree.root.instance).toBeDefined(); expect(childNode?.instance).not.toBeDefined(); expect(tree.root.instance?.getData(inputMirrorDataRef)).toEqual({ @@ -264,13 +289,14 @@ describe('instantiateAppNodeTree', () => { node: makeNode(simpleExtension), attachments, apis: testApis, + collector, }); - expect(Array.from(instance.getDataRefs())).toEqual([ + expect(Array.from(instance?.getDataRefs() ?? [])).toEqual([ testDataRef, otherDataRef.optional(), ]); - expect(instance.getData(testDataRef)).toEqual('test'); + expect(instance?.getData(testDataRef)).toEqual('test'); }); it('should create an extension with different kind of inputs', () => { @@ -346,12 +372,13 @@ describe('instantiateAppNodeTree', () => { }, }), ), + collector, }); - expect(Array.from(instance.getDataRefs())).toEqual([ + expect(Array.from(instance?.getDataRefs() ?? [])).toEqual([ inputMirrorDataRef, ]); - expect(instance.getData(inputMirrorDataRef)).toMatchObject({ + expect(instance?.getData(inputMirrorDataRef)).toMatchObject({ optionalSingletonPresent: { node: { spec: { id: 'app/test' } }, output: { test: 'optionalSingletonPresent' }, @@ -371,118 +398,158 @@ describe('instantiateAppNodeTree', () => { }); it('should refuse to create an extension with invalid config', () => { - expect(() => + const node = makeNode(simpleExtension, { + config: { other: 'not-a-number' }, + }); + expect( createAppNodeInstance({ apis: testApis, - node: makeNode(simpleExtension, { - config: { other: 'not-a-number' }, - }), + node, attachments: new Map(), + collector, }), - ).toThrow( - "Invalid configuration for extension 'app/test'; caused by Error: Expected number, received string at 'other'", - ); + ).toBeUndefined(); + expect(collector.collectErrors()).toEqual([ + { + code: 'INVALID_CONFIGURATION', + message: + "Invalid configuration for extension 'app/test'; caused by Error: Expected number, received string at 'other'", + context: { node }, + }, + ]); }); it('should forward extension factory errors', () => { - expect(() => + const node = makeNode( + createV1Extension({ + namespace: 'app', + name: 'test', + attachTo: { id: 'ignored', input: 'ignored' }, + output: {}, + factory() { + const error = new Error('NOPE'); + error.name = 'NopeError'; + throw error; + }, + }), + ); + expect( createAppNodeInstance({ apis: testApis, - node: makeNode( - createV1Extension({ - namespace: 'app', - name: 'test', - attachTo: { id: 'ignored', input: 'ignored' }, - output: {}, - factory() { - const error = new Error('NOPE'); - error.name = 'NopeError'; - throw error; - }, - }), - ), + node, attachments: new Map(), + collector, }), - ).toThrow( - "Failed to instantiate extension 'app/test'; caused by NopeError: NOPE", - ); + ).toBeUndefined(); + expect(collector.collectErrors()).toEqual([ + { + code: 'FAILED_TO_INSTANTIATE_EXTENSION', + message: + "Failed to instantiate extension 'app/test'; caused by NopeError: NOPE", + context: { node }, + }, + ]); }); it('should refuse to create an instance with duplicate output', () => { - expect(() => + const node = makeNode( + createV1Extension({ + namespace: 'app', + name: 'test', + attachTo: { id: 'ignored', input: 'ignored' }, + output: { + test1: testDataRef, + test2: testDataRef, + }, + factory({}) { + return { test1: 'test', test2: 'test2' }; + }, + }), + ); + expect( createAppNodeInstance({ apis: testApis, - node: makeNode( - createV1Extension({ - namespace: 'app', - name: 'test', - attachTo: { id: 'ignored', input: 'ignored' }, - output: { - test1: testDataRef, - test2: testDataRef, - }, - factory({}) { - return { test1: 'test', test2: 'test2' }; - }, - }), - ), + node, attachments: new Map(), + collector, }), - ).toThrow( - "Failed to instantiate extension 'app/test', duplicate extension data 'test' received via output 'test2'", - ); + ).toBeUndefined(); + expect(collector.collectErrors()).toEqual([ + { + code: 'FAILED_TO_INSTANTIATE_EXTENSION', + message: + "Failed to instantiate extension 'app/test', duplicate extension data 'test' received via output 'test2'", + context: { node }, + }, + ]); }); it('should refuse to create an instance with disconnected output data', () => { - expect(() => + const node = makeNode( + createV1Extension({ + namespace: 'app', + name: 'test', + attachTo: { id: 'ignored', input: 'ignored' }, + output: { + test: testDataRef, + }, + factory({}) { + return { nonexistent: 'test' } as any; + }, + }), + ); + expect( createAppNodeInstance({ apis: testApis, - node: makeNode( - createV1Extension({ - namespace: 'app', - name: 'test', - attachTo: { id: 'ignored', input: 'ignored' }, - output: { - test: testDataRef, - }, - factory({}) { - return { nonexistent: 'test' } as any; - }, - }), - ), + node, attachments: new Map(), + collector, }), - ).toThrow( - "Failed to instantiate extension 'app/test', unknown output provided via 'nonexistent'", - ); + ).toBeUndefined(); + expect(collector.collectErrors()).toEqual([ + { + code: 'FAILED_TO_INSTANTIATE_EXTENSION', + message: + "Failed to instantiate extension 'app/test', unknown output provided via 'nonexistent'", + context: { node }, + }, + ]); }); it('should refuse to create an instance with missing required input', () => { - expect(() => + const node = makeNode( + createV1Extension({ + namespace: 'app', + name: 'test', + attachTo: { id: 'ignored', input: 'ignored' }, + inputs: { + singleton: createV1ExtensionInput( + { + test: testDataRef, + }, + { singleton: true }, + ), + }, + output: {}, + factory: () => ({}), + }), + ); + expect( createAppNodeInstance({ apis: testApis, - node: makeNode( - createV1Extension({ - namespace: 'app', - name: 'test', - attachTo: { id: 'ignored', input: 'ignored' }, - inputs: { - singleton: createV1ExtensionInput( - { - test: testDataRef, - }, - { singleton: true }, - ), - }, - output: {}, - factory: () => ({}), - }), - ), + node, attachments: new Map(), + collector, }), - ).toThrow( - "Failed to instantiate extension 'app/test', input 'singleton' is required but was not received", - ); + ).toBeUndefined(); + expect(collector.collectErrors()).toEqual([ + { + code: 'FAILED_TO_INSTANTIATE_EXTENSION', + message: + "Failed to instantiate extension 'app/test', input 'singleton' is required but was not received", + context: { node }, + }, + ]); }); it('should warn when creating an instance with undeclared inputs', () => { @@ -521,6 +588,7 @@ describe('instantiateAppNodeTree', () => { factory: () => ({}), }), ), + collector, }), ); @@ -555,6 +623,7 @@ describe('instantiateAppNodeTree', () => { factory: () => ({}), }), ), + collector, }), ); @@ -565,7 +634,24 @@ describe('instantiateAppNodeTree', () => { }); it('should refuse to create an instance with multiple inputs for required singleton', () => { - expect(() => + const node = makeNode( + createV1Extension({ + namespace: 'app', + name: 'test', + attachTo: { id: 'ignored', input: 'ignored' }, + inputs: { + singleton: createV1ExtensionInput( + { + test: testDataRef, + }, + { singleton: true }, + ), + }, + output: {}, + factory: () => ({}), + }), + ); + expect( createAppNodeInstance({ apis: testApis, attachments: new Map([ @@ -577,31 +663,39 @@ describe('instantiateAppNodeTree', () => { ], ], ]), - node: makeNode( - createV1Extension({ - namespace: 'app', - name: 'test', - attachTo: { id: 'ignored', input: 'ignored' }, - inputs: { - singleton: createV1ExtensionInput( - { - test: testDataRef, - }, - { singleton: true }, - ), - }, - output: {}, - factory: () => ({}), - }), - ), + node, + collector, }), - ).toThrow( - "Failed to instantiate extension 'app/test', expected exactly one 'singleton' input but received multiple: 'app/test', 'app/test'", - ); + ).toBeUndefined(); + expect(collector.collectErrors()).toEqual([ + { + code: 'FAILED_TO_INSTANTIATE_EXTENSION', + message: + "Failed to instantiate extension 'app/test', expected exactly one 'singleton' input but received multiple: 'app/test', 'app/test'", + context: { node }, + }, + ]); }); it('should refuse to create an instance with multiple inputs for optional singleton', () => { - expect(() => + const node = makeNode( + createV1Extension({ + namespace: 'app', + name: 'test', + attachTo: { id: 'ignored', input: 'ignored' }, + inputs: { + singleton: createV1ExtensionInput( + { + test: testDataRef, + }, + { singleton: true, optional: true }, + ), + }, + output: {}, + factory: () => ({}), + }), + ); + expect( createAppNodeInstance({ apis: testApis, attachments: new Map([ @@ -613,57 +707,56 @@ describe('instantiateAppNodeTree', () => { ], ], ]), - node: makeNode( - createV1Extension({ - namespace: 'app', - name: 'test', - attachTo: { id: 'ignored', input: 'ignored' }, - inputs: { - singleton: createV1ExtensionInput( - { - test: testDataRef, - }, - { singleton: true, optional: true }, - ), - }, - output: {}, - factory: () => ({}), - }), - ), + node, + collector, }), - ).toThrow( - "Failed to instantiate extension 'app/test', expected at most one 'singleton' input but received multiple: 'app/test', 'app/test'", - ); + ).toBeUndefined(); + expect(collector.collectErrors()).toEqual([ + { + code: 'FAILED_TO_INSTANTIATE_EXTENSION', + message: + "Failed to instantiate extension 'app/test', expected at most one 'singleton' input but received multiple: 'app/test', 'app/test'", + context: { node }, + }, + ]); }); it('should refuse to create an instance with multiple inputs that did not provide required data', () => { - expect(() => + const node = makeNode( + createV1Extension({ + namespace: 'app', + name: 'test', + attachTo: { id: 'ignored', input: 'ignored' }, + inputs: { + singleton: createV1ExtensionInput( + { + other: otherDataRef, + }, + { singleton: true }, + ), + }, + output: {}, + factory: () => ({}), + }), + ); + expect( createAppNodeInstance({ apis: testApis, attachments: new Map([ ['singleton', [makeInstanceWithId(simpleExtension, undefined)]], ]), - node: makeNode( - createV1Extension({ - namespace: 'app', - name: 'test', - attachTo: { id: 'ignored', input: 'ignored' }, - inputs: { - singleton: createV1ExtensionInput( - { - other: otherDataRef, - }, - { singleton: true }, - ), - }, - output: {}, - factory: () => ({}), - }), - ), + node, + collector, }), - ).toThrowErrorMatchingInlineSnapshot( - `"Failed to instantiate extension 'app/test', extension 'app/test' could not be attached because its output data ('test', 'other') does not match what the input 'singleton' requires ('other')"`, - ); + ).toBeUndefined(); + expect(collector.collectErrors()).toEqual([ + { + code: 'FAILED_TO_INSTANTIATE_EXTENSION', + message: + "Failed to instantiate extension 'app/test', extension 'app/test' could not be attached because its output data ('test', 'other') does not match what the input 'singleton' requires ('other')", + context: { node }, + }, + ]); }); }); }); @@ -727,71 +820,37 @@ describe('instantiateAppNodeTree', () => { } it('should instantiate a single node', () => { - const tree = resolveAppTree('root-node', [ - makeSpec(simpleExtension, { id: 'root-node' }), - ]); + const tree = resolveAppTree( + 'root-node', + [makeSpec(simpleExtension, { id: 'root-node' })], + collector, + ); expect(tree.root.instance).not.toBeDefined(); - instantiateAppNodeTree(tree.root, testApis); + instantiateAppNodeTree(tree.root, testApis, collector); expect(tree.root.instance).toBeDefined(); expect(tree.root.instance?.getData(testDataRef)).toBe('test'); // Multiple calls should have no effect - instantiateAppNodeTree(tree.root, testApis); + instantiateAppNodeTree(tree.root, testApis, collector); expect(tree.root.instance).toBeDefined(); }); it('should not instantiate disabled nodes', () => { - const tree = resolveAppTree('root-node', [ - makeSpec(simpleExtension, { id: 'root-node', disabled: true }), - ]); + const tree = resolveAppTree( + 'root-node', + [makeSpec(simpleExtension, { id: 'root-node', disabled: true })], + collector, + ); expect(tree.root.instance).not.toBeDefined(); - instantiateAppNodeTree(tree.root, testApis); + instantiateAppNodeTree(tree.root, testApis, collector); expect(tree.root.instance).not.toBeDefined(); }); it('should instantiate a node with attachments', () => { - const tree = resolveAppTree('root-node', [ - makeSpec( - resolveExtensionDefinition( - createExtension({ - attachTo: { id: 'ignored', input: 'ignored' }, - inputs: { - test: createExtensionInput([testDataRef]), - }, - output: [inputMirrorDataRef], - factory: mirrorInputs, - }), - { namespace: 'root-node' }, - ), - ), - makeSpec(simpleExtension, { - id: 'child-node', - attachTo: { id: 'root-node', input: 'test' }, - }), - ]); - - const childNode = tree.nodes.get('child-node'); - expect(childNode).toBeDefined(); - - expect(tree.root.instance).not.toBeDefined(); - expect(childNode?.instance).not.toBeDefined(); - instantiateAppNodeTree(tree.root, testApis); - expect(tree.root.instance).toBeDefined(); - expect(childNode?.instance).toBeDefined(); - expect(tree.root.instance?.getData(inputMirrorDataRef)).toMatchObject({ - test: [{ node: { spec: { id: 'child-node' } }, test: 'test' }], - }); - - // Multiple calls should have no effect - instantiateAppNodeTree(tree.root, testApis); - expect(tree.root.instance).toBeDefined(); - expect(childNode?.instance).toBeDefined(); - }); - - it('should not instantiate disabled attachments', () => { - const tree = resolveAppTree('root-node', [ - { - ...makeSpec( + const tree = resolveAppTree( + 'root-node', + [ + makeSpec( resolveExtensionDefinition( createExtension({ attachTo: { id: 'ignored', input: 'ignored' }, @@ -804,22 +863,68 @@ describe('instantiateAppNodeTree', () => { { namespace: 'root-node' }, ), ), - }, - { - ...makeSpec(simpleExtension), - id: 'child-node', - // Using an invalid input should not be an error when disabled - attachTo: { id: 'root-node', input: 'invalid' }, - disabled: true, - }, - ]); + makeSpec(simpleExtension, { + id: 'child-node', + attachTo: { id: 'root-node', input: 'test' }, + }), + ], + collector, + ); const childNode = tree.nodes.get('child-node'); expect(childNode).toBeDefined(); expect(tree.root.instance).not.toBeDefined(); expect(childNode?.instance).not.toBeDefined(); - instantiateAppNodeTree(tree.root, testApis); + instantiateAppNodeTree(tree.root, testApis, collector); + expect(tree.root.instance).toBeDefined(); + expect(childNode?.instance).toBeDefined(); + expect(tree.root.instance?.getData(inputMirrorDataRef)).toMatchObject({ + test: [{ node: { spec: { id: 'child-node' } }, test: 'test' }], + }); + + // Multiple calls should have no effect + instantiateAppNodeTree(tree.root, testApis, collector); + expect(tree.root.instance).toBeDefined(); + expect(childNode?.instance).toBeDefined(); + }); + + it('should not instantiate disabled attachments', () => { + const tree = resolveAppTree( + 'root-node', + [ + { + ...makeSpec( + resolveExtensionDefinition( + createExtension({ + attachTo: { id: 'ignored', input: 'ignored' }, + inputs: { + test: createExtensionInput([testDataRef]), + }, + output: [inputMirrorDataRef], + factory: mirrorInputs, + }), + { namespace: 'root-node' }, + ), + ), + }, + { + ...makeSpec(simpleExtension), + id: 'child-node', + // Using an invalid input should not be an error when disabled + attachTo: { id: 'root-node', input: 'invalid' }, + disabled: true, + }, + ], + collector, + ); + + const childNode = tree.nodes.get('child-node'); + expect(childNode).toBeDefined(); + + expect(tree.root.instance).not.toBeDefined(); + expect(childNode?.instance).not.toBeDefined(); + instantiateAppNodeTree(tree.root, testApis, collector); expect(tree.root.instance).toBeDefined(); expect(childNode?.instance).not.toBeDefined(); expect(tree.root.instance?.getData(inputMirrorDataRef)).toEqual({ @@ -834,10 +939,13 @@ describe('instantiateAppNodeTree', () => { node: makeNode(simpleExtension), attachments, apis: testApis, + collector, }); - expect(Array.from(instance.getDataRefs())).toEqual([testDataRef]); - expect(instance.getData(testDataRef)).toEqual('test'); + expect(Array.from(instance?.getDataRefs() ?? [])).toEqual([ + testDataRef, + ]); + expect(instance?.getData(testDataRef)).toEqual('test'); }); it('should create an extension with different kind of inputs', () => { @@ -903,12 +1011,13 @@ describe('instantiateAppNodeTree', () => { { namespace: 'app' }, ), ), + collector, }); - expect(Array.from(instance.getDataRefs())).toEqual([ + expect(Array.from(instance?.getDataRefs() ?? [])).toEqual([ inputMirrorDataRef, ]); - expect(instance.getData(inputMirrorDataRef)).toMatchObject({ + expect(instance?.getData(inputMirrorDataRef)).toMatchObject({ optionalSingletonPresent: { node: { spec: { id: 'app/test' } }, test: 'optionalSingletonPresent', @@ -941,24 +1050,35 @@ describe('instantiateAppNodeTree', () => { }); yield* output; }, + collector, }); - expect(Array.from(instance.getDataRefs())).toEqual([testDataRef]); - expect(instance.getData(testDataRef)).toEqual('modified'); + expect(Array.from(instance?.getDataRefs() ?? [])).toEqual([ + testDataRef, + ]); + expect(instance?.getData(testDataRef)).toEqual('modified'); }); it('should refuse to create an extension with invalid config', () => { - expect(() => + const node = makeNode(simpleExtension, { + config: { other: 'not-a-number' }, + }); + expect( createAppNodeInstance({ apis: testApis, - node: makeNode(simpleExtension, { - config: { other: 'not-a-number' }, - }), + node, attachments: new Map(), + collector, }), - ).toThrow( - "Invalid configuration for extension 'app/test'; caused by Error: Expected number, received string at 'other'", - ); + ).toBeUndefined(); + expect(collector.collectErrors()).toEqual([ + { + code: 'INVALID_CONFIGURATION', + message: + "Invalid configuration for extension 'app/test'; caused by Error: Expected number, received string at 'other'", + context: { node }, + }, + ]); }); it('should throw if extension factories do not provide an iterable object', () => { @@ -966,14 +1086,21 @@ describe('instantiateAppNodeTree', () => { extension: ExtensionDefinition, middleware?: ExtensionFactoryMiddleware, ) { - return createAppNodeInstance({ + const node = makeNode( + resolveExtensionDefinition(extension, { namespace: 'test' }), + ); + createAppNodeInstance({ extensionFactoryMiddleware: middleware, apis: testApis, - node: makeNode( - resolveExtensionDefinition(extension, { namespace: 'test' }), - ), + node, attachments: new Map(), + collector, }); + const errors = collector.collectErrors(); + for (const error of errors ?? []) { + expect(error.context?.node).toBe(node); + } + return errors; } const baseOpts = { @@ -984,7 +1111,7 @@ describe('instantiateAppNodeTree', () => { const badFactory = () => 'not-iterable' as any; const goodFactory = () => [testDataRef('test')]; - expect(() => + expect( createInstance( createExtension({ attachTo: { id: 'ignored', input: 'ignored' }, @@ -992,11 +1119,15 @@ describe('instantiateAppNodeTree', () => { factory: badFactory, }), ), - ).toThrow( - `Failed to instantiate extension 'test', extension factory did not provide an iterable object`, - ); + ).toEqual([ + { + code: 'EXTENSION_FACTORY_INVALID_OUTPUT', + message: 'extension factory did not provide an iterable object', + context: { node: expect.anything() }, + }, + ]); - expect(() => + expect( createInstance( createExtension({ ...baseOpts, @@ -1005,12 +1136,18 @@ describe('instantiateAppNodeTree', () => { factory: badFactory, }), ), - ).toThrow( - `Failed to instantiate extension 'test', extension factory override did not provide an iterable object`, - ); + ).toEqual([ + { + // TODO: This should probably be EXTENSION_FACTORY_INVALID_OUTPUT + code: 'FAILED_TO_INSTANTIATE_EXTENSION', + message: + "Failed to instantiate extension 'test', extension factory override did not provide an iterable object", + context: { node: expect.anything() }, + }, + ]); // Bad middleware - expect(() => + expect( createInstance( createExtension({ ...baseOpts, @@ -1018,11 +1155,17 @@ describe('instantiateAppNodeTree', () => { }), () => 'not-iterable' as any, ), - ).toThrow( - `Failed to instantiate extension 'test', extension factory middleware did not provide an iterable object`, - ); + ).toEqual([ + { + // TODO: This should probably be EXTENSION_FACTORY_INVALID_OUTPUT + code: 'FAILED_TO_INSTANTIATE_EXTENSION', + message: + "Failed to instantiate extension 'test', extension factory middleware did not provide an iterable object", + context: { node: expect.anything() }, + }, + ]); - expect(() => + expect( createInstance( createExtensionBlueprint({ kind: 'test', @@ -1030,12 +1173,16 @@ describe('instantiateAppNodeTree', () => { factory: badFactory, }).make({ params: {} }), ), - ).toThrow( - `Failed to instantiate extension 'test:test', extension factory did not provide an iterable object`, - ); + ).toEqual([ + { + code: 'EXTENSION_FACTORY_INVALID_OUTPUT', + message: 'extension factory did not provide an iterable object', + context: { node: expect.anything() }, + }, + ]); // Using makeWithOverrides - expect(() => + expect( createInstance( createExtensionBlueprint({ kind: 'test', @@ -1045,12 +1192,16 @@ describe('instantiateAppNodeTree', () => { factory: badFactory, }), ), - ).toThrow( - `Failed to instantiate extension 'test:test', extension factory did not provide an iterable object`, - ); + ).toEqual([ + { + code: 'EXTENSION_FACTORY_INVALID_OUTPUT', + message: 'extension factory did not provide an iterable object', + context: { node: expect.anything() }, + }, + ]); // Using makeWithOverrides and factory middleware - expect(() => + expect( createInstance( createExtensionBlueprint({ kind: 'test', @@ -1061,12 +1212,18 @@ describe('instantiateAppNodeTree', () => { }), orig => orig(), ), - ).toThrow( - `Failed to instantiate extension 'test:test', extension factory did not provide an iterable object`, - ); + ).toEqual([ + { + // TODO: This should probably be EXTENSION_FACTORY_INVALID_OUTPUT + code: 'FAILED_TO_INSTANTIATE_EXTENSION', + message: + "Failed to instantiate extension 'test:test', extension factory did not provide an iterable object", + context: { node: expect.anything() }, + }, + ]); // Using makeWithOverrides and factory middleware - expect(() => + expect( createInstance( createExtensionBlueprint({ kind: 'test', @@ -1077,135 +1234,181 @@ describe('instantiateAppNodeTree', () => { }), orig => orig(), ), - ).toThrow( - `Failed to instantiate extension 'test:test', original blueprint factory did not provide an iterable object`, - ); + ).toEqual([ + { + // TODO: This should probably be EXTENSION_FACTORY_INVALID_OUTPUT + code: 'FAILED_TO_INSTANTIATE_EXTENSION', + message: + "Failed to instantiate extension 'test:test', original blueprint factory did not provide an iterable object", + context: { node: expect.anything() }, + }, + ]); }); it('should forward extension factory errors', () => { - expect(() => + const node = makeNode( + resolveExtensionDefinition( + createExtension({ + name: 'test', + attachTo: { id: 'ignored', input: 'ignored' }, + output: [testDataRef], + factory() { + const error = new Error('NOPE'); + error.name = 'NopeError'; + throw error; + }, + }), + { namespace: 'app' }, + ), + ); + expect( createAppNodeInstance({ apis: testApis, - node: makeNode( - resolveExtensionDefinition( - createExtension({ - name: 'test', - attachTo: { id: 'ignored', input: 'ignored' }, - output: [testDataRef], - factory() { - const error = new Error('NOPE'); - error.name = 'NopeError'; - throw error; - }, - }), - { namespace: 'app' }, - ), - ), + node, attachments: new Map(), + collector, }), - ).toThrow( - "Failed to instantiate extension 'app/test'; caused by NopeError: NOPE", - ); + ).toBeUndefined(); + expect(collector.collectErrors()).toEqual([ + { + code: 'FAILED_TO_INSTANTIATE_EXTENSION', + message: + "Failed to instantiate extension 'app/test'; caused by NopeError: NOPE", + context: { node }, + }, + ]); }); it('should refuse to create an instance with duplicate output', () => { - expect(() => + const node = makeNode( + resolveExtensionDefinition( + createExtension({ + name: 'test', + attachTo: { id: 'ignored', input: 'ignored' }, + output: [testDataRef, testDataRef], + factory({}) { + return [testDataRef('test'), testDataRef('test2')]; + }, + }), + { namespace: 'app' }, + ), + ); + expect( createAppNodeInstance({ apis: testApis, - node: makeNode( - resolveExtensionDefinition( - createExtension({ - name: 'test', - attachTo: { id: 'ignored', input: 'ignored' }, - output: [testDataRef, testDataRef], - factory({}) { - return [testDataRef('test'), testDataRef('test2')]; - }, - }), - { namespace: 'app' }, - ), - ), + node, attachments: new Map(), + collector, }), - ).toThrow( - "Failed to instantiate extension 'app/test', duplicate extension data output 'test'", - ); + ).toBeUndefined(); + expect(collector.collectErrors()).toEqual([ + { + code: 'EXTENSION_FACTORY_DUPLICATE_OUTPUT', + message: "extension factory output duplicate data 'test'", + context: { dataRefId: 'test', node }, + }, + ]); }); it('should refuse to create an instance without required', () => { - expect(() => + const node = makeNode( + resolveExtensionDefinition( + createExtension({ + name: 'test', + attachTo: { id: 'ignored', input: 'ignored' }, + output: [testDataRef], + factory({}) { + return [] as any; + }, + }), + { namespace: 'app' }, + ), + ); + + expect( createAppNodeInstance({ apis: testApis, - node: makeNode( - resolveExtensionDefinition( - createExtension({ - name: 'test', - attachTo: { id: 'ignored', input: 'ignored' }, - output: [testDataRef], - factory({}) { - return [] as any; - }, - }), - { namespace: 'app' }, - ), - ), + node, attachments: new Map(), + collector, }), - ).toThrow( - "Failed to instantiate extension 'app/test', missing required extension data output 'test'", - ); + ).toBeUndefined(); + expect(collector.collectErrors()).toEqual([ + { + code: 'EXTENSION_FACTORY_MISSING_REQUIRED_OUTPUT', + message: "missing required extension data output 'test'", + context: { + dataRefId: 'test', + node, + }, + }, + ]); }); it('should refuse to create an instance with unknown output data', () => { - expect(() => + const node = makeNode( + resolveExtensionDefinition( + // @ts-expect-error + createExtension({ + name: 'test', + attachTo: { id: 'ignored', input: 'ignored' }, + output: [], // Output not declared + factory({}) { + return [testDataRef('test')] as any; + }, + }), + { namespace: 'app' }, + ), + ); + expect( createAppNodeInstance({ apis: testApis, - node: makeNode( - resolveExtensionDefinition( - // @ts-expect-error - createExtension({ - name: 'test', - attachTo: { id: 'ignored', input: 'ignored' }, - output: [], // Output not declared - factory({}) { - return [testDataRef('test')] as any; - }, - }), - { namespace: 'app' }, - ), - ), + node, attachments: new Map(), + collector, }), - ).toThrow( - "Failed to instantiate extension 'app/test', unexpected output 'test'", - ); + ).toBeDefined(); + expect(collector.collectErrors()).toEqual([ + { + code: 'EXTENSION_FACTORY_UNEXPECTED_OUTPUT', + message: "unexpected output 'test'", + context: { dataRefId: 'test', node }, + }, + ]); }); it('should refuse to create an instance with missing required input', () => { - expect(() => + const node = makeNode( + resolveExtensionDefinition( + createExtension({ + name: 'test', + attachTo: { id: 'ignored', input: 'ignored' }, + inputs: { + singleton: createExtensionInput([testDataRef], { + singleton: true, + }), + }, + output: [], + factory: () => [], + }), + { namespace: 'app' }, + ), + ); + expect( createAppNodeInstance({ apis: testApis, - node: makeNode( - resolveExtensionDefinition( - createExtension({ - name: 'test', - attachTo: { id: 'ignored', input: 'ignored' }, - inputs: { - singleton: createExtensionInput([testDataRef], { - singleton: true, - }), - }, - output: [], - factory: () => [], - }), - { namespace: 'app' }, - ), - ), + node, attachments: new Map(), + collector, }), - ).toThrow( - "Failed to instantiate extension 'app/test', input 'singleton' is required but was not received", - ); + ).toBeUndefined(); + expect(collector.collectErrors()).toEqual([ + { + code: 'EXTENSION_MISSING_REQUIRED_INPUT', + message: "input 'singleton' is required but was not received", + context: { inputName: 'singleton', node }, + }, + ]); }); it('should warn when creating an instance with undeclared inputs', () => { @@ -1244,6 +1447,7 @@ describe('instantiateAppNodeTree', () => { { namespace: 'app' }, ), ), + collector, }), ); @@ -1280,6 +1484,7 @@ describe('instantiateAppNodeTree', () => { { namespace: 'app' }, ), ), + collector, }), ); @@ -1290,7 +1495,23 @@ describe('instantiateAppNodeTree', () => { }); it('should refuse to create an instance with multiple inputs for required singleton', () => { - expect(() => + const node = makeNode( + resolveExtensionDefinition( + createExtension({ + name: 'test', + attachTo: { id: 'ignored', input: 'ignored' }, + inputs: { + singleton: createExtensionInput([testDataRef], { + singleton: true, + }), + }, + output: [], + factory: () => [], + }), + { namespace: 'app' }, + ), + ); + expect( createAppNodeInstance({ apis: testApis, attachments: new Map([ @@ -1302,30 +1523,39 @@ describe('instantiateAppNodeTree', () => { ], ], ]), - node: makeNode( - resolveExtensionDefinition( - createExtension({ - name: 'test', - attachTo: { id: 'ignored', input: 'ignored' }, - inputs: { - singleton: createExtensionInput([testDataRef], { - singleton: true, - }), - }, - output: [], - factory: () => [], - }), - { namespace: 'app' }, - ), - ), + node, + collector, }), - ).toThrow( - "Failed to instantiate extension 'app/test', expected exactly one 'singleton' input but received multiple: 'app/test', 'app/test'", - ); + ).toBeUndefined(); + expect(collector.collectErrors()).toEqual([ + { + code: 'EXTENSION_TOO_MANY_ATTACHMENTS', + message: + "expected exactly one 'singleton' input but received multiple: 'app/test', 'app/test'", + context: { inputName: 'singleton', node }, + }, + ]); }); it('should refuse to create an instance with multiple inputs for optional singleton', () => { - expect(() => + const node = makeNode( + resolveExtensionDefinition( + createExtension({ + name: 'test', + attachTo: { id: 'ignored', input: 'ignored' }, + inputs: { + singleton: createExtensionInput([testDataRef], { + singleton: true, + optional: true, + }), + }, + output: [], + factory: () => [], + }), + { namespace: 'app' }, + ), + ); + expect( createAppNodeInstance({ apis: testApis, attachments: new Map([ @@ -1337,56 +1567,55 @@ describe('instantiateAppNodeTree', () => { ], ], ]), - node: makeNode( - resolveExtensionDefinition( - createExtension({ - name: 'test', - attachTo: { id: 'ignored', input: 'ignored' }, - inputs: { - singleton: createExtensionInput([testDataRef], { - singleton: true, - optional: true, - }), - }, - output: [], - factory: () => [], - }), - { namespace: 'app' }, - ), - ), + node, + collector, }), - ).toThrow( - "Failed to instantiate extension 'app/test', expected at most one 'singleton' input but received multiple: 'app/test', 'app/test'", - ); + ).toBeUndefined(); + expect(collector.collectErrors()).toEqual([ + { + code: 'EXTENSION_TOO_MANY_ATTACHMENTS', + message: + "expected at most one 'singleton' input but received multiple: 'app/test', 'app/test'", + context: { inputName: 'singleton', node }, + }, + ]); }); it('should refuse to create an instance with multiple inputs that did not provide required data', () => { - expect(() => + const node = makeNode( + resolveExtensionDefinition( + createExtension({ + name: 'test', + attachTo: { id: 'ignored', input: 'ignored' }, + inputs: { + singleton: createExtensionInput([otherDataRef], { + singleton: true, + }), + }, + output: [], + factory: () => [], + }), + { namespace: 'app' }, + ), + ); + expect( createAppNodeInstance({ apis: testApis, attachments: new Map([ ['singleton', [makeInstanceWithId(simpleExtension, undefined)]], ]), - node: makeNode( - resolveExtensionDefinition( - createExtension({ - name: 'test', - attachTo: { id: 'ignored', input: 'ignored' }, - inputs: { - singleton: createExtensionInput([otherDataRef], { - singleton: true, - }), - }, - output: [], - factory: () => [], - }), - { namespace: 'app' }, - ), - ), + node, + collector, }), - ).toThrowErrorMatchingInlineSnapshot( - `"Failed to instantiate extension 'app/test', extension 'app/test' could not be attached because its output data ('test') does not match what the input 'singleton' requires ('other')"`, - ); + ).toBeUndefined(); + expect(collector.collectErrors()).toEqual([ + { + code: 'EXTENSION_MISSING_INPUT_DATA', + message: + "extension 'app/test' could not be attached because its output data ('test') does not match what the input 'singleton' requires ('other')", + context: { node }, + }, + ]); }); }); }); diff --git a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts index 2c917ed5d4..a33c5943a1 100644 --- a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts +++ b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts @@ -27,6 +27,7 @@ import { AppNode, AppNodeInstance } from '@backstage/frontend-plugin-api'; // eslint-disable-next-line @backstage/no-relative-monorepo-imports import { toInternalExtension } from '../../../frontend-plugin-api/src/wiring/resolveExtensionDefinition'; import { createExtensionDataContainer } from '@internal/frontend'; +import { ErrorCollector } from '../wiring/createErrorCollector'; type Mutable = { -readonly [P in keyof T]: T[P]; @@ -63,12 +64,18 @@ function resolveInputDataContainer( extensionData: Array, attachment: AppNode, inputName: string, -): { node: AppNode } & ExtensionDataContainer { + collector: ErrorCollector, +): ({ node: AppNode } & ExtensionDataContainer) | undefined { const dataMap = new Map(); + let failed = false; for (const ref of extensionData) { if (dataMap.has(ref.id)) { - throw new Error(`Unexpected duplicate input data '${ref.id}'`); + collector.report({ + code: 'EXTENSION_DUPLICATE_INPUT', + message: `Unexpected duplicate input data '${ref.id}'`, + }); + continue; } const value = attachment.instance?.getData(ref); if (value === undefined && !ref.config.optional) { @@ -81,14 +88,20 @@ function resolveInputDataContainer( .map(r => `'${r.id}'`) .join(', '); - throw new Error( - `extension '${attachment.spec.id}' could not be attached because its output data (${provided}) does not match what the input '${inputName}' requires (${expected})`, - ); + collector.report({ + code: 'EXTENSION_MISSING_INPUT_DATA', + message: `extension '${attachment.spec.id}' could not be attached because its output data (${provided}) does not match what the input '${inputName}' requires (${expected})`, + }); + failed = true; } dataMap.set(ref.id, value); } + if (failed) { + return undefined; + } + return { node: attachment, get(ref) { @@ -198,42 +211,79 @@ function resolveV2Inputs( >; }, attachments: ReadonlyMap, -): ResolvedExtensionInputs<{ - [inputName in string]: ExtensionInput< - ExtensionDataRef, - { optional: boolean; singleton: boolean } - >; -}> { - return mapValues(inputMap, (input, inputName) => { + collector: ErrorCollector, +): + | undefined + | ResolvedExtensionInputs<{ + [inputName in string]: ExtensionInput< + ExtensionDataRef, + { optional: boolean; singleton: boolean } + >; + }> { + let failed = false; + const resolvedInputs = mapValues(inputMap, (input, inputName) => { const attachedNodes = attachments.get(inputName) ?? []; if (input.config.singleton) { if (attachedNodes.length > 1) { - const attachedNodeIds = attachedNodes.map(e => e.spec.id); - throw Error( - `expected ${ + const attachedNodeIds = attachedNodes.map(e => e.spec.id).join("', '"); + collector.report({ + code: 'EXTENSION_TOO_MANY_ATTACHMENTS', + message: `expected ${ input.config.optional ? 'at most' : 'exactly' - } one '${inputName}' input but received multiple: '${attachedNodeIds.join( - "', '", - )}'`, - ); + } one '${inputName}' input but received multiple: '${attachedNodeIds}'`, + context: { inputName }, + }); + failed = true; + return undefined; } else if (attachedNodes.length === 0) { - if (input.config.optional) { - return undefined; + if (!input.config.optional) { + collector.report({ + code: 'EXTENSION_MISSING_REQUIRED_INPUT', + message: `input '${inputName}' is required but was not received`, + context: { inputName }, + }); + failed = true; } - throw Error(`input '${inputName}' is required but was not received`); + return undefined; } - return resolveInputDataContainer( + const data = resolveInputDataContainer( input.extensionData, attachedNodes[0], inputName, + collector, ); + if (data === undefined) { + failed = true; + return undefined; + } + return data; } - return attachedNodes.map(attachment => - resolveInputDataContainer(input.extensionData, attachment, inputName), - ); - }) as ResolvedExtensionInputs<{ + if (failed) { + return undefined; + } + + return attachedNodes.map(attachment => { + const data = resolveInputDataContainer( + input.extensionData, + attachment, + inputName, + collector, + ); + if (data === undefined) { + failed = true; + return undefined; + } + return data; + }); + }); + + if (failed) { + return undefined; + } + + return resolvedInputs as ResolvedExtensionInputs<{ [inputName in string]: ExtensionInput< ExtensionDataRef, { optional: boolean; singleton: boolean } @@ -247,8 +297,10 @@ export function createAppNodeInstance(options: { node: AppNode; apis: ApiHolder; attachments: ReadonlyMap; -}): AppNodeInstance { + collector: ErrorCollector; +}): AppNodeInstance | undefined { const { node, apis, attachments } = options; + const collector = options.collector.child({ node }); const { id, extension, config } = node.spec; const extensionData = new Map(); const extensionDataRefs = new Set>(); @@ -259,9 +311,11 @@ export function createAppNodeInstance(options: { [x: string]: any; }; } catch (e) { - throw new Error( - `Invalid configuration for extension '${id}'; caused by ${e}`, - ); + collector.report({ + code: 'INVALID_CONFIGURATION', + message: `Invalid configuration for extension '${id}'; caused by ${e}`, + }); + return undefined; } try { @@ -293,11 +347,19 @@ export function createAppNodeInstance(options: { extensionDataRefs.add(ref); } } else if (internalExtension.version === 'v2') { + const inputs = resolveV2Inputs( + internalExtension.inputs, + attachments, + collector, + ); + if (inputs === undefined) { + return undefined; + } const context = { node, apis, config: parsedConfig, - inputs: resolveV2Inputs(internalExtension.inputs, attachments), + inputs, }; const outputDataValues = options.extensionFactoryMiddleware ? createExtensionDataContainer( @@ -320,15 +382,32 @@ export function createAppNodeInstance(options: { typeof outputDataValues !== 'object' || !outputDataValues?.[Symbol.iterator] ) { - throw new Error('extension factory did not provide an iterable object'); + collector.report({ + code: 'EXTENSION_FACTORY_INVALID_OUTPUT', + message: 'extension factory did not provide an iterable object', + }); + return undefined; } + let failed = false; + const outputDataMap = new Map(); for (const value of outputDataValues) { if (outputDataMap.has(value.id)) { - throw new Error(`duplicate extension data output '${value.id}'`); + collector.report({ + code: 'EXTENSION_FACTORY_DUPLICATE_OUTPUT', + message: `extension factory output duplicate data '${value.id}'`, + context: { + dataRefId: value.id, + }, + }); + failed = true; + } else { + outputDataMap.set(value.id, value.value); } - outputDataMap.set(value.id, value.value); + } + if (failed) { + return undefined; } for (const ref of internalExtension.output) { @@ -336,34 +415,53 @@ export function createAppNodeInstance(options: { outputDataMap.delete(ref.id); if (value === undefined) { if (!ref.config.optional) { - throw new Error( - `missing required extension data output '${ref.id}'`, - ); + collector.report({ + code: 'EXTENSION_FACTORY_MISSING_REQUIRED_OUTPUT', + message: `missing required extension data output '${ref.id}'`, + context: { + dataRefId: ref.id, + }, + }); + failed = true; } } else { extensionData.set(ref.id, value); extensionDataRefs.add(ref); } } + if (failed) { + return undefined; + } if (outputDataMap.size > 0) { - throw new Error( - `unexpected output '${Array.from(outputDataMap.keys()).join( - "', '", - )}'`, - ); + for (const dataRefId of outputDataMap.keys()) { + // TODO: Make this a warning + collector.report({ + code: 'EXTENSION_FACTORY_UNEXPECTED_OUTPUT', + message: `unexpected output '${dataRefId}'`, + context: { + dataRefId: dataRefId, + }, + }); + } } } else { - throw new Error( - `unexpected extension version '${(internalExtension as any).version}'`, - ); + collector.report({ + code: 'UNEXPECTED_EXTENSION_VERSION', + message: `unexpected extension version '${ + (internalExtension as any).version + }'`, + }); + return undefined; } } catch (e) { - throw new Error( - `Failed to instantiate extension '${id}'${ - e.name === 'Error' ? `, ${e.message}` : `; caused by ${e.stack}` + collector.report({ + code: 'FAILED_TO_INSTANTIATE_EXTENSION', + message: `Failed to instantiate extension '${id}'${ + e.name === 'Error' ? `, ${e.message}` : `; caused by ${e}` }`, - ); + }); + return undefined; } return { @@ -383,8 +481,9 @@ export function createAppNodeInstance(options: { export function instantiateAppNodeTree( rootNode: AppNode, apis: ApiHolder, + errors: ErrorCollector, extensionFactoryMiddleware?: ExtensionFactoryMiddleware, -): void { +): boolean { function createInstance(node: AppNode): AppNodeInstance | undefined { if (node.instance) { return node.instance; @@ -413,10 +512,11 @@ export function instantiateAppNodeTree( node, apis, attachments: instantiatedAttachments, + collector: errors, }); return node.instance; } - createInstance(rootNode); + return createInstance(rootNode) !== undefined; } diff --git a/packages/frontend-app-api/src/tree/resolveAppNodeSpecs.test.ts b/packages/frontend-app-api/src/tree/resolveAppNodeSpecs.test.ts index 83abcf3dc4..23217f697a 100644 --- a/packages/frontend-app-api/src/tree/resolveAppNodeSpecs.test.ts +++ b/packages/frontend-app-api/src/tree/resolveAppNodeSpecs.test.ts @@ -21,6 +21,18 @@ import { ExtensionDefinition, } from '@backstage/frontend-plugin-api'; import { resolveAppNodeSpecs } from './resolveAppNodeSpecs'; +import { createErrorCollector } from '../wiring/createErrorCollector'; + +const collector = createErrorCollector(); + +afterEach(() => { + const errors = collector.collectErrors(); + if (errors) { + throw new Error( + `Unexpected errors: ${errors.map(e => e.message).join(', ')}`, + ); + } +}); function makeExt( id: string, @@ -61,6 +73,7 @@ describe('resolveAppNodeSpecs', () => { features: [], builtinExtensions: [a], parameters: [], + collector, }), ).toEqual([ { @@ -83,6 +96,7 @@ describe('resolveAppNodeSpecs', () => { features: [], builtinExtensions: [a, b], parameters: [], + collector, }), ).toEqual([ { @@ -122,6 +136,7 @@ describe('resolveAppNodeSpecs', () => { attachTo: { id: 'derp', input: 'default' }, }, ], + collector, }), ).toEqual([ { @@ -169,6 +184,7 @@ describe('resolveAppNodeSpecs', () => { config: { foo: { qux: 3 } }, }, ], + collector, }), ).toEqual([ { @@ -209,6 +225,7 @@ describe('resolveAppNodeSpecs', () => { disabled: false, }, ], + collector, }), ).toEqual([ { @@ -249,6 +266,7 @@ describe('resolveAppNodeSpecs', () => { { id: 'd', disabled: false }, { id: 'c', disabled: false }, ], + collector, }), ).toEqual([ { @@ -341,6 +359,7 @@ describe('resolveAppNodeSpecs', () => { ], builtinExtensions: [], parameters: [], + collector, }), ).toEqual([ { @@ -395,6 +414,7 @@ describe('resolveAppNodeSpecs', () => { id, disabled: false, })), + collector, }); expect(result.map(r => r.extension.id)).toEqual([ @@ -405,53 +425,83 @@ describe('resolveAppNodeSpecs', () => { }); it('throws an error when a forbidden extension is overridden by a plugin', () => { - expect(() => - resolveAppNodeSpecs({ - features: [ - createFrontendPlugin({ - pluginId: 'test', - extensions: [makeExtDef('forbidden')], - }), - ], - builtinExtensions: [], - parameters: [], - forbidden: new Set(['test/forbidden']), - }), - ).toThrow( - "It is forbidden to override the following extension(s): 'test/forbidden', which is done by the following plugin(s): 'test'", - ); + const plugin = createFrontendPlugin({ + pluginId: 'test', + extensions: [makeExtDef('forbidden')], + }); + const result = resolveAppNodeSpecs({ + features: [plugin], + builtinExtensions: [], + parameters: [], + forbidden: new Set(['test/forbidden']), + collector, + }); + expect(result).toEqual([]); + + expect(collector.collectErrors()).toEqual([ + { + code: 'EXTENSION_FORBIDDEN', + message: + "It is forbidden to override the 'test/forbidden' extension, attempted by the 'test' plugin", + context: { + plugin, + extensionId: 'test/forbidden', + }, + }, + ]); }); it('throws an error when a forbidden extension is overridden by module', () => { - expect(() => - resolveAppNodeSpecs({ - features: [ - createFrontendPlugin({ - pluginId: 'forbidden', - extensions: [], - }), - createFrontendModule({ - pluginId: 'forbidden', - extensions: [makeExtDef()], - }), - ], - builtinExtensions: [], - parameters: [], - forbidden: new Set(['forbidden']), - }), - ).toThrow( - "It is forbidden to override the following extension(s): 'forbidden', which is done by a module for the following plugin(s): 'forbidden'", - ); + const plugin = createFrontendPlugin({ + pluginId: 'forbidden', + extensions: [], + }); + const result = resolveAppNodeSpecs({ + features: [ + plugin, + createFrontendModule({ + pluginId: 'forbidden', + extensions: [makeExtDef()], + }), + ], + builtinExtensions: [], + parameters: [], + forbidden: new Set(['forbidden']), + collector, + }); + expect(result).toEqual([]); + + expect(collector.collectErrors()).toEqual([ + { + code: 'EXTENSION_FORBIDDEN', + message: + "It is forbidden to override the 'forbidden' extension, attempted by the 'forbidden' plugin", + context: { + plugin, + extensionId: 'forbidden', + }, + }, + ]); }); it('throws an error when a forbidden extension is parametrized', () => { - expect(() => - resolveAppNodeSpecs({ - features: [], - builtinExtensions: [], - parameters: [{ id: 'forbidden', disabled: false }], - forbidden: new Set(['forbidden']), - }), - ).toThrow("Configuration of the 'forbidden' extension is forbidden"); + const result = resolveAppNodeSpecs({ + features: [], + builtinExtensions: [], + parameters: [{ id: 'forbidden', disabled: false }], + forbidden: new Set(['forbidden']), + collector, + }); + expect(result).toEqual([]); + + expect(collector.collectErrors()).toEqual([ + { + code: 'EXTENSION_CONFIG_FORBIDDEN', + message: "Configuration of the 'forbidden' extension is forbidden", + context: { + extensionId: 'forbidden', + }, + }, + ]); }); }); diff --git a/packages/frontend-app-api/src/tree/resolveAppNodeSpecs.ts b/packages/frontend-app-api/src/tree/resolveAppNodeSpecs.ts index f9ce7387c2..23a1847b43 100644 --- a/packages/frontend-app-api/src/tree/resolveAppNodeSpecs.ts +++ b/packages/frontend-app-api/src/tree/resolveAppNodeSpecs.ts @@ -18,6 +18,7 @@ import { createFrontendPlugin, Extension, FrontendFeature, + FrontendPlugin, } from '@backstage/frontend-plugin-api'; import { ExtensionParameters } from './readAppExtensionsConfig'; import { AppNodeSpec } from '@backstage/frontend-plugin-api'; @@ -29,6 +30,7 @@ import { } from '../../../frontend-plugin-api/src/wiring/createFrontendModule'; // eslint-disable-next-line @backstage/no-relative-monorepo-imports import { toInternalExtension } from '../../../frontend-plugin-api/src/wiring/resolveExtensionDefinition'; +import { ErrorCollector } from '../wiring/createErrorCollector'; /** @internal */ export function resolveAppNodeSpecs(options: { @@ -36,61 +38,58 @@ export function resolveAppNodeSpecs(options: { builtinExtensions?: Extension[]; parameters?: Array; forbidden?: Set; - allowUnknownExtensionConfig?: boolean; + collector: ErrorCollector; }): AppNodeSpec[] { const { builtinExtensions = [], parameters = [], forbidden = new Set(), features = [], - allowUnknownExtensionConfig = false, + collector, } = options; const plugins = features.filter(OpaqueFrontendPlugin.isType); const modules = features.filter(isInternalFrontendModule); + const filterForbidden = ( + extension: Extension & { plugin: FrontendPlugin }, + ) => { + if (forbidden.has(extension.id)) { + collector.report({ + code: 'EXTENSION_FORBIDDEN', + message: `It is forbidden to override the '${extension.id}' extension, attempted by the '${extension.plugin.id}' plugin`, + context: { + plugin: extension.plugin, + extensionId: extension.id, + }, + }); + return false; + } + return true; + }; + const pluginExtensions = plugins.flatMap(plugin => { - return OpaqueFrontendPlugin.toInternal(plugin).extensions.map( - extension => ({ + return OpaqueFrontendPlugin.toInternal(plugin) + .extensions.map(extension => ({ ...extension, plugin, - }), - ); + })) + .filter(filterForbidden); }); const moduleExtensions = modules.flatMap(mod => - toInternalFrontendModule(mod).extensions.flatMap(extension => { - // Modules for plugins that are not installed are ignored - const plugin = plugins.find(p => p.id === mod.pluginId); - if (!plugin) { - return []; - } + toInternalFrontendModule(mod) + .extensions.flatMap(extension => { + // Modules for plugins that are not installed are ignored + const plugin = plugins.find(p => p.id === mod.pluginId); + if (!plugin) { + return []; + } - return [{ ...extension, plugin }]; - }), + return [{ ...extension, plugin }]; + }) + .filter(filterForbidden), ); - // Prevent core override - if (pluginExtensions.some(({ id }) => forbidden.has(id))) { - const pluginsStr = pluginExtensions - .filter(({ id }) => forbidden.has(id)) - .map(({ plugin }) => `'${plugin.id}'`) - .join(', '); - const forbiddenStr = [...forbidden].map(id => `'${id}'`).join(', '); - throw new Error( - `It is forbidden to override the following extension(s): ${forbiddenStr}, which is done by the following plugin(s): ${pluginsStr}`, - ); - } - if (moduleExtensions.some(({ id }) => forbidden.has(id))) { - const pluginsStr = moduleExtensions - .filter(({ id }) => forbidden.has(id)) - .map(({ plugin }) => `'${plugin.id}'`) - .join(', '); - const forbiddenStr = [...forbidden].map(id => `'${id}'`).join(', '); - throw new Error( - `It is forbidden to override the following extension(s): ${forbiddenStr}, which is done by a module for the following plugin(s): ${pluginsStr}`, - ); - } - const appPlugin = plugins.find(plugin => plugin.id === 'app') ?? createFrontendPlugin({ @@ -154,52 +153,41 @@ export function resolveAppNodeSpecs(options: { } } - const duplicatedExtensionIds = new Set(); - const duplicatedExtensionData = configuredExtensions.reduce< - Record> - >((data, { extension, params }) => { - const extensionId = extension.id; - const extensionData = data?.[extensionId]; - if (extensionData) duplicatedExtensionIds.add(extensionId); - const pluginId = params.source?.id ?? 'internal'; - const pluginCount = extensionData?.[pluginId] ?? 0; - return { - ...data, - [extensionId]: { ...extensionData, [pluginId]: pluginCount + 1 }, - }; - }, {}); + const seendExtensionIds = new Set(); + const deduplicatedExtensions = configuredExtensions.filter( + ({ extension, params }) => { + if (seendExtensionIds.has(extension.id)) { + collector.report({ + code: 'EXTENSION_DUPLICATED', + message: `The extension '${extension.id}' is duplicated`, + context: { + plugin: params.plugin, + extensionId: extension.id, + }, + }); + return false; + } + seendExtensionIds.add(extension.id); + return true; + }, + ); - if (duplicatedExtensionIds.size > 0) { - throw new Error( - `The following extensions are duplicated: ${Array.from( - duplicatedExtensionIds, - ) - .map( - extensionId => - `The extension '${extensionId}' was provided ${Object.keys( - duplicatedExtensionData[extensionId], - ) - .map( - pluginId => - `${duplicatedExtensionData[extensionId][pluginId]} time(s) by the plugin '${pluginId}'`, - ) - .join(' and ')}`, - ) - .join(', ')}`, - ); - } - - const order = new Map(); + const order = new Map(); for (const overrideParam of parameters) { const extensionId = overrideParam.id; if (forbidden.has(extensionId)) { - throw new Error( - `Configuration of the '${extensionId}' extension is forbidden`, - ); + collector.report({ + code: 'EXTENSION_CONFIG_FORBIDDEN', + message: `Configuration of the '${extensionId}' extension is forbidden`, + context: { + extensionId, + }, + }); + continue; } - const existing = configuredExtensions.find( + const existing = deduplicatedExtensions.find( e => e.extension.id === extensionId, ); if (existing) { @@ -216,14 +204,20 @@ export function resolveAppNodeSpecs(options: { existing.params.disabled = Boolean(overrideParam.disabled); } order.set(extensionId, existing); - } else if (!allowUnknownExtensionConfig) { - throw new Error(`Extension ${extensionId} does not exist`); + } else { + collector.report({ + code: 'EXTENSION_CONFIG_UNKNOWN_EXTENSION', + message: `Extension ${extensionId} does not exist`, + context: { + extensionId, + }, + }); } } const orderedExtensions = [ ...order.values(), - ...configuredExtensions.filter(e => !order.has(e.extension.id)), + ...deduplicatedExtensions.filter(e => !order.has(e.extension.id)), ]; return orderedExtensions.map(param => ({ diff --git a/packages/frontend-app-api/src/tree/resolveAppTree.test.ts b/packages/frontend-app-api/src/tree/resolveAppTree.test.ts index f1161b202e..79bdf81126 100644 --- a/packages/frontend-app-api/src/tree/resolveAppTree.test.ts +++ b/packages/frontend-app-api/src/tree/resolveAppTree.test.ts @@ -24,6 +24,18 @@ import { import { resolveAppTree } from './resolveAppTree'; // eslint-disable-next-line @backstage/no-relative-monorepo-imports import { resolveExtensionDefinition } from '../../../frontend-plugin-api/src/wiring/resolveExtensionDefinition'; +import { createErrorCollector } from '../wiring/createErrorCollector'; + +const collector = createErrorCollector(); + +afterEach(() => { + const errors = collector.collectErrors(); + if (errors) { + throw new Error( + `Unexpected errors: ${errors.map(e => e.message).join(', ')}`, + ); + } +}); const extension = resolveExtensionDefinition( createExtension({ @@ -43,13 +55,13 @@ const baseSpec = { describe('buildAppTree', () => { it('should fail to create an empty tree', () => { - expect(() => resolveAppTree('app', [])).toThrow( + expect(() => resolveAppTree('app', [], collector)).toThrow( "No root node with id 'app' found in app tree", ); }); it('should create a tree with only one node', () => { - const tree = resolveAppTree('app', [{ ...baseSpec, id: 'app' }]); + const tree = resolveAppTree('app', [{ ...baseSpec, id: 'app' }], collector); expect(tree.root).toEqual({ spec: { ...baseSpec, id: 'app' }, edges: { attachments: new Map() }, @@ -59,15 +71,19 @@ describe('buildAppTree', () => { }); it('should create a tree', () => { - const tree = resolveAppTree('b', [ - { ...baseSpec, id: 'a' }, - { ...baseSpec, id: 'b' }, - { ...baseSpec, id: 'c' }, - { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx1' }, - { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx2' }, - { ...baseSpec, attachTo: { id: 'b', input: 'y' }, id: 'by1' }, - { ...baseSpec, attachTo: { id: 'd', input: 'x' }, id: 'dx1' }, - ]); + const tree = resolveAppTree( + 'b', + [ + { ...baseSpec, id: 'a' }, + { ...baseSpec, id: 'b' }, + { ...baseSpec, id: 'c' }, + { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx1' }, + { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx2' }, + { ...baseSpec, attachTo: { id: 'b', input: 'y' }, id: 'by1' }, + { ...baseSpec, attachTo: { id: 'd', input: 'x' }, id: 'dx1' }, + ], + collector, + ); expect(Array.from(tree.nodes.keys())).toEqual([ 'a', @@ -122,26 +138,30 @@ describe('buildAppTree', () => { }); it('should create a tree with clones', () => { - const tree = resolveAppTree('a', [ - { ...baseSpec, id: 'a' }, - { ...baseSpec, id: 'b', attachTo: { id: 'a', input: 'x' } }, - { - ...baseSpec, - id: 'c', - attachTo: [ - { id: 'a', input: 'x' }, - { id: 'b', input: 'x' }, - ], - }, - { - ...baseSpec, - id: 'd', - attachTo: [ - { id: 'b', input: 'x' }, - { id: 'c', input: 'x' }, - ], - }, - ]); + const tree = resolveAppTree( + 'a', + [ + { ...baseSpec, id: 'a' }, + { ...baseSpec, id: 'b', attachTo: { id: 'a', input: 'x' } }, + { + ...baseSpec, + id: 'c', + attachTo: [ + { id: 'a', input: 'x' }, + { id: 'b', input: 'x' }, + ], + }, + { + ...baseSpec, + id: 'd', + attachTo: [ + { id: 'b', input: 'x' }, + { id: 'c', input: 'x' }, + ], + }, + ], + collector, + ); expect(Array.from(tree.nodes.keys())).toEqual(['a', 'b', 'c', 'd']); @@ -172,15 +192,19 @@ describe('buildAppTree', () => { }); it('should create a tree out of order', () => { - const tree = resolveAppTree('b', [ - { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx2' }, - { ...baseSpec, id: 'a' }, - { ...baseSpec, attachTo: { id: 'b', input: 'y' }, id: 'by1' }, - { ...baseSpec, id: 'b' }, - { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx1' }, - { ...baseSpec, id: 'c' }, - { ...baseSpec, attachTo: { id: 'd', input: 'x' }, id: 'dx1' }, - ]); + const tree = resolveAppTree( + 'b', + [ + { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx2' }, + { ...baseSpec, id: 'a' }, + { ...baseSpec, attachTo: { id: 'b', input: 'y' }, id: 'by1' }, + { ...baseSpec, id: 'b' }, + { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx1' }, + { ...baseSpec, id: 'c' }, + { ...baseSpec, attachTo: { id: 'd', input: 'x' }, id: 'dx1' }, + ], + collector, + ); expect(Array.from(tree.nodes.keys())).toEqual([ 'bx2', @@ -214,13 +238,23 @@ describe('buildAppTree', () => { `); }); - it('throws an error when duplicated extensions are detected', () => { - expect(() => - resolveAppTree('app', [ + it('emits an error when duplicated extensions are detected', () => { + const tree = resolveAppTree( + 'a', + [ { ...baseSpec, id: 'a' }, { ...baseSpec, id: 'a' }, - ]), - ).toThrow("Unexpected duplicate extension id 'a'"); + ], + collector, + ); + expect(Array.from(tree.nodes.keys())).toEqual(['a']); + expect(collector.collectErrors()).toEqual([ + { + code: 'DUPLICATE_EXTENSION_ID', + message: "Unexpected duplicate extension id 'a'", + context: { spec: { ...baseSpec, id: 'a' } }, + }, + ]); }); describe('redirects', () => { @@ -253,12 +287,28 @@ describe('buildAppTree', () => { }), ) as Extension; - expect(() => - resolveAppTree('a', [ + const tree = resolveAppTree( + 'a', + [ { ...baseSpec, id: 'a', extension: e1 }, { ...baseSpec, id: 'b', extension: e2 }, - ]), - ).toThrow("Duplicate redirect target for input 'test' in extension 'b'"); + ], + collector, + ); + + expect(Array.from(tree.nodes.keys())).toEqual(['a', 'b']); + + expect(collector.collectErrors()).toEqual([ + { + code: 'DUPLICATE_REDIRECT_TARGET', + message: + "Duplicate redirect target for input 'test' in extension 'b'", + context: { + spec: { ...baseSpec, id: 'b', extension: e2 }, + inputName: 'test', + }, + }, + ]); }); it('should set the correct attachment point for a redirect', () => { @@ -285,22 +335,26 @@ describe('buildAppTree', () => { }), ) as Extension; - const tree = resolveAppTree('a', [ - { - attachTo: e1.attachTo, - id: 'a', - extension: e1, - disabled: false, - plugin: baseSpec.plugin, - }, - { - attachTo: e2.attachTo, - id: 'b', - extension: e2, - disabled: false, - plugin: baseSpec.plugin, - }, - ]); + const tree = resolveAppTree( + 'a', + [ + { + attachTo: e1.attachTo, + id: 'a', + extension: e1, + disabled: false, + plugin: baseSpec.plugin, + }, + { + attachTo: e2.attachTo, + id: 'b', + extension: e2, + disabled: false, + plugin: baseSpec.plugin, + }, + ], + collector, + ); expect(tree.root).toMatchInlineSnapshot(` { @@ -365,29 +419,33 @@ describe('buildAppTree', () => { }), ) as Extension; - const tree = resolveAppTree('test-2', [ - { - attachTo: e1.attachTo, - id: e1.id, - extension: e1, - disabled: false, - plugin: baseSpec.plugin, - }, - { - attachTo: e2.attachTo, - id: e2.id, - extension: e2, - disabled: false, - plugin: baseSpec.plugin, - }, - { - attachTo: e3.attachTo, - id: e3.id, - extension: e3, - disabled: false, - plugin: baseSpec.plugin, - }, - ]); + const tree = resolveAppTree( + 'test-2', + [ + { + attachTo: e1.attachTo, + id: e1.id, + extension: e1, + disabled: false, + plugin: baseSpec.plugin, + }, + { + attachTo: e2.attachTo, + id: e2.id, + extension: e2, + disabled: false, + plugin: baseSpec.plugin, + }, + { + attachTo: e3.attachTo, + id: e3.id, + extension: e3, + disabled: false, + plugin: baseSpec.plugin, + }, + ], + collector, + ); expect(tree.nodes.get('test-3')?.edges.attachedTo?.node).toBe( tree.nodes.get('test-2'), diff --git a/packages/frontend-app-api/src/tree/resolveAppTree.ts b/packages/frontend-app-api/src/tree/resolveAppTree.ts index dacd06407a..fc2c8f845e 100644 --- a/packages/frontend-app-api/src/tree/resolveAppTree.ts +++ b/packages/frontend-app-api/src/tree/resolveAppTree.ts @@ -23,6 +23,7 @@ import { // eslint-disable-next-line @backstage/no-relative-monorepo-imports import { toInternalExtension } from '../../../frontend-plugin-api/src/wiring/resolveExtensionDefinition'; +import { ErrorCollector } from '../wiring/createErrorCollector'; function indent(str: string) { return str.replace(/^/gm, ' '); @@ -114,6 +115,7 @@ const isValidAttachmentPoint = ( export function resolveAppTree( rootNodeId: string, specs: AppNodeSpec[], + errorCollector: ErrorCollector, ): AppTree { const nodes = new Map(); @@ -122,7 +124,14 @@ export function resolveAppTree( for (const spec of specs) { // The main check with a more helpful error message happens in resolveAppNodeSpecs if (nodes.has(spec.id)) { - throw new Error(`Unexpected duplicate extension id '${spec.id}'`); + errorCollector.report({ + code: 'DUPLICATE_EXTENSION_ID', + message: `Unexpected duplicate extension id '${spec.id}'`, + context: { + spec, + }, + }); + continue; } const node = new SerializableAppNode(spec); @@ -134,9 +143,15 @@ export function resolveAppTree( for (const replace of input.replaces) { const key = makeRedirectKey(replace); if (redirectTargetsByKey.has(key)) { - throw new Error( - `Duplicate redirect target for input '${inputName}' in extension '${spec.id}'`, - ); + errorCollector.report({ + code: 'DUPLICATE_REDIRECT_TARGET', + message: `Duplicate redirect target for input '${inputName}' in extension '${spec.id}'`, + context: { + spec, + inputName, + }, + }); + continue; } redirectTargetsByKey.set(key, { id: spec.id, input: inputName }); } diff --git a/packages/frontend-app-api/src/wiring/createErrorCollector.ts b/packages/frontend-app-api/src/wiring/createErrorCollector.ts new file mode 100644 index 0000000000..65ef91ccfc --- /dev/null +++ b/packages/frontend-app-api/src/wiring/createErrorCollector.ts @@ -0,0 +1,69 @@ +/* + * Copyright 2025 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + AppNode, + AppNodeSpec, + FrontendPlugin, +} from '@backstage/frontend-plugin-api'; + +/** @public */ +export type AppError = { + code: string; + message: string; + context?: { + node?: AppNode; + spec?: AppNodeSpec; + plugin?: FrontendPlugin; + extensionId?: string; + inputName?: string; + dataRefId?: string; + }; +}; + +/** @internal */ +export interface ErrorCollector { + report(report: AppError): void; + child(context?: AppError['context']): ErrorCollector; + collectErrors(): AppError[] | undefined; +} + +/** @internal */ +export function createErrorCollector(context?: AppError['context']) { + const errors: AppError[] = []; + const children: ErrorCollector[] = []; + return { + report(report: AppError) { + errors.push({ ...report, context: { ...context, ...report.context } }); + }, + collectErrors() { + const allErrors = [ + ...errors, + ...children.flatMap(child => child.collectErrors() ?? []), + ]; + errors.length = 0; + if (allErrors.length === 0) { + return undefined; + } + return allErrors; + }, + child(childContext: AppError['context']) { + const child = createErrorCollector(childContext); + children.push(child); + return child; + }, + }; +} diff --git a/packages/frontend-app-api/src/wiring/createSpecializedApp.tsx b/packages/frontend-app-api/src/wiring/createSpecializedApp.tsx index 01e0a6f657..bcb57ce7e7 100644 --- a/packages/frontend-app-api/src/wiring/createSpecializedApp.tsx +++ b/packages/frontend-app-api/src/wiring/createSpecializedApp.tsx @@ -83,6 +83,11 @@ import { FrontendPluginInfoResolver, } from './createPluginInfoAttacher'; import { createRouteAliasResolver } from '../routing/RouteAliasResolver'; +import { + AppError, + createErrorCollector, + ErrorCollector, +} from './createErrorCollector'; function deduplicateFeatures( allFeatures: FrontendFeature[], @@ -284,12 +289,15 @@ export type CreateSpecializedAppOptions = { export function createSpecializedApp(options?: CreateSpecializedAppOptions): { apis: ApiHolder; tree: AppTree; + errors?: AppError[]; } { const config = options?.config ?? new ConfigReader({}, 'empty-config'); const features = deduplicateFeatures(options?.features ?? []).map( createPluginInfoAttacher(config, options?.advanced?.pluginInfoResolver), ); + const collector = createErrorCollector(); + const tree = resolveAppTree( 'root', resolveAppNodeSpecs({ @@ -299,12 +307,12 @@ export function createSpecializedApp(options?: CreateSpecializedAppOptions): { ], parameters: readAppExtensionsConfig(config), forbidden: new Set(['root']), - allowUnknownExtensionConfig: - options?.advanced?.allowUnknownExtensionConfig, + collector, }), + collector, ); - const factories = createApiFactories({ tree }); + const factories = createApiFactories({ tree, collector }); const appBasePath = getBasePath(config); const appTreeApi = new AppTreeApiProxy(tree, appBasePath); @@ -353,6 +361,7 @@ export function createSpecializedApp(options?: CreateSpecializedAppOptions): { instantiateAppNodeTree( tree.root, apis, + collector, mergeExtensionFactoryMiddleware( options?.advanced?.extensionFactoryMiddleware, ), @@ -366,22 +375,32 @@ export function createSpecializedApp(options?: CreateSpecializedAppOptions): { routeResolutionApi.initialize(routeInfo, routeRefsById.routes); appTreeApi.initialize(routeInfo); - return { apis, tree }; + return { apis, tree, errors: collector.collectErrors() }; } -function createApiFactories(options: { tree: AppTree }): AnyApiFactory[] { +function createApiFactories(options: { + tree: AppTree; + collector: ErrorCollector; +}): AnyApiFactory[] { const emptyApiHolder = ApiRegistry.from([]); const factories = new Array(); for (const apiNode of options.tree.root.edges.attachments.get('apis') ?? []) { - instantiateAppNodeTree(apiNode, emptyApiHolder); - const apiFactory = apiNode.instance?.getData(ApiBlueprint.dataRefs.factory); - if (!apiFactory) { - throw new Error( - `No API factory found in for extension ${apiNode.spec.id}`, - ); + if (!instantiateAppNodeTree(apiNode, emptyApiHolder, options.collector)) { + continue; + } + const apiFactory = apiNode.instance?.getData(ApiBlueprint.dataRefs.factory); + if (apiFactory) { + factories.push(apiFactory); + } else { + options.collector.report({ + code: 'NO_API_FACTORY', + message: `API extension '${apiNode.spec.id}' did not output an API factory`, + context: { + node: apiNode, + }, + }); } - factories.push(apiFactory); } return factories; diff --git a/packages/frontend-app-api/src/wiring/index.ts b/packages/frontend-app-api/src/wiring/index.ts index 2571cadfbd..ce7574852a 100644 --- a/packages/frontend-app-api/src/wiring/index.ts +++ b/packages/frontend-app-api/src/wiring/index.ts @@ -19,3 +19,4 @@ export { type CreateSpecializedAppOptions, } from './createSpecializedApp'; export { type FrontendPluginInfoResolver } from './createPluginInfoAttacher'; +export { type AppError } from './createErrorCollector';