From 69938650471bae695452a051ee26966971e8c84c Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 31 Aug 2025 10:10:36 +0200 Subject: [PATCH 01/11] 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'; From c9c1d69f3904edf744dd96a6b7492b046edb11ff Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 6 Sep 2025 16:22:06 +0200 Subject: [PATCH 02/11] frontend-app-api: type safe error collection and context Signed-off-by: Patrik Oldsberg --- .../src/tree/instantiateAppNodeTree.ts | 11 +- .../src/wiring/createErrorCollector.ts | 100 ++++++++++++++---- 2 files changed, 86 insertions(+), 25 deletions(-) diff --git a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts index a33c5943a1..cbf24a98fa 100644 --- a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts +++ b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts @@ -64,7 +64,7 @@ function resolveInputDataContainer( extensionData: Array, attachment: AppNode, inputName: string, - collector: ErrorCollector, + collector: ErrorCollector<'node' | 'inputName'>, ): ({ node: AppNode } & ExtensionDataContainer) | undefined { const dataMap = new Map(); @@ -211,7 +211,7 @@ function resolveV2Inputs( >; }, attachments: ReadonlyMap, - collector: ErrorCollector, + parentCollector: ErrorCollector<'node'>, ): | undefined | ResolvedExtensionInputs<{ @@ -223,6 +223,7 @@ function resolveV2Inputs( let failed = false; const resolvedInputs = mapValues(inputMap, (input, inputName) => { const attachedNodes = attachments.get(inputName) ?? []; + const collector = parentCollector.child({ inputName }); if (input.config.singleton) { if (attachedNodes.length > 1) { @@ -232,7 +233,6 @@ function resolveV2Inputs( message: `expected ${ input.config.optional ? 'at most' : 'exactly' } one '${inputName}' input but received multiple: '${attachedNodeIds}'`, - context: { inputName }, }); failed = true; return undefined; @@ -241,7 +241,6 @@ function resolveV2Inputs( collector.report({ code: 'EXTENSION_MISSING_REQUIRED_INPUT', message: `input '${inputName}' is required but was not received`, - context: { inputName }, }); failed = true; } @@ -481,7 +480,7 @@ export function createAppNodeInstance(options: { export function instantiateAppNodeTree( rootNode: AppNode, apis: ApiHolder, - errors: ErrorCollector, + collector: ErrorCollector, extensionFactoryMiddleware?: ExtensionFactoryMiddleware, ): boolean { function createInstance(node: AppNode): AppNodeInstance | undefined { @@ -512,7 +511,7 @@ export function instantiateAppNodeTree( node, apis, attachments: instantiatedAttachments, - collector: errors, + collector, }); return node.instance; diff --git a/packages/frontend-app-api/src/wiring/createErrorCollector.ts b/packages/frontend-app-api/src/wiring/createErrorCollector.ts index 65ef91ccfc..494e6c2d85 100644 --- a/packages/frontend-app-api/src/wiring/createErrorCollector.ts +++ b/packages/frontend-app-api/src/wiring/createErrorCollector.ts @@ -19,34 +19,96 @@ import { AppNodeSpec, FrontendPlugin, } from '@backstage/frontend-plugin-api'; +import { Expand } from '@backstage/types'; -/** @public */ -export type AppError = { - code: string; - message: string; - context?: { - node?: AppNode; - spec?: AppNodeSpec; - plugin?: FrontendPlugin; - extensionId?: string; - inputName?: string; - dataRefId?: string; - }; +type AppErrorTypes = { + // resolveAppNodeSpecs + EXTENSION_FORBIDDEN: 'plugin' | 'extensionId'; + EXTENSION_DUPLICATED: 'plugin' | 'extensionId'; + EXTENSION_CONFIG_FORBIDDEN: 'extensionId'; + EXTENSION_CONFIG_UNKNOWN_EXTENSION: 'extensionId'; + // resolveAppTree + DUPLICATE_EXTENSION_ID: 'spec'; + DUPLICATE_REDIRECT_TARGET: 'spec' | 'inputName'; + // instantiateAppNodeTree + EXTENSION_DUPLICATE_INPUT: 'node' | 'inputName'; + EXTENSION_MISSING_INPUT_DATA: 'node' | 'inputName'; + EXTENSION_TOO_MANY_ATTACHMENTS: 'node' | 'inputName'; + EXTENSION_MISSING_REQUIRED_INPUT: 'node' | 'inputName'; + INVALID_CONFIGURATION: 'node'; + EXTENSION_FACTORY_INVALID_OUTPUT: 'node'; + EXTENSION_FACTORY_DUPLICATE_OUTPUT: 'node' | 'dataRefId'; + EXTENSION_FACTORY_MISSING_REQUIRED_OUTPUT: 'node' | 'dataRefId'; + EXTENSION_FACTORY_UNEXPECTED_OUTPUT: 'node' | 'dataRefId'; + UNEXPECTED_EXTENSION_VERSION: 'node'; + FAILED_TO_INSTANTIATE_EXTENSION: 'node'; + // createSpecializedApp + NO_API_FACTORY: 'node'; }; +type AppErrorContext = { + node?: AppNode; + spec?: AppNodeSpec; + plugin?: FrontendPlugin; + extensionId?: string; + dataRefId?: string; + inputName?: string; +}; + +/** @public */ +export type AppError = + { + code: TCode; + message: string; + context: Expand< + AppErrorContext & + Pick< + Required, + keyof AppErrorTypes extends TCode ? never : AppErrorTypes[TCode] + > + >; + }; + /** @internal */ -export interface ErrorCollector { - report(report: AppError): void; - child(context?: AppError['context']): ErrorCollector; +export interface ErrorCollector< + TContext extends keyof AppErrorContext = never, +> { + // Type-only: here to make sure that all required keys are present + $$contextKeys: { [K in TContext]: K }; + report( + report: Exclude< + AppErrorTypes[TCode], + TContext + > extends infer IContext extends keyof AppErrorContext + ? [IContext] extends [never] + ? { + code: TCode; + message: string; + } + : { + code: TCode; + message: string; + context: Pick, IContext>; + } + : never, + ): void; + child( + context?: TAdditionalContext & AppErrorContext, + ): ErrorCollector< + (TContext | keyof TAdditionalContext) & keyof AppErrorContext + >; collectErrors(): AppError[] | undefined; } /** @internal */ -export function createErrorCollector(context?: AppError['context']) { +export function createErrorCollector( + context?: AppError['context'], +): ErrorCollector { const errors: AppError[] = []; const children: ErrorCollector[] = []; return { - report(report: AppError) { + $$contextKeys: null as any, + report(report) { errors.push({ ...report, context: { ...context, ...report.context } }); }, collectErrors() { @@ -60,10 +122,10 @@ export function createErrorCollector(context?: AppError['context']) { } return allErrors; }, - child(childContext: AppError['context']) { + child(childContext) { const child = createErrorCollector(childContext); children.push(child); - return child; + return child as ErrorCollector; }, }; } From 74ea3e3a86bab5c2a814904afb027df41848a216 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 6 Sep 2025 18:07:42 +0200 Subject: [PATCH 03/11] frontend-app-api: simplified control flow for extension instantiation + fixes Signed-off-by: Patrik Oldsberg --- .../src/tree/instantiateAppNodeTree.test.ts | 2 +- .../src/tree/instantiateAppNodeTree.ts | 253 +++++++++--------- .../src/wiring/createErrorCollector.ts | 2 +- 3 files changed, 131 insertions(+), 126 deletions(-) diff --git a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.test.ts b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.test.ts index 9ed0319045..4b086b58b5 100644 --- a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.test.ts +++ b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.test.ts @@ -1613,7 +1613,7 @@ describe('instantiateAppNodeTree', () => { 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 }, + context: { node, inputName: 'singleton' }, }, ]); }); diff --git a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts index cbf24a98fa..212cb19bc5 100644 --- a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts +++ b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts @@ -29,6 +29,35 @@ import { toInternalExtension } from '../../../frontend-plugin-api/src/wiring/res import { createExtensionDataContainer } from '@internal/frontend'; import { ErrorCollector } from '../wiring/createErrorCollector'; +const INSTANTIATION_FAILED = new Error('Instantiation failed'); + +/** + * Like `array.map`, but if `INSTANTIATION_FAILED` is thrown, the iteration will continue but afterwards re-throw `INSTANTIATION_FAILED` + * @returns + */ +function mapWithFailures( + iterable: Iterable, + callback: (item: T) => U, +): U[] { + let failed = false; + const results = Array.from(iterable).map(item => { + try { + return callback(item); + } catch (error) { + if (error === INSTANTIATION_FAILED) { + failed = true; + } else { + throw error; + } + return null as any; + } + }); + if (failed) { + throw INSTANTIATION_FAILED; + } + return results; +} + type Mutable = { -readonly [P in keyof T]: T[P]; }; @@ -40,24 +69,26 @@ function resolveV1InputDataMap( attachment: AppNode, inputName: string, ) { - return mapValues(dataMap, ref => { - const value = attachment.instance?.getData(ref); - if (value === undefined && !ref.config.optional) { - const expected = Object.values(dataMap) - .filter(r => !r.config.optional) - .map(r => `'${r.id}'`) - .join(', '); + return Object.fromEntries( + mapWithFailures(Object.entries(dataMap), ([key, ref]) => { + const value = attachment.instance?.getData(ref); + if (value === undefined && !ref.config.optional) { + const expected = Object.values(dataMap) + .filter(r => !r.config.optional) + .map(r => `'${r.id}'`) + .join(', '); - const provided = [...(attachment.instance?.getDataRefs() ?? [])] - .map(r => `'${r.id}'`) - .join(', '); + const provided = [...(attachment.instance?.getDataRefs() ?? [])] + .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})`, - ); - } - return value; - }); + 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})`, + ); + } + return [key, value]; + }), + ); } function resolveInputDataContainer( @@ -65,18 +96,18 @@ function resolveInputDataContainer( attachment: AppNode, inputName: string, collector: ErrorCollector<'node' | 'inputName'>, -): ({ node: AppNode } & ExtensionDataContainer) | undefined { +): { node: AppNode } & ExtensionDataContainer { const dataMap = new Map(); - let failed = false; - for (const ref of extensionData) { + mapWithFailures(extensionData, ref => { if (dataMap.has(ref.id)) { collector.report({ code: 'EXTENSION_DUPLICATE_INPUT', message: `Unexpected duplicate input data '${ref.id}'`, }); - continue; + return; } + const value = attachment.instance?.getData(ref); if (value === undefined && !ref.config.optional) { const expected = extensionData @@ -92,15 +123,11 @@ function resolveInputDataContainer( 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; + throw INSTANTIATION_FAILED; } dataMap.set(ref.id, value); - } - - if (failed) { - return undefined; - } + }); return { node: attachment, @@ -160,40 +187,52 @@ function resolveV1Inputs( }, attachments: ReadonlyMap, ) { - return mapValues(inputMap, (input, inputName) => { - const attachedNodes = attachments.get(inputName) ?? []; + return Object.fromEntries( + mapWithFailures(Object.entries(inputMap), ([inputName, input]) => { + const attachedNodes = attachments.get(inputName) ?? []; - if (input.config.singleton) { - if (attachedNodes.length > 1) { - const attachedNodeIds = attachedNodes.map(e => e.spec.id); - throw Error( - `expected ${ - input.config.optional ? 'at most' : 'exactly' - } one '${inputName}' input but received multiple: '${attachedNodeIds.join( - "', '", - )}'`, - ); - } else if (attachedNodes.length === 0) { - if (input.config.optional) { - return undefined; + if (input.config.singleton) { + if (attachedNodes.length > 1) { + const attachedNodeIds = attachedNodes.map(e => e.spec.id); + throw Error( + `expected ${ + input.config.optional ? 'at most' : 'exactly' + } one '${inputName}' input but received multiple: '${attachedNodeIds.join( + "', '", + )}'`, + ); + } else if (attachedNodes.length === 0) { + if (input.config.optional) { + return [inputName, undefined]; + } + throw Error(`input '${inputName}' is required but was not received`); } - throw Error(`input '${inputName}' is required but was not received`); - } - return { - node: attachedNodes[0], - output: resolveV1InputDataMap( - input.extensionData, - attachedNodes[0], + return [ inputName, - ), - }; - } + { + node: attachedNodes[0], + output: resolveV1InputDataMap( + input.extensionData, + attachedNodes[0], + inputName, + ), + }, + ]; + } - return attachedNodes.map(attachment => ({ - node: attachment, - output: resolveV1InputDataMap(input.extensionData, attachment, inputName), - })); - }) as { + return [ + inputName, + attachedNodes.map(attachment => ({ + node: attachment, + output: resolveV1InputDataMap( + input.extensionData, + attachment, + inputName, + ), + })), + ]; + }), + ) as { [inputName in string]: { node: AppNode; output: { @@ -212,16 +251,13 @@ function resolveV2Inputs( }, attachments: ReadonlyMap, parentCollector: ErrorCollector<'node'>, -): - | undefined - | ResolvedExtensionInputs<{ - [inputName in string]: ExtensionInput< - ExtensionDataRef, - { optional: boolean; singleton: boolean } - >; - }> { - let failed = false; - const resolvedInputs = mapValues(inputMap, (input, inputName) => { +): ResolvedExtensionInputs<{ + [inputName in string]: ExtensionInput< + ExtensionDataRef, + { optional: boolean; singleton: boolean } + >; +}> { + return mapValues(inputMap, (input, inputName) => { const attachedNodes = attachments.get(inputName) ?? []; const collector = parentCollector.child({ inputName }); @@ -234,55 +270,34 @@ function resolveV2Inputs( input.config.optional ? 'at most' : 'exactly' } one '${inputName}' input but received multiple: '${attachedNodeIds}'`, }); - failed = true; - return undefined; + throw INSTANTIATION_FAILED; } else if (attachedNodes.length === 0) { if (!input.config.optional) { collector.report({ code: 'EXTENSION_MISSING_REQUIRED_INPUT', message: `input '${inputName}' is required but was not received`, }); - failed = true; + throw INSTANTIATION_FAILED; } return undefined; } - const data = resolveInputDataContainer( + return resolveInputDataContainer( input.extensionData, attachedNodes[0], inputName, collector, ); - if (data === undefined) { - failed = true; - return undefined; - } - return data; } - if (failed) { - return undefined; - } - - return attachedNodes.map(attachment => { - const data = resolveInputDataContainer( + return mapWithFailures(attachedNodes, attachment => + resolveInputDataContainer( input.extensionData, attachment, inputName, collector, - ); - if (data === undefined) { - failed = true; - return undefined; - } - return data; - }); - }); - - if (failed) { - return undefined; - } - - return resolvedInputs as ResolvedExtensionInputs<{ + ), + ); + }) as ResolvedExtensionInputs<{ [inputName in string]: ExtensionInput< ExtensionDataRef, { optional: boolean; singleton: boolean } @@ -346,19 +361,15 @@ 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, + inputs: resolveV2Inputs( + internalExtension.inputs, + attachments, + collector, + ), }; const outputDataValues = options.extensionFactoryMiddleware ? createExtensionDataContainer( @@ -385,13 +396,11 @@ export function createAppNodeInstance(options: { code: 'EXTENSION_FACTORY_INVALID_OUTPUT', message: 'extension factory did not provide an iterable object', }); - return undefined; + throw INSTANTIATION_FAILED; } - let failed = false; - const outputDataMap = new Map(); - for (const value of outputDataValues) { + mapWithFailures(outputDataValues, value => { if (outputDataMap.has(value.id)) { collector.report({ code: 'EXTENSION_FACTORY_DUPLICATE_OUTPUT', @@ -400,14 +409,11 @@ export function createAppNodeInstance(options: { dataRefId: value.id, }, }); - failed = true; + throw INSTANTIATION_FAILED; } else { outputDataMap.set(value.id, value.value); } - } - if (failed) { - return undefined; - } + }); for (const ref of internalExtension.output) { const value = outputDataMap.get(ref.id); @@ -421,16 +427,13 @@ export function createAppNodeInstance(options: { dataRefId: ref.id, }, }); - failed = true; + throw INSTANTIATION_FAILED; } } else { extensionData.set(ref.id, value); extensionDataRefs.add(ref); } } - if (failed) { - return undefined; - } if (outputDataMap.size > 0) { for (const dataRefId of outputDataMap.keys()) { @@ -451,15 +454,17 @@ export function createAppNodeInstance(options: { (internalExtension as any).version }'`, }); - return undefined; + throw INSTANTIATION_FAILED; } } catch (e) { - collector.report({ - code: 'FAILED_TO_INSTANTIATE_EXTENSION', - message: `Failed to instantiate extension '${id}'${ - e.name === 'Error' ? `, ${e.message}` : `; caused by ${e}` - }`, - }); + if (e !== INSTANTIATION_FAILED) { + collector.report({ + code: 'FAILED_TO_INSTANTIATE_EXTENSION', + message: `Failed to instantiate extension '${id}'${ + e.name === 'Error' ? `, ${e.message}` : `; caused by ${e}` + }`, + }); + } return undefined; } diff --git a/packages/frontend-app-api/src/wiring/createErrorCollector.ts b/packages/frontend-app-api/src/wiring/createErrorCollector.ts index 494e6c2d85..cc51f146ce 100644 --- a/packages/frontend-app-api/src/wiring/createErrorCollector.ts +++ b/packages/frontend-app-api/src/wiring/createErrorCollector.ts @@ -123,7 +123,7 @@ export function createErrorCollector( return allErrors; }, child(childContext) { - const child = createErrorCollector(childContext); + const child = createErrorCollector({ ...context, ...childContext }); children.push(child); return child as ErrorCollector; }, From d27c0327768917a1dbeb7b5b2ab0aef77b22a44d Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 7 Sep 2025 11:45:40 +0200 Subject: [PATCH 04/11] frontend-app-api: refine app error types Signed-off-by: Patrik Oldsberg --- .../src/tree/instantiateAppNodeTree.test.ts | 56 +++++++++---------- .../src/tree/instantiateAppNodeTree.ts | 22 ++++---- .../src/tree/resolveAppNodeSpecs.test.ts | 6 +- .../src/tree/resolveAppNodeSpecs.ts | 16 +++--- .../src/tree/resolveAppTree.test.ts | 15 ++--- .../src/tree/resolveAppTree.ts | 13 +---- .../src/wiring/createErrorCollector.ts | 39 +++++-------- .../src/wiring/createSpecializedApp.tsx | 2 +- 8 files changed, 74 insertions(+), 95 deletions(-) diff --git a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.test.ts b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.test.ts index 4b086b58b5..4f1589d3a3 100644 --- a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.test.ts +++ b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.test.ts @@ -411,7 +411,7 @@ describe('instantiateAppNodeTree', () => { ).toBeUndefined(); expect(collector.collectErrors()).toEqual([ { - code: 'INVALID_CONFIGURATION', + code: 'EXTENSION_CONFIGURATION_INVALID', message: "Invalid configuration for extension 'app/test'; caused by Error: Expected number, received string at 'other'", context: { node }, @@ -443,7 +443,7 @@ describe('instantiateAppNodeTree', () => { ).toBeUndefined(); expect(collector.collectErrors()).toEqual([ { - code: 'FAILED_TO_INSTANTIATE_EXTENSION', + code: 'EXTENSION_FACTORY_ERROR', message: "Failed to instantiate extension 'app/test'; caused by NopeError: NOPE", context: { node }, @@ -476,7 +476,7 @@ describe('instantiateAppNodeTree', () => { ).toBeUndefined(); expect(collector.collectErrors()).toEqual([ { - code: 'FAILED_TO_INSTANTIATE_EXTENSION', + code: 'EXTENSION_FACTORY_ERROR', message: "Failed to instantiate extension 'app/test', duplicate extension data 'test' received via output 'test2'", context: { node }, @@ -508,7 +508,7 @@ describe('instantiateAppNodeTree', () => { ).toBeUndefined(); expect(collector.collectErrors()).toEqual([ { - code: 'FAILED_TO_INSTANTIATE_EXTENSION', + code: 'EXTENSION_FACTORY_ERROR', message: "Failed to instantiate extension 'app/test', unknown output provided via 'nonexistent'", context: { node }, @@ -544,7 +544,7 @@ describe('instantiateAppNodeTree', () => { ).toBeUndefined(); expect(collector.collectErrors()).toEqual([ { - code: 'FAILED_TO_INSTANTIATE_EXTENSION', + code: 'EXTENSION_FACTORY_ERROR', message: "Failed to instantiate extension 'app/test', input 'singleton' is required but was not received", context: { node }, @@ -669,7 +669,7 @@ describe('instantiateAppNodeTree', () => { ).toBeUndefined(); expect(collector.collectErrors()).toEqual([ { - code: 'FAILED_TO_INSTANTIATE_EXTENSION', + code: 'EXTENSION_FACTORY_ERROR', message: "Failed to instantiate extension 'app/test', expected exactly one 'singleton' input but received multiple: 'app/test', 'app/test'", context: { node }, @@ -713,7 +713,7 @@ describe('instantiateAppNodeTree', () => { ).toBeUndefined(); expect(collector.collectErrors()).toEqual([ { - code: 'FAILED_TO_INSTANTIATE_EXTENSION', + code: 'EXTENSION_FACTORY_ERROR', message: "Failed to instantiate extension 'app/test', expected at most one 'singleton' input but received multiple: 'app/test', 'app/test'", context: { node }, @@ -751,7 +751,7 @@ describe('instantiateAppNodeTree', () => { ).toBeUndefined(); expect(collector.collectErrors()).toEqual([ { - code: 'FAILED_TO_INSTANTIATE_EXTENSION', + code: 'EXTENSION_FACTORY_ERROR', 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 }, @@ -1073,7 +1073,7 @@ describe('instantiateAppNodeTree', () => { ).toBeUndefined(); expect(collector.collectErrors()).toEqual([ { - code: 'INVALID_CONFIGURATION', + code: 'EXTENSION_CONFIGURATION_INVALID', message: "Invalid configuration for extension 'app/test'; caused by Error: Expected number, received string at 'other'", context: { node }, @@ -1121,7 +1121,7 @@ describe('instantiateAppNodeTree', () => { ), ).toEqual([ { - code: 'EXTENSION_FACTORY_INVALID_OUTPUT', + code: 'EXTENSION_INVALID', message: 'extension factory did not provide an iterable object', context: { node: expect.anything() }, }, @@ -1138,8 +1138,8 @@ describe('instantiateAppNodeTree', () => { ), ).toEqual([ { - // TODO: This should probably be EXTENSION_FACTORY_INVALID_OUTPUT - code: 'FAILED_TO_INSTANTIATE_EXTENSION', + // TODO: This should probably be EXTENSION_INVALID + code: 'EXTENSION_FACTORY_ERROR', message: "Failed to instantiate extension 'test', extension factory override did not provide an iterable object", context: { node: expect.anything() }, @@ -1157,8 +1157,8 @@ describe('instantiateAppNodeTree', () => { ), ).toEqual([ { - // TODO: This should probably be EXTENSION_FACTORY_INVALID_OUTPUT - code: 'FAILED_TO_INSTANTIATE_EXTENSION', + // TODO: This should probably be EXTENSION_INVALID + code: 'EXTENSION_FACTORY_ERROR', message: "Failed to instantiate extension 'test', extension factory middleware did not provide an iterable object", context: { node: expect.anything() }, @@ -1175,7 +1175,7 @@ describe('instantiateAppNodeTree', () => { ), ).toEqual([ { - code: 'EXTENSION_FACTORY_INVALID_OUTPUT', + code: 'EXTENSION_INVALID', message: 'extension factory did not provide an iterable object', context: { node: expect.anything() }, }, @@ -1194,7 +1194,7 @@ describe('instantiateAppNodeTree', () => { ), ).toEqual([ { - code: 'EXTENSION_FACTORY_INVALID_OUTPUT', + code: 'EXTENSION_INVALID', message: 'extension factory did not provide an iterable object', context: { node: expect.anything() }, }, @@ -1214,8 +1214,8 @@ describe('instantiateAppNodeTree', () => { ), ).toEqual([ { - // TODO: This should probably be EXTENSION_FACTORY_INVALID_OUTPUT - code: 'FAILED_TO_INSTANTIATE_EXTENSION', + // TODO: This should probably be EXTENSION_INVALID + code: 'EXTENSION_FACTORY_ERROR', message: "Failed to instantiate extension 'test:test', extension factory did not provide an iterable object", context: { node: expect.anything() }, @@ -1236,8 +1236,8 @@ describe('instantiateAppNodeTree', () => { ), ).toEqual([ { - // TODO: This should probably be EXTENSION_FACTORY_INVALID_OUTPUT - code: 'FAILED_TO_INSTANTIATE_EXTENSION', + // TODO: This should probably be EXTENSION_INVALID + code: 'EXTENSION_FACTORY_ERROR', message: "Failed to instantiate extension 'test:test', original blueprint factory did not provide an iterable object", context: { node: expect.anything() }, @@ -1271,7 +1271,7 @@ describe('instantiateAppNodeTree', () => { ).toBeUndefined(); expect(collector.collectErrors()).toEqual([ { - code: 'FAILED_TO_INSTANTIATE_EXTENSION', + code: 'EXTENSION_FACTORY_ERROR', message: "Failed to instantiate extension 'app/test'; caused by NopeError: NOPE", context: { node }, @@ -1303,7 +1303,7 @@ describe('instantiateAppNodeTree', () => { ).toBeUndefined(); expect(collector.collectErrors()).toEqual([ { - code: 'EXTENSION_FACTORY_DUPLICATE_OUTPUT', + code: 'EXTENSION_OUTPUT_CONFLICT', message: "extension factory output duplicate data 'test'", context: { dataRefId: 'test', node }, }, @@ -1335,7 +1335,7 @@ describe('instantiateAppNodeTree', () => { ).toBeUndefined(); expect(collector.collectErrors()).toEqual([ { - code: 'EXTENSION_FACTORY_MISSING_REQUIRED_OUTPUT', + code: 'EXTENSION_OUTPUT_MISSING', message: "missing required extension data output 'test'", context: { dataRefId: 'test', @@ -1370,7 +1370,7 @@ describe('instantiateAppNodeTree', () => { ).toBeDefined(); expect(collector.collectErrors()).toEqual([ { - code: 'EXTENSION_FACTORY_UNEXPECTED_OUTPUT', + code: 'EXTENSION_OUTPUT_IGNORED', message: "unexpected output 'test'", context: { dataRefId: 'test', node }, }, @@ -1404,7 +1404,7 @@ describe('instantiateAppNodeTree', () => { ).toBeUndefined(); expect(collector.collectErrors()).toEqual([ { - code: 'EXTENSION_MISSING_REQUIRED_INPUT', + code: 'EXTENSION_ATTACHMENT_MISSING', message: "input 'singleton' is required but was not received", context: { inputName: 'singleton', node }, }, @@ -1529,7 +1529,7 @@ describe('instantiateAppNodeTree', () => { ).toBeUndefined(); expect(collector.collectErrors()).toEqual([ { - code: 'EXTENSION_TOO_MANY_ATTACHMENTS', + code: 'EXTENSION_ATTACHMENT_CONFLICT', message: "expected exactly one 'singleton' input but received multiple: 'app/test', 'app/test'", context: { inputName: 'singleton', node }, @@ -1573,7 +1573,7 @@ describe('instantiateAppNodeTree', () => { ).toBeUndefined(); expect(collector.collectErrors()).toEqual([ { - code: 'EXTENSION_TOO_MANY_ATTACHMENTS', + code: 'EXTENSION_ATTACHMENT_CONFLICT', message: "expected at most one 'singleton' input but received multiple: 'app/test', 'app/test'", context: { inputName: 'singleton', node }, @@ -1610,7 +1610,7 @@ describe('instantiateAppNodeTree', () => { ).toBeUndefined(); expect(collector.collectErrors()).toEqual([ { - code: 'EXTENSION_MISSING_INPUT_DATA', + code: 'EXTENSION_INPUT_DATA_MISSING', message: "extension 'app/test' could not be attached because its output data ('test') does not match what the input 'singleton' requires ('other')", context: { node, inputName: 'singleton' }, diff --git a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts index 212cb19bc5..62bf64ebc7 100644 --- a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts +++ b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts @@ -102,7 +102,7 @@ function resolveInputDataContainer( mapWithFailures(extensionData, ref => { if (dataMap.has(ref.id)) { collector.report({ - code: 'EXTENSION_DUPLICATE_INPUT', + code: 'EXTENSION_INPUT_DATA_IGNORED', message: `Unexpected duplicate input data '${ref.id}'`, }); return; @@ -120,7 +120,7 @@ function resolveInputDataContainer( .join(', '); collector.report({ - code: 'EXTENSION_MISSING_INPUT_DATA', + code: 'EXTENSION_INPUT_DATA_MISSING', message: `extension '${attachment.spec.id}' could not be attached because its output data (${provided}) does not match what the input '${inputName}' requires (${expected})`, }); throw INSTANTIATION_FAILED; @@ -265,7 +265,7 @@ function resolveV2Inputs( if (attachedNodes.length > 1) { const attachedNodeIds = attachedNodes.map(e => e.spec.id).join("', '"); collector.report({ - code: 'EXTENSION_TOO_MANY_ATTACHMENTS', + code: 'EXTENSION_ATTACHMENT_CONFLICT', message: `expected ${ input.config.optional ? 'at most' : 'exactly' } one '${inputName}' input but received multiple: '${attachedNodeIds}'`, @@ -274,7 +274,7 @@ function resolveV2Inputs( } else if (attachedNodes.length === 0) { if (!input.config.optional) { collector.report({ - code: 'EXTENSION_MISSING_REQUIRED_INPUT', + code: 'EXTENSION_ATTACHMENT_MISSING', message: `input '${inputName}' is required but was not received`, }); throw INSTANTIATION_FAILED; @@ -326,7 +326,7 @@ export function createAppNodeInstance(options: { }; } catch (e) { collector.report({ - code: 'INVALID_CONFIGURATION', + code: 'EXTENSION_CONFIGURATION_INVALID', message: `Invalid configuration for extension '${id}'; caused by ${e}`, }); return undefined; @@ -393,7 +393,7 @@ export function createAppNodeInstance(options: { !outputDataValues?.[Symbol.iterator] ) { collector.report({ - code: 'EXTENSION_FACTORY_INVALID_OUTPUT', + code: 'EXTENSION_INVALID', message: 'extension factory did not provide an iterable object', }); throw INSTANTIATION_FAILED; @@ -403,7 +403,7 @@ export function createAppNodeInstance(options: { mapWithFailures(outputDataValues, value => { if (outputDataMap.has(value.id)) { collector.report({ - code: 'EXTENSION_FACTORY_DUPLICATE_OUTPUT', + code: 'EXTENSION_OUTPUT_CONFLICT', message: `extension factory output duplicate data '${value.id}'`, context: { dataRefId: value.id, @@ -421,7 +421,7 @@ export function createAppNodeInstance(options: { if (value === undefined) { if (!ref.config.optional) { collector.report({ - code: 'EXTENSION_FACTORY_MISSING_REQUIRED_OUTPUT', + code: 'EXTENSION_OUTPUT_MISSING', message: `missing required extension data output '${ref.id}'`, context: { dataRefId: ref.id, @@ -439,7 +439,7 @@ export function createAppNodeInstance(options: { for (const dataRefId of outputDataMap.keys()) { // TODO: Make this a warning collector.report({ - code: 'EXTENSION_FACTORY_UNEXPECTED_OUTPUT', + code: 'EXTENSION_OUTPUT_IGNORED', message: `unexpected output '${dataRefId}'`, context: { dataRefId: dataRefId, @@ -449,7 +449,7 @@ export function createAppNodeInstance(options: { } } else { collector.report({ - code: 'UNEXPECTED_EXTENSION_VERSION', + code: 'EXTENSION_INVALID', message: `unexpected extension version '${ (internalExtension as any).version }'`, @@ -459,7 +459,7 @@ export function createAppNodeInstance(options: { } catch (e) { if (e !== INSTANTIATION_FAILED) { collector.report({ - code: 'FAILED_TO_INSTANTIATE_EXTENSION', + code: 'EXTENSION_FACTORY_ERROR', message: `Failed to instantiate extension '${id}'${ e.name === 'Error' ? `, ${e.message}` : `; caused by ${e}` }`, diff --git a/packages/frontend-app-api/src/tree/resolveAppNodeSpecs.test.ts b/packages/frontend-app-api/src/tree/resolveAppNodeSpecs.test.ts index 23217f697a..2eacd0ae67 100644 --- a/packages/frontend-app-api/src/tree/resolveAppNodeSpecs.test.ts +++ b/packages/frontend-app-api/src/tree/resolveAppNodeSpecs.test.ts @@ -440,7 +440,7 @@ describe('resolveAppNodeSpecs', () => { expect(collector.collectErrors()).toEqual([ { - code: 'EXTENSION_FORBIDDEN', + code: 'EXTENSION_IGNORED', message: "It is forbidden to override the 'test/forbidden' extension, attempted by the 'test' plugin", context: { @@ -473,7 +473,7 @@ describe('resolveAppNodeSpecs', () => { expect(collector.collectErrors()).toEqual([ { - code: 'EXTENSION_FORBIDDEN', + code: 'EXTENSION_IGNORED', message: "It is forbidden to override the 'forbidden' extension, attempted by the 'forbidden' plugin", context: { @@ -496,7 +496,7 @@ describe('resolveAppNodeSpecs', () => { expect(collector.collectErrors()).toEqual([ { - code: 'EXTENSION_CONFIG_FORBIDDEN', + code: 'INVALID_EXTENSION_CONFIG_KEY', 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 23a1847b43..d3b4ad6723 100644 --- a/packages/frontend-app-api/src/tree/resolveAppNodeSpecs.ts +++ b/packages/frontend-app-api/src/tree/resolveAppNodeSpecs.ts @@ -56,7 +56,7 @@ export function resolveAppNodeSpecs(options: { ) => { if (forbidden.has(extension.id)) { collector.report({ - code: 'EXTENSION_FORBIDDEN', + code: 'EXTENSION_IGNORED', message: `It is forbidden to override the '${extension.id}' extension, attempted by the '${extension.plugin.id}' plugin`, context: { plugin: extension.plugin, @@ -153,13 +153,13 @@ export function resolveAppNodeSpecs(options: { } } - const seendExtensionIds = new Set(); + const seenExtensionIds = new Set(); const deduplicatedExtensions = configuredExtensions.filter( ({ extension, params }) => { - if (seendExtensionIds.has(extension.id)) { + if (seenExtensionIds.has(extension.id)) { collector.report({ - code: 'EXTENSION_DUPLICATED', - message: `The extension '${extension.id}' is duplicated`, + code: 'EXTENSION_IGNORED', + message: `The '${extension.id}' extension from the '${params.plugin.id}' plugin is a duplicate and will be ignored`, context: { plugin: params.plugin, extensionId: extension.id, @@ -167,7 +167,7 @@ export function resolveAppNodeSpecs(options: { }); return false; } - seendExtensionIds.add(extension.id); + seenExtensionIds.add(extension.id); return true; }, ); @@ -178,7 +178,7 @@ export function resolveAppNodeSpecs(options: { if (forbidden.has(extensionId)) { collector.report({ - code: 'EXTENSION_CONFIG_FORBIDDEN', + code: 'INVALID_EXTENSION_CONFIG_KEY', message: `Configuration of the '${extensionId}' extension is forbidden`, context: { extensionId, @@ -206,7 +206,7 @@ export function resolveAppNodeSpecs(options: { order.set(extensionId, existing); } else { collector.report({ - code: 'EXTENSION_CONFIG_UNKNOWN_EXTENSION', + code: 'INVALID_EXTENSION_CONFIG_KEY', message: `Extension ${extensionId} does not exist`, context: { extensionId, diff --git a/packages/frontend-app-api/src/tree/resolveAppTree.test.ts b/packages/frontend-app-api/src/tree/resolveAppTree.test.ts index 79bdf81126..bffc156509 100644 --- a/packages/frontend-app-api/src/tree/resolveAppTree.test.ts +++ b/packages/frontend-app-api/src/tree/resolveAppTree.test.ts @@ -238,7 +238,7 @@ describe('buildAppTree', () => { `); }); - it('emits an error when duplicated extensions are detected', () => { + it('ignores duplicate extensions', () => { const tree = resolveAppTree( 'a', [ @@ -248,13 +248,6 @@ describe('buildAppTree', () => { 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', () => { @@ -300,11 +293,13 @@ describe('buildAppTree', () => { expect(collector.collectErrors()).toEqual([ { - code: 'DUPLICATE_REDIRECT_TARGET', + code: 'EXTENSION_INPUT_REDIRECT_CONFLICT', message: "Duplicate redirect target for input 'test' in extension 'b'", context: { - spec: { ...baseSpec, id: 'b', extension: e2 }, + node: expect.objectContaining({ + spec: { ...baseSpec, id: 'b', extension: e2 }, + }), inputName: 'test', }, }, diff --git a/packages/frontend-app-api/src/tree/resolveAppTree.ts b/packages/frontend-app-api/src/tree/resolveAppTree.ts index fc2c8f845e..f2f31f8950 100644 --- a/packages/frontend-app-api/src/tree/resolveAppTree.ts +++ b/packages/frontend-app-api/src/tree/resolveAppTree.ts @@ -122,15 +122,8 @@ export function resolveAppTree( const redirectTargetsByKey = new Map(); for (const spec of specs) { - // The main check with a more helpful error message happens in resolveAppNodeSpecs + // The main check with a helpful error message happens in resolveAppNodeSpecs if (nodes.has(spec.id)) { - errorCollector.report({ - code: 'DUPLICATE_EXTENSION_ID', - message: `Unexpected duplicate extension id '${spec.id}'`, - context: { - spec, - }, - }); continue; } @@ -144,10 +137,10 @@ export function resolveAppTree( const key = makeRedirectKey(replace); if (redirectTargetsByKey.has(key)) { errorCollector.report({ - code: 'DUPLICATE_REDIRECT_TARGET', + code: 'EXTENSION_INPUT_REDIRECT_CONFLICT', message: `Duplicate redirect target for input '${inputName}' in extension '${spec.id}'`, context: { - spec, + node, inputName, }, }); diff --git a/packages/frontend-app-api/src/wiring/createErrorCollector.ts b/packages/frontend-app-api/src/wiring/createErrorCollector.ts index cc51f146ce..fd028f1f18 100644 --- a/packages/frontend-app-api/src/wiring/createErrorCollector.ts +++ b/packages/frontend-app-api/src/wiring/createErrorCollector.ts @@ -14,41 +14,32 @@ * limitations under the License. */ -import { - AppNode, - AppNodeSpec, - FrontendPlugin, -} from '@backstage/frontend-plugin-api'; +import { AppNode, FrontendPlugin } from '@backstage/frontend-plugin-api'; import { Expand } from '@backstage/types'; type AppErrorTypes = { // resolveAppNodeSpecs - EXTENSION_FORBIDDEN: 'plugin' | 'extensionId'; - EXTENSION_DUPLICATED: 'plugin' | 'extensionId'; - EXTENSION_CONFIG_FORBIDDEN: 'extensionId'; - EXTENSION_CONFIG_UNKNOWN_EXTENSION: 'extensionId'; + EXTENSION_IGNORED: 'plugin' | 'extensionId'; + INVALID_EXTENSION_CONFIG_KEY: 'extensionId'; // resolveAppTree - DUPLICATE_EXTENSION_ID: 'spec'; - DUPLICATE_REDIRECT_TARGET: 'spec' | 'inputName'; + EXTENSION_INPUT_REDIRECT_CONFLICT: 'node' | 'inputName'; // instantiateAppNodeTree - EXTENSION_DUPLICATE_INPUT: 'node' | 'inputName'; - EXTENSION_MISSING_INPUT_DATA: 'node' | 'inputName'; - EXTENSION_TOO_MANY_ATTACHMENTS: 'node' | 'inputName'; - EXTENSION_MISSING_REQUIRED_INPUT: 'node' | 'inputName'; - INVALID_CONFIGURATION: 'node'; - EXTENSION_FACTORY_INVALID_OUTPUT: 'node'; - EXTENSION_FACTORY_DUPLICATE_OUTPUT: 'node' | 'dataRefId'; - EXTENSION_FACTORY_MISSING_REQUIRED_OUTPUT: 'node' | 'dataRefId'; - EXTENSION_FACTORY_UNEXPECTED_OUTPUT: 'node' | 'dataRefId'; - UNEXPECTED_EXTENSION_VERSION: 'node'; - FAILED_TO_INSTANTIATE_EXTENSION: 'node'; + EXTENSION_INPUT_DATA_IGNORED: 'node' | 'inputName'; + EXTENSION_INPUT_DATA_MISSING: 'node' | 'inputName'; + EXTENSION_ATTACHMENT_CONFLICT: 'node' | 'inputName'; + EXTENSION_ATTACHMENT_MISSING: 'node' | 'inputName'; + EXTENSION_CONFIGURATION_INVALID: 'node'; + EXTENSION_INVALID: 'node'; + EXTENSION_OUTPUT_CONFLICT: 'node' | 'dataRefId'; + EXTENSION_OUTPUT_MISSING: 'node' | 'dataRefId'; + EXTENSION_OUTPUT_IGNORED: 'node' | 'dataRefId'; + EXTENSION_FACTORY_ERROR: 'node'; // createSpecializedApp - NO_API_FACTORY: 'node'; + API_EXTENSION_INVALID: 'node'; }; type AppErrorContext = { node?: AppNode; - spec?: AppNodeSpec; plugin?: FrontendPlugin; extensionId?: string; dataRefId?: string; diff --git a/packages/frontend-app-api/src/wiring/createSpecializedApp.tsx b/packages/frontend-app-api/src/wiring/createSpecializedApp.tsx index bcb57ce7e7..a9b7e6864a 100644 --- a/packages/frontend-app-api/src/wiring/createSpecializedApp.tsx +++ b/packages/frontend-app-api/src/wiring/createSpecializedApp.tsx @@ -394,7 +394,7 @@ function createApiFactories(options: { factories.push(apiFactory); } else { options.collector.report({ - code: 'NO_API_FACTORY', + code: 'API_EXTENSION_INVALID', message: `API extension '${apiNode.spec.id}' did not output an API factory`, context: { node: apiNode, From 018de42a012b3d6cb150cda1c3acbc9c98779267 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 7 Sep 2025 13:07:22 +0200 Subject: [PATCH 05/11] frontend-app-api: use EXTENSION_FACTORY_ERROR for invalid return types Signed-off-by: Patrik Oldsberg --- .../src/tree/instantiateAppNodeTree.test.ts | 19 +++++++++---------- .../src/tree/instantiateAppNodeTree.ts | 6 +----- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.test.ts b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.test.ts index 4f1589d3a3..8f892afd56 100644 --- a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.test.ts +++ b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.test.ts @@ -1121,8 +1121,9 @@ describe('instantiateAppNodeTree', () => { ), ).toEqual([ { - code: 'EXTENSION_INVALID', - message: 'extension factory did not provide an iterable object', + code: 'EXTENSION_FACTORY_ERROR', + message: + "Failed to instantiate extension 'test', extension factory did not provide an iterable object", context: { node: expect.anything() }, }, ]); @@ -1138,7 +1139,6 @@ describe('instantiateAppNodeTree', () => { ), ).toEqual([ { - // TODO: This should probably be EXTENSION_INVALID code: 'EXTENSION_FACTORY_ERROR', message: "Failed to instantiate extension 'test', extension factory override did not provide an iterable object", @@ -1157,7 +1157,6 @@ describe('instantiateAppNodeTree', () => { ), ).toEqual([ { - // TODO: This should probably be EXTENSION_INVALID code: 'EXTENSION_FACTORY_ERROR', message: "Failed to instantiate extension 'test', extension factory middleware did not provide an iterable object", @@ -1175,8 +1174,9 @@ describe('instantiateAppNodeTree', () => { ), ).toEqual([ { - code: 'EXTENSION_INVALID', - message: 'extension factory did not provide an iterable object', + code: 'EXTENSION_FACTORY_ERROR', + message: + "Failed to instantiate extension 'test:test', extension factory did not provide an iterable object", context: { node: expect.anything() }, }, ]); @@ -1194,8 +1194,9 @@ describe('instantiateAppNodeTree', () => { ), ).toEqual([ { - code: 'EXTENSION_INVALID', - message: 'extension factory did not provide an iterable object', + code: 'EXTENSION_FACTORY_ERROR', + message: + "Failed to instantiate extension 'test:test', extension factory did not provide an iterable object", context: { node: expect.anything() }, }, ]); @@ -1214,7 +1215,6 @@ describe('instantiateAppNodeTree', () => { ), ).toEqual([ { - // TODO: This should probably be EXTENSION_INVALID code: 'EXTENSION_FACTORY_ERROR', message: "Failed to instantiate extension 'test:test', extension factory did not provide an iterable object", @@ -1236,7 +1236,6 @@ describe('instantiateAppNodeTree', () => { ), ).toEqual([ { - // TODO: This should probably be EXTENSION_INVALID code: 'EXTENSION_FACTORY_ERROR', message: "Failed to instantiate extension 'test:test', original blueprint factory did not provide an iterable object", diff --git a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts index 62bf64ebc7..f8e18cdcbd 100644 --- a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts +++ b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts @@ -392,11 +392,7 @@ export function createAppNodeInstance(options: { typeof outputDataValues !== 'object' || !outputDataValues?.[Symbol.iterator] ) { - collector.report({ - code: 'EXTENSION_INVALID', - message: 'extension factory did not provide an iterable object', - }); - throw INSTANTIATION_FAILED; + throw new Error('extension factory did not provide an iterable object'); } const outputDataMap = new Map(); From 02fced51db0e4f3f8f88da0fe9eb02dbea18d1f6 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 7 Sep 2025 13:41:16 +0200 Subject: [PATCH 06/11] frontend-app-api: rework app error types Signed-off-by: Patrik Oldsberg --- packages/frontend-app-api/report.api.md | 102 +++++++++++-- .../src/tree/instantiateAppNodeTree.test.ts | 2 +- .../src/tree/instantiateAppNodeTree.ts | 4 +- .../src/wiring/createErrorCollector.ts | 140 ++++++++++-------- packages/frontend-app-api/src/wiring/index.ts | 2 +- 5 files changed, 174 insertions(+), 76 deletions(-) diff --git a/packages/frontend-app-api/report.api.md b/packages/frontend-app-api/report.api.md index 1806f47b02..fca32ed38c 100644 --- a/packages/frontend-app-api/report.api.md +++ b/packages/frontend-app-api/report.api.md @@ -5,7 +5,6 @@ ```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'; @@ -18,16 +17,97 @@ 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; +export type AppError = + keyof AppErrorTypes extends infer ICode extends keyof AppErrorTypes + ? ICode extends any + ? { + code: ICode; + message: string; + context: AppErrorTypes[ICode]['context']; + } + : never + : never; + +// @public (undocumented) +export type AppErrorTypes = { + EXTENSION_IGNORED: { + context: { + plugin: FrontendPlugin; + extensionId: string; + }; + }; + INVALID_EXTENSION_CONFIG_KEY: { + context: { + extensionId: string; + }; + }; + EXTENSION_INPUT_REDIRECT_CONFLICT: { + context: { + node: AppNode; + inputName: string; + }; + }; + EXTENSION_INPUT_DATA_IGNORED: { + context: { + node: AppNode; + inputName: string; + }; + }; + EXTENSION_INPUT_DATA_MISSING: { + context: { + node: AppNode; + inputName: string; + }; + }; + EXTENSION_ATTACHMENT_CONFLICT: { + context: { + node: AppNode; + inputName: string; + }; + }; + EXTENSION_ATTACHMENT_MISSING: { + context: { + node: AppNode; + inputName: string; + }; + }; + EXTENSION_CONFIGURATION_INVALID: { + context: { + node: AppNode; + }; + }; + EXTENSION_INVALID: { + context: { + node: AppNode; + }; + }; + EXTENSION_OUTPUT_CONFLICT: { + context: { + node: AppNode; + dataRefId: string; + }; + }; + EXTENSION_OUTPUT_MISSING: { + context: { + node: AppNode; + dataRefId: string; + }; + }; + EXTENSION_OUTPUT_IGNORED: { + context: { + node: AppNode; + dataRefId: string; + }; + }; + EXTENSION_FACTORY_ERROR: { + context: { + node: AppNode; + }; + }; + API_EXTENSION_INVALID: { + context: { + node: AppNode; + }; }; }; diff --git a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.test.ts b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.test.ts index 8f892afd56..13ec6e8a59 100644 --- a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.test.ts +++ b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.test.ts @@ -1098,7 +1098,7 @@ describe('instantiateAppNodeTree', () => { }); const errors = collector.collectErrors(); for (const error of errors ?? []) { - expect(error.context?.node).toBe(node); + expect('node' in error.context && error.context.node).toBe(node); } return errors; } diff --git a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts index f8e18cdcbd..39fc88c19a 100644 --- a/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts +++ b/packages/frontend-app-api/src/tree/instantiateAppNodeTree.ts @@ -95,7 +95,7 @@ function resolveInputDataContainer( extensionData: Array, attachment: AppNode, inputName: string, - collector: ErrorCollector<'node' | 'inputName'>, + collector: ErrorCollector<{ node: AppNode; inputName: string }>, ): { node: AppNode } & ExtensionDataContainer { const dataMap = new Map(); @@ -250,7 +250,7 @@ function resolveV2Inputs( >; }, attachments: ReadonlyMap, - parentCollector: ErrorCollector<'node'>, + parentCollector: ErrorCollector<{ node: AppNode }>, ): ResolvedExtensionInputs<{ [inputName in string]: ExtensionInput< ExtensionDataRef, diff --git a/packages/frontend-app-api/src/wiring/createErrorCollector.ts b/packages/frontend-app-api/src/wiring/createErrorCollector.ts index fd028f1f18..633febd945 100644 --- a/packages/frontend-app-api/src/wiring/createErrorCollector.ts +++ b/packages/frontend-app-api/src/wiring/createErrorCollector.ts @@ -15,63 +15,81 @@ */ import { AppNode, FrontendPlugin } from '@backstage/frontend-plugin-api'; -import { Expand } from '@backstage/types'; -type AppErrorTypes = { +/** + * @public + */ +export type AppErrorTypes = { // resolveAppNodeSpecs - EXTENSION_IGNORED: 'plugin' | 'extensionId'; - INVALID_EXTENSION_CONFIG_KEY: 'extensionId'; - // resolveAppTree - EXTENSION_INPUT_REDIRECT_CONFLICT: 'node' | 'inputName'; - // instantiateAppNodeTree - EXTENSION_INPUT_DATA_IGNORED: 'node' | 'inputName'; - EXTENSION_INPUT_DATA_MISSING: 'node' | 'inputName'; - EXTENSION_ATTACHMENT_CONFLICT: 'node' | 'inputName'; - EXTENSION_ATTACHMENT_MISSING: 'node' | 'inputName'; - EXTENSION_CONFIGURATION_INVALID: 'node'; - EXTENSION_INVALID: 'node'; - EXTENSION_OUTPUT_CONFLICT: 'node' | 'dataRefId'; - EXTENSION_OUTPUT_MISSING: 'node' | 'dataRefId'; - EXTENSION_OUTPUT_IGNORED: 'node' | 'dataRefId'; - EXTENSION_FACTORY_ERROR: 'node'; - // createSpecializedApp - API_EXTENSION_INVALID: 'node'; -}; - -type AppErrorContext = { - node?: AppNode; - plugin?: FrontendPlugin; - extensionId?: string; - dataRefId?: string; - inputName?: string; -}; - -/** @public */ -export type AppError = - { - code: TCode; - message: string; - context: Expand< - AppErrorContext & - Pick< - Required, - keyof AppErrorTypes extends TCode ? never : AppErrorTypes[TCode] - > - >; + EXTENSION_IGNORED: { + context: { plugin: FrontendPlugin; extensionId: string }; }; + INVALID_EXTENSION_CONFIG_KEY: { + context: { extensionId: string }; + }; + // resolveAppTree + EXTENSION_INPUT_REDIRECT_CONFLICT: { + context: { node: AppNode; inputName: string }; + }; + // instantiateAppNodeTree + EXTENSION_INPUT_DATA_IGNORED: { + context: { node: AppNode; inputName: string }; + }; + EXTENSION_INPUT_DATA_MISSING: { + context: { node: AppNode; inputName: string }; + }; + EXTENSION_ATTACHMENT_CONFLICT: { + context: { node: AppNode; inputName: string }; + }; + EXTENSION_ATTACHMENT_MISSING: { + context: { node: AppNode; inputName: string }; + }; + EXTENSION_CONFIGURATION_INVALID: { + context: { node: AppNode }; + }; + EXTENSION_INVALID: { + context: { node: AppNode }; + }; + EXTENSION_OUTPUT_CONFLICT: { + context: { node: AppNode; dataRefId: string }; + }; + EXTENSION_OUTPUT_MISSING: { + context: { node: AppNode; dataRefId: string }; + }; + EXTENSION_OUTPUT_IGNORED: { + context: { node: AppNode; dataRefId: string }; + }; + EXTENSION_FACTORY_ERROR: { + context: { node: AppNode }; + }; + // createSpecializedApp + API_EXTENSION_INVALID: { + context: { node: AppNode }; + }; +}; + +/** + * @public + */ +export type AppError = + keyof AppErrorTypes extends infer ICode extends keyof AppErrorTypes + ? ICode extends any + ? { + code: ICode; + message: string; + context: AppErrorTypes[ICode]['context']; + } + : never + : never; /** @internal */ -export interface ErrorCollector< - TContext extends keyof AppErrorContext = never, -> { - // Type-only: here to make sure that all required keys are present - $$contextKeys: { [K in TContext]: K }; +export interface ErrorCollector { report( - report: Exclude< - AppErrorTypes[TCode], - TContext - > extends infer IContext extends keyof AppErrorContext - ? [IContext] extends [never] + report: Omit< + AppErrorTypes[TCode]['context'], + keyof TContext + > extends infer IContext extends {} + ? [{}] extends [IContext] ? { code: TCode; message: string; @@ -79,28 +97,28 @@ export interface ErrorCollector< : { code: TCode; message: string; - context: Pick, IContext>; + context: IContext; } : never, ): void; - child( - context?: TAdditionalContext & AppErrorContext, - ): ErrorCollector< - (TContext | keyof TAdditionalContext) & keyof AppErrorContext - >; + child( + context: TAdditionalContext, + ): ErrorCollector; collectErrors(): AppError[] | undefined; } /** @internal */ export function createErrorCollector( - context?: AppError['context'], + context?: Partial, ): ErrorCollector { const errors: AppError[] = []; const children: ErrorCollector[] = []; return { - $$contextKeys: null as any, - report(report) { - errors.push({ ...report, context: { ...context, ...report.context } }); + report(report: { code: string; message: string; context?: {} }) { + errors.push({ + ...report, + context: { ...context, ...report.context }, + } as AppError); }, collectErrors() { const allErrors = [ diff --git a/packages/frontend-app-api/src/wiring/index.ts b/packages/frontend-app-api/src/wiring/index.ts index ce7574852a..1e18859ddd 100644 --- a/packages/frontend-app-api/src/wiring/index.ts +++ b/packages/frontend-app-api/src/wiring/index.ts @@ -19,4 +19,4 @@ export { type CreateSpecializedAppOptions, } from './createSpecializedApp'; export { type FrontendPluginInfoResolver } from './createPluginInfoAttacher'; -export { type AppError } from './createErrorCollector'; +export { type AppError, type AppErrorTypes } from './createErrorCollector'; From fa4cd1d47f7f91f2c1ce41900d705dc7cba81c69 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 8 Sep 2025 10:21:58 +0200 Subject: [PATCH 07/11] frontend-defaults: new display for app startup errors Signed-off-by: Patrik Oldsberg --- packages/frontend-defaults/report.api.md | 12 ++ packages/frontend-defaults/src/createApp.tsx | 6 + packages/frontend-defaults/src/index.ts | 1 + .../src/maybeCreateErrorPage.tsx | 116 ++++++++++++++++++ 4 files changed, 135 insertions(+) create mode 100644 packages/frontend-defaults/src/maybeCreateErrorPage.tsx diff --git a/packages/frontend-defaults/report.api.md b/packages/frontend-defaults/report.api.md index 6fd2536251..75392d2dc1 100644 --- a/packages/frontend-defaults/report.api.md +++ b/packages/frontend-defaults/report.api.md @@ -3,6 +3,8 @@ > Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). ```ts +import { AppError } from '@backstage/frontend-app-api'; +import { AppErrorTypes } from '@backstage/frontend-app-api'; import { Config } from '@backstage/config'; import { ConfigApi } from '@backstage/frontend-plugin-api'; import { CreateAppRouteBinder } from '@backstage/frontend-app-api'; @@ -45,6 +47,16 @@ export function discoverAvailableFeatures(config: Config): { features: (FrontendFeature | FrontendFeatureLoader)[]; }; +// @public +export function maybeCreateErrorPage( + app: { + errors?: AppError[]; + }, + options?: { + warningCodes?: Array; + }, +): JSX_2.Element | undefined; + // @public (undocumented) export function resolveAsyncFeatures(options: { config: Config; diff --git a/packages/frontend-defaults/src/createApp.tsx b/packages/frontend-defaults/src/createApp.tsx index 2c8a80791f..4ff67f6d5a 100644 --- a/packages/frontend-defaults/src/createApp.tsx +++ b/packages/frontend-defaults/src/createApp.tsx @@ -36,6 +36,7 @@ import { import appPlugin from '@backstage/plugin-app'; import { discoverAvailableFeatures } from './discovery'; import { resolveAsyncFeatures } from './resolution'; +import { maybeCreateErrorPage } from './maybeCreateErrorPage'; /** * Options for {@link createApp}. @@ -136,6 +137,11 @@ export function createApp(options?: CreateAppOptions): { advanced: options?.advanced, }); + const errorPage = maybeCreateErrorPage(app); + if (errorPage) { + return { default: () => errorPage }; + } + const rootEl = app.tree.root.instance!.getData( coreExtensionData.reactElement, ); diff --git a/packages/frontend-defaults/src/index.ts b/packages/frontend-defaults/src/index.ts index 68d4c72568..37ec62e794 100644 --- a/packages/frontend-defaults/src/index.ts +++ b/packages/frontend-defaults/src/index.ts @@ -24,3 +24,4 @@ export { createApp, type CreateAppOptions } from './createApp'; export { createPublicSignInApp } from './createPublicSignInApp'; export { discoverAvailableFeatures } from './discovery'; export { resolveAsyncFeatures } from './resolution'; +export { maybeCreateErrorPage } from './maybeCreateErrorPage'; diff --git a/packages/frontend-defaults/src/maybeCreateErrorPage.tsx b/packages/frontend-defaults/src/maybeCreateErrorPage.tsx new file mode 100644 index 0000000000..c4bce8cf73 --- /dev/null +++ b/packages/frontend-defaults/src/maybeCreateErrorPage.tsx @@ -0,0 +1,116 @@ +/* + * Copyright 2024 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 { FrontendPluginInfo } from '@backstage/frontend-plugin-api'; +import { JSX, useEffect, useState } from 'react'; +// eslint-disable-next-line @backstage/no-relative-monorepo-imports +// eslint-disable-next-line @backstage/no-relative-monorepo-imports +import { AppError, AppErrorTypes } from '@backstage/frontend-app-api'; + +const DEFAULT_WARNING_CODES: Array = [ + 'EXTENSION_IGNORED', + 'INVALID_EXTENSION_CONFIG_KEY', + 'EXTENSION_INPUT_DATA_IGNORED', + 'EXTENSION_OUTPUT_IGNORED', +]; + +function AppErrorItem(props: { error: AppError }): JSX.Element { + const { context } = props.error; + + const extensionId = + 'extensionId' in context ? context.extensionId : context.node.spec.id; + const node = 'node' in context ? context.node : undefined; + const plugin = 'plugin' in context ? context.plugin : node?.spec.plugin; + const pluginId = plugin?.id ?? 'N/A'; + + const [info, setInfo] = useState(undefined); + useEffect(() => { + plugin?.info().then(setInfo, error => { + // eslint-disable-next-line no-console + console.error(`Failed to load info for plugin ${plugin.id}: ${error}`); + }); + }, [plugin]); + + return ( +
+ {props.error.code}: {props.error.message} + 
+        
extensionId: {extensionId}

+        {pluginId && 
pluginId: {pluginId}
}
+        {info && (
+          

+            package: {info.packageName}@{info.version}
+          

+        )}
+
+
+ ); +} + +function AppErrorPage(props: { errors: AppError[] }): JSX.Element { + return ( +
+

App startup failed

+ {props.errors.map((error, index) => ( + + ))} +
+ ); +} + +/** + * If there are any unrecoverable errors in the app, this will return an error page in the form of a JSX element. + * + * If there are any recoverable errors, they will always be logged as warnings in the console. + * @public + */ +export function maybeCreateErrorPage( + app: { errors?: AppError[] }, + options?: { + warningCodes?: Array; + }, +): JSX.Element | undefined { + if (!app.errors) { + return undefined; + } + + const errors = new Array(); + const warnings = new Array(); + + const warningCodes = new Set(options?.warningCodes ?? DEFAULT_WARNING_CODES); + for (const error of app.errors) { + if (warningCodes.has(error.code)) { + warnings.push(error); + } else { + errors.push(error); + } + } + + if (warnings.length > 0) { + // eslint-disable-next-line no-console + console.warn('App startup encountered warnings:'); + for (const warning of warnings) { + // eslint-disable-next-line no-console + console.warn(`${warning.code}: ${warning.message}`); + } + } + + if (errors.length === 0) { + return undefined; + } + + return ; +} From 4907da392ae556fef1732ebce8dc041d9494d2ba Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 8 Sep 2025 10:28:06 +0200 Subject: [PATCH 08/11] frontend-test-utils: updated to use and report errors via error collector Signed-off-by: Patrik Oldsberg --- .../src/wiring/createExtension.test.ts | 8 ++++---- .../src/app/createExtensionTester.tsx | 17 ++++++++++++++++- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/packages/frontend-plugin-api/src/wiring/createExtension.test.ts b/packages/frontend-plugin-api/src/wiring/createExtension.test.ts index 00dd981e26..59a81f1b59 100644 --- a/packages/frontend-plugin-api/src/wiring/createExtension.test.ts +++ b/packages/frontend-plugin-api/src/wiring/createExtension.test.ts @@ -1078,7 +1078,7 @@ describe('createExtension', () => { .add(multi2Ext) .get(outputRef), ).toThrowErrorMatchingInlineSnapshot( - `"Failed to instantiate extension 'subject', override data provided for input 'multi' must match the length of the original inputs"`, + `"Failed to resolve the extension tree: Failed to instantiate extension 'subject', override data provided for input 'multi' must match the length of the original inputs"`, ); // Mix forward and data override @@ -1101,7 +1101,7 @@ describe('createExtension', () => { .add(multi2Ext) .get(outputRef), ).toThrowErrorMatchingInlineSnapshot( - `"Failed to instantiate extension 'subject', override data for input 'multi' may not mix forwarded inputs with data overrides"`, + `"Failed to resolve the extension tree: Failed to instantiate extension 'subject', override data for input 'multi' may not mix forwarded inputs with data overrides"`, ); // Required input not provided @@ -1124,7 +1124,7 @@ describe('createExtension', () => { .add(multi2Ext) .get(outputRef), ).toThrowErrorMatchingInlineSnapshot( - `"Failed to instantiate extension 'subject', missing required extension data value(s) 'test1'"`, + `"Failed to resolve the extension tree: Failed to instantiate extension 'subject', missing required extension data value(s) 'test1'"`, ); // Wrong value provided @@ -1153,7 +1153,7 @@ describe('createExtension', () => { .add(multi2Ext) .get(outputRef), ).toThrowErrorMatchingInlineSnapshot( - `"Failed to instantiate extension 'subject', extension data 'test2' was provided but not declared"`, + `"Failed to resolve the extension tree: Failed to instantiate extension 'subject', extension data 'test2' was provided but not declared"`, ); }); }); diff --git a/packages/frontend-test-utils/src/app/createExtensionTester.tsx b/packages/frontend-test-utils/src/app/createExtensionTester.tsx index 7b321d4413..50d65fddc7 100644 --- a/packages/frontend-test-utils/src/app/createExtensionTester.tsx +++ b/packages/frontend-test-utils/src/app/createExtensionTester.tsx @@ -35,6 +35,8 @@ import { resolveAppNodeSpecs } from '../../../frontend-app-api/src/tree/resolveA import { instantiateAppNodeTree } from '../../../frontend-app-api/src/tree/instantiateAppNodeTree'; // eslint-disable-next-line @backstage/no-relative-monorepo-imports import { readAppExtensionsConfig } from '../../../frontend-app-api/src/tree/readAppExtensionsConfig'; +// eslint-disable-next-line @backstage/no-relative-monorepo-imports +import { createErrorCollector } from '../../../frontend-app-api/src/wiring/createErrorCollector'; import { TestApiRegistry } from '@backstage/test-utils'; import { OpaqueExtensionDefinition } from '@internal/frontend'; @@ -191,16 +193,29 @@ export class ExtensionTester { ); } + const collector = createErrorCollector(); + const tree = resolveAppTree( subject.id, resolveAppNodeSpecs({ features: [], builtinExtensions: this.#extensions.map(_ => _.extension), parameters: readAppExtensionsConfig(this.#getConfig()), + collector, }), + collector, ); - instantiateAppNodeTree(tree.root, TestApiRegistry.from()); + instantiateAppNodeTree(tree.root, TestApiRegistry.from(), collector); + + const errors = collector.collectErrors(); + if (errors) { + throw new Error( + `Failed to resolve the extension tree: ${errors + .map(e => e.message) + .join(', ')}`, + ); + } this.#tree = tree; From 6516c3d81554ad736965a0cc1d00717e4a8e145c Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 8 Sep 2025 10:32:49 +0200 Subject: [PATCH 09/11] changesets: add changesets for new app error API Signed-off-by: Patrik Oldsberg --- .changeset/loud-forks-teach.md | 5 +++++ .changeset/puny-tires-fold.md | 5 +++++ .changeset/silver-baboons-ring.md | 5 +++++ 3 files changed, 15 insertions(+) create mode 100644 .changeset/loud-forks-teach.md create mode 100644 .changeset/puny-tires-fold.md create mode 100644 .changeset/silver-baboons-ring.md diff --git a/.changeset/loud-forks-teach.md b/.changeset/loud-forks-teach.md new file mode 100644 index 0000000000..662ff458ca --- /dev/null +++ b/.changeset/loud-forks-teach.md @@ -0,0 +1,5 @@ +--- +'@backstage/frontend-test-utils': patch +--- + +Internal update to use and throw app errors. diff --git a/.changeset/puny-tires-fold.md b/.changeset/puny-tires-fold.md new file mode 100644 index 0000000000..e54187cc85 --- /dev/null +++ b/.changeset/puny-tires-fold.md @@ -0,0 +1,5 @@ +--- +'@backstage/frontend-app-api': minor +--- + +The `createSpecializedApp` no longer throws when encountering many common errors when starting up the app. It will instead return them through the `errors` property so that they can be handled more gracefully in the app. diff --git a/.changeset/silver-baboons-ring.md b/.changeset/silver-baboons-ring.md new file mode 100644 index 0000000000..578914b35c --- /dev/null +++ b/.changeset/silver-baboons-ring.md @@ -0,0 +1,5 @@ +--- +'@backstage/frontend-defaults': patch +--- + +The default app now leverages the new error reporting functionality from `@backstage/frontend-app-api`. If there are critical errors during startup, an error screen that shows a summary of all errors will now be shown, rather than leaving the screen blank. Other errors will be logged as warnings in the console. From c89e954bf401ac8097ec496a41ac6036ea3f03ad Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 15 Sep 2025 14:29:34 +0200 Subject: [PATCH 10/11] error collection: review fixes Signed-off-by: Patrik Oldsberg --- packages/frontend-app-api/src/wiring/createErrorCollector.ts | 2 +- packages/frontend-defaults/src/maybeCreateErrorPage.tsx | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/frontend-app-api/src/wiring/createErrorCollector.ts b/packages/frontend-app-api/src/wiring/createErrorCollector.ts index 633febd945..93540cec63 100644 --- a/packages/frontend-app-api/src/wiring/createErrorCollector.ts +++ b/packages/frontend-app-api/src/wiring/createErrorCollector.ts @@ -89,7 +89,7 @@ export interface ErrorCollector { AppErrorTypes[TCode]['context'], keyof TContext > extends infer IContext extends {} - ? [{}] extends [IContext] + ? {} extends IContext ? { code: TCode; message: string; diff --git a/packages/frontend-defaults/src/maybeCreateErrorPage.tsx b/packages/frontend-defaults/src/maybeCreateErrorPage.tsx index c4bce8cf73..40da64e987 100644 --- a/packages/frontend-defaults/src/maybeCreateErrorPage.tsx +++ b/packages/frontend-defaults/src/maybeCreateErrorPage.tsx @@ -16,8 +16,6 @@ import { FrontendPluginInfo } from '@backstage/frontend-plugin-api'; import { JSX, useEffect, useState } from 'react'; -// eslint-disable-next-line @backstage/no-relative-monorepo-imports -// eslint-disable-next-line @backstage/no-relative-monorepo-imports import { AppError, AppErrorTypes } from '@backstage/frontend-app-api'; const DEFAULT_WARNING_CODES: Array = [ From d89815ce9f26375af32c516af8661c4b3c8ef631 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 15 Sep 2025 16:55:32 +0200 Subject: [PATCH 11/11] frontend-app-api: add collection of route binding errors Signed-off-by: Patrik Oldsberg --- packages/frontend-app-api/report.api.md | 15 +++ .../src/routing/collectRouteIds.test.ts | 65 ++++++++- .../src/routing/collectRouteIds.ts | 20 ++- .../src/routing/resolveRouteBindings.test.ts | 127 ++++++++++++------ .../src/routing/resolveRouteBindings.ts | 38 ++++-- .../src/wiring/createErrorCollector.ts | 10 ++ .../src/wiring/createSpecializedApp.tsx | 4 +- .../src/maybeCreateErrorPage.tsx | 8 +- 8 files changed, 220 insertions(+), 67 deletions(-) diff --git a/packages/frontend-app-api/report.api.md b/packages/frontend-app-api/report.api.md index fca32ed38c..bd0709813b 100644 --- a/packages/frontend-app-api/report.api.md +++ b/packages/frontend-app-api/report.api.md @@ -109,6 +109,21 @@ export type AppErrorTypes = { node: AppNode; }; }; + ROUTE_DUPLICATE: { + context: { + routeId: string; + }; + }; + ROUTE_BINDING_INVALID_VALUE: { + context: { + routeId: string; + }; + }; + ROUTE_NOT_FOUND: { + context: { + routeId: string; + }; + }; }; // @public diff --git a/packages/frontend-app-api/src/routing/collectRouteIds.test.ts b/packages/frontend-app-api/src/routing/collectRouteIds.test.ts index 395886f982..b1bab3fbbe 100644 --- a/packages/frontend-app-api/src/routing/collectRouteIds.test.ts +++ b/packages/frontend-app-api/src/routing/collectRouteIds.test.ts @@ -20,6 +20,18 @@ import { createFrontendPlugin, } from '@backstage/frontend-plugin-api'; import { collectRouteIds } from './collectRouteIds'; +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(', ')}`, + ); + } +}); describe('collectRouteIds', () => { it('should assign IDs to routes', () => { @@ -33,13 +45,16 @@ describe('collectRouteIds', () => { /^ExternalRouteRef\{created at '.*collectRouteIds\.test\.ts.*'\}$/, ); - const collected = collectRouteIds([ - createFrontendPlugin({ - pluginId: 'test', - routes: { ref }, - externalRoutes: { extRef }, - }), - ]); + const collected = collectRouteIds( + [ + createFrontendPlugin({ + pluginId: 'test', + routes: { ref }, + externalRoutes: { extRef }, + }), + ], + collector, + ); expect(Object.fromEntries(collected.routes)).toEqual({ 'test.ref': ref, }); @@ -50,4 +65,40 @@ describe('collectRouteIds', () => { expect(String(ref)).toBe('RouteRef{test.ref}'); expect(String(extRef)).toBe('ExternalRouteRef{test.extRef}'); }); + + it('should report duplicate route IDs', () => { + const ref = createRouteRef(); + const extRef = createExternalRouteRef(); + + collectRouteIds( + [ + createFrontendPlugin({ + pluginId: 'test', + routes: { 'mid.ref': ref }, + externalRoutes: { 'mid.extRef': extRef }, + }), + createFrontendPlugin({ + pluginId: 'test.mid', + routes: { ref }, + externalRoutes: { extRef }, + }), + ], + collector, + ); + + expect(collector.collectErrors()).toEqual([ + { + code: 'ROUTE_DUPLICATE', + message: + "Duplicate route id 'test.mid.ref' encountered while collecting routes", + context: { routeId: 'test.mid.ref' }, + }, + { + code: 'ROUTE_DUPLICATE', + message: + "Duplicate external route id 'test.mid.extRef' encountered while collecting routes", + context: { routeId: 'test.mid.extRef' }, + }, + ]); + }); }); diff --git a/packages/frontend-app-api/src/routing/collectRouteIds.ts b/packages/frontend-app-api/src/routing/collectRouteIds.ts index 8f189bda81..79afb07928 100644 --- a/packages/frontend-app-api/src/routing/collectRouteIds.ts +++ b/packages/frontend-app-api/src/routing/collectRouteIds.ts @@ -30,6 +30,7 @@ import { toInternalExternalRouteRef } from '../../../frontend-plugin-api/src/rou // eslint-disable-next-line @backstage/no-relative-monorepo-imports import { toInternalSubRouteRef } from '../../../frontend-plugin-api/src/routing/SubRouteRef'; import { OpaqueFrontendPlugin } from '@internal/frontend'; +import { ErrorCollector } from '../wiring/createErrorCollector'; /** @internal */ export interface RouteRefsById { @@ -38,7 +39,10 @@ export interface RouteRefsById { } /** @internal */ -export function collectRouteIds(features: FrontendFeature[]): RouteRefsById { +export function collectRouteIds( + features: FrontendFeature[], + collector: ErrorCollector, +): RouteRefsById { const routesById = new Map(); const externalRoutesById = new Map(); @@ -50,7 +54,12 @@ export function collectRouteIds(features: FrontendFeature[]): RouteRefsById { for (const [name, ref] of Object.entries(feature.routes)) { const refId = `${feature.id}.${name}`; if (routesById.has(refId)) { - throw new Error(`Unexpected duplicate route '${refId}'`); + collector.report({ + code: 'ROUTE_DUPLICATE', + message: `Duplicate route id '${refId}' encountered while collecting routes`, + context: { routeId: refId }, + }); + continue; } if (isRouteRef(ref)) { @@ -65,7 +74,12 @@ export function collectRouteIds(features: FrontendFeature[]): RouteRefsById { for (const [name, ref] of Object.entries(feature.externalRoutes)) { const refId = `${feature.id}.${name}`; if (externalRoutesById.has(refId)) { - throw new Error(`Unexpected duplicate external route '${refId}'`); + collector.report({ + code: 'ROUTE_DUPLICATE', + message: `Duplicate external route id '${refId}' encountered while collecting routes`, + context: { routeId: refId }, + }); + continue; } const internalRef = toInternalExternalRouteRef(ref); diff --git a/packages/frontend-app-api/src/routing/resolveRouteBindings.test.ts b/packages/frontend-app-api/src/routing/resolveRouteBindings.test.ts index 6618bef152..003980e617 100644 --- a/packages/frontend-app-api/src/routing/resolveRouteBindings.test.ts +++ b/packages/frontend-app-api/src/routing/resolveRouteBindings.test.ts @@ -20,6 +20,18 @@ import { } from '@backstage/frontend-plugin-api'; import { resolveRouteBindings } from './resolveRouteBindings'; import { ConfigReader } from '@backstage/config'; +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 emptyIds = { routes: new Map(), externalRoutes: new Map() }; @@ -33,23 +45,30 @@ describe('resolveRouteBindings', () => { }, new ConfigReader({}), emptyIds, + collector, ); expect(result.get(external.myRoute)).toBe(ref); }); - it('throws on unknown keys', () => { + it('reports error on unknown keys', () => { const external = { myRoute: createExternalRouteRef() }; const ref = createRouteRef(); - expect(() => - resolveRouteBindings( - ({ bind }) => { - bind(external, { someOtherRoute: ref } as any); - }, - new ConfigReader({}), - emptyIds, - ), - ).toThrow('Key someOtherRoute is not an existing external route'); + resolveRouteBindings( + ({ bind }) => { + bind(external, { someOtherRoute: ref } as any); + }, + new ConfigReader({}), + emptyIds, + collector, + ); + expect(collector.collectErrors()).toEqual([ + { + code: 'ROUTE_NOT_FOUND', + message: 'Key someOtherRoute is not an existing external route', + context: { routeId: 'someOtherRoute' }, + }, + ]); }); it('reads bindings from config', () => { @@ -64,6 +83,7 @@ describe('resolveRouteBindings', () => { routes: new Map([['myTarget', myTarget]]), externalRoutes: new Map([['mySource', mySource]]), }, + collector, ); expect(result.get(mySource)).toBe(myTarget); @@ -85,6 +105,7 @@ describe('resolveRouteBindings', () => { routes: new Map([['myTarget', myTarget]]), externalRoutes: new Map([['mySource', mySource]]), }, + collector, ).get(mySource), ).toBe(undefined); @@ -100,57 +121,74 @@ describe('resolveRouteBindings', () => { routes: new Map([['myTarget', myTarget]]), externalRoutes: new Map([['mySource', mySource]]), }, + collector, ).get(mySource), ).toBe(myTarget); }); - it('throws on invalid config', () => { + it('reports errors on invalid config', () => { expect(() => resolveRouteBindings( () => {}, new ConfigReader({ app: { routes: { bindings: 'derp' } } }), emptyIds, + collector, ), ).toThrow( "Invalid type in config for key 'app.routes.bindings' in 'mock-config', got string, wanted object", ); - expect(() => - resolveRouteBindings( - () => {}, - new ConfigReader({ app: { routes: { bindings: { mySource: true } } } }), - emptyIds, - ), - ).toThrow( - "Invalid config at app.routes.bindings['mySource'], value must be a non-empty string", + resolveRouteBindings( + () => {}, + new ConfigReader({ app: { routes: { bindings: { mySource: true } } } }), + emptyIds, + collector, ); + expect(collector.collectErrors()).toEqual([ + { + code: 'ROUTE_BINDING_INVALID_VALUE', + message: + "Invalid config at app.routes.bindings['mySource'], value must be a non-empty string or false", + context: { routeId: 'mySource' }, + }, + ]); - expect(() => - resolveRouteBindings( - () => {}, - new ConfigReader({ - app: { routes: { bindings: { mySource: 'myTarget' } } }, - }), - emptyIds, - ), - ).toThrow( - "Invalid config at app.routes.bindings, 'mySource' is not a valid external route", + resolveRouteBindings( + () => {}, + new ConfigReader({ + app: { routes: { bindings: { mySource: 'myTarget' } } }, + }), + emptyIds, + collector, ); + expect(collector.collectErrors()).toEqual([ + { + code: 'ROUTE_NOT_FOUND', + message: + "Invalid config at app.routes.bindings, 'mySource' is not a valid external route", + context: { routeId: 'mySource' }, + }, + ]); - expect(() => - resolveRouteBindings( - () => {}, - new ConfigReader({ - app: { routes: { bindings: { mySource: 'myTarget' } } }, - }), - { - ...emptyIds, - externalRoutes: new Map([['mySource', createExternalRouteRef()]]), - }, - ), - ).toThrow( - "Invalid config at app.routes.bindings['mySource'], 'myTarget' is not a valid route", + resolveRouteBindings( + () => {}, + new ConfigReader({ + app: { routes: { bindings: { mySource: 'myTarget' } } }, + }), + { + ...emptyIds, + externalRoutes: new Map([['mySource', createExternalRouteRef()]]), + }, + collector, ); + expect(collector.collectErrors()).toEqual([ + { + code: 'ROUTE_NOT_FOUND', + message: + "Invalid config at app.routes.bindings['mySource'], 'myTarget' is not a valid route", + context: { routeId: 'myTarget' }, + }, + ]); }); it('can have default targets, but at the lowest priority', () => { @@ -170,6 +208,7 @@ describe('resolveRouteBindings', () => { () => {}, new ConfigReader({}), routesById, + collector, ); expect(result.get(source)).toBe(target1); @@ -181,6 +220,7 @@ describe('resolveRouteBindings', () => { app: { routes: { bindings: { source: 'target2' } } }, }), routesById, + collector, ); expect(result.get(source)).toBe(target2); @@ -192,6 +232,7 @@ describe('resolveRouteBindings', () => { }, new ConfigReader({}), routesById, + collector, ); expect(result.get(source)).toBe(target2); @@ -210,6 +251,7 @@ describe('resolveRouteBindings', () => { () => {}, new ConfigReader({}), routesById, + collector, ); expect(result.get(source)).toBe(target1); @@ -219,6 +261,7 @@ describe('resolveRouteBindings', () => { () => {}, new ConfigReader({ app: { routes: { bindings: { source: false } } } }), routesById, + collector, ); expect(result.get(source)).toBe(undefined); diff --git a/packages/frontend-app-api/src/routing/resolveRouteBindings.ts b/packages/frontend-app-api/src/routing/resolveRouteBindings.ts index 76d620c20c..f9836d7107 100644 --- a/packages/frontend-app-api/src/routing/resolveRouteBindings.ts +++ b/packages/frontend-app-api/src/routing/resolveRouteBindings.ts @@ -20,6 +20,7 @@ import { ExternalRouteRef, } from '@backstage/frontend-plugin-api'; import { RouteRefsById } from './collectRouteIds'; +import { ErrorCollector } from '../wiring/createErrorCollector'; import { Config } from '@backstage/config'; import { JsonObject } from '@backstage/types'; // eslint-disable-next-line @backstage/no-relative-monorepo-imports @@ -80,6 +81,7 @@ export function resolveRouteBindings( bindRoutes: ((context: { bind: CreateAppRouteBinder }) => void) | undefined, config: Config, routesById: RouteRefsById, + collector: ErrorCollector, ): Map { const result = new Map(); const disabledExternalRefs = new Set(); @@ -93,7 +95,12 @@ export function resolveRouteBindings( for (const [key, value] of Object.entries(targetRoutes)) { const externalRoute = externalRoutes[key]; if (!externalRoute) { - throw new Error(`Key ${key} is not an existing external route`); + collector.report({ + code: 'ROUTE_NOT_FOUND', + message: `Key ${key} is not an existing external route`, + context: { routeId: String(key) }, + }); + continue; } if (value) { result.set(externalRoute, value); @@ -112,16 +119,22 @@ export function resolveRouteBindings( if (bindings) { for (const [externalRefId, targetRefId] of Object.entries(bindings)) { if (!isValidTargetRefId(targetRefId)) { - throw new Error( - `Invalid config at app.routes.bindings['${externalRefId}'], value must be a non-empty string or false`, - ); + collector.report({ + code: 'ROUTE_BINDING_INVALID_VALUE', + message: `Invalid config at app.routes.bindings['${externalRefId}'], value must be a non-empty string or false`, + context: { routeId: externalRefId }, + }); + continue; } const externalRef = routesById.externalRoutes.get(externalRefId); if (!externalRef) { - throw new Error( - `Invalid config at app.routes.bindings, '${externalRefId}' is not a valid external route`, - ); + collector.report({ + code: 'ROUTE_NOT_FOUND', + message: `Invalid config at app.routes.bindings, '${externalRefId}' is not a valid external route`, + context: { routeId: externalRefId }, + }); + continue; } // Skip if binding was already defined in code @@ -134,9 +147,14 @@ export function resolveRouteBindings( } else { const targetRef = routesById.routes.get(targetRefId); if (!targetRef) { - throw new Error( - `Invalid config at app.routes.bindings['${externalRefId}'], '${targetRefId}' is not a valid route`, - ); + collector.report({ + code: 'ROUTE_NOT_FOUND', + message: `Invalid config at app.routes.bindings['${externalRefId}'], '${String( + targetRefId, + )}' is not a valid route`, + context: { routeId: String(targetRefId) }, + }); + continue; } result.set(externalRef, targetRef); diff --git a/packages/frontend-app-api/src/wiring/createErrorCollector.ts b/packages/frontend-app-api/src/wiring/createErrorCollector.ts index 93540cec63..c20e92d0f0 100644 --- a/packages/frontend-app-api/src/wiring/createErrorCollector.ts +++ b/packages/frontend-app-api/src/wiring/createErrorCollector.ts @@ -66,6 +66,16 @@ export type AppErrorTypes = { API_EXTENSION_INVALID: { context: { node: AppNode }; }; + // routing + ROUTE_DUPLICATE: { + context: { routeId: string }; + }; + ROUTE_BINDING_INVALID_VALUE: { + context: { routeId: string }; + }; + ROUTE_NOT_FOUND: { + context: { routeId: string }; + }; }; /** diff --git a/packages/frontend-app-api/src/wiring/createSpecializedApp.tsx b/packages/frontend-app-api/src/wiring/createSpecializedApp.tsx index a9b7e6864a..6c6d75415b 100644 --- a/packages/frontend-app-api/src/wiring/createSpecializedApp.tsx +++ b/packages/frontend-app-api/src/wiring/createSpecializedApp.tsx @@ -316,9 +316,9 @@ export function createSpecializedApp(options?: CreateSpecializedAppOptions): { const appBasePath = getBasePath(config); const appTreeApi = new AppTreeApiProxy(tree, appBasePath); - const routeRefsById = collectRouteIds(features); + const routeRefsById = collectRouteIds(features, collector); const routeResolutionApi = new RouteResolutionApiProxy( - resolveRouteBindings(options?.bindRoutes, config, routeRefsById), + resolveRouteBindings(options?.bindRoutes, config, routeRefsById, collector), appBasePath, ); diff --git a/packages/frontend-defaults/src/maybeCreateErrorPage.tsx b/packages/frontend-defaults/src/maybeCreateErrorPage.tsx index 40da64e987..6bad97f666 100644 --- a/packages/frontend-defaults/src/maybeCreateErrorPage.tsx +++ b/packages/frontend-defaults/src/maybeCreateErrorPage.tsx @@ -28,9 +28,10 @@ const DEFAULT_WARNING_CODES: Array = [ function AppErrorItem(props: { error: AppError }): JSX.Element { const { context } = props.error; - const extensionId = - 'extensionId' in context ? context.extensionId : context.node.spec.id; const node = 'node' in context ? context.node : undefined; + const extensionId = + 'extensionId' in context ? context.extensionId : node?.spec.id; + const routeId = 'routeId' in context ? context.routeId : undefined; const plugin = 'plugin' in context ? context.plugin : node?.spec.plugin; const pluginId = plugin?.id ?? 'N/A'; @@ -46,7 +47,8 @@ function AppErrorItem(props: { error: AppError }): JSX.Element {
{props.error.code}: {props.error.message} 
-        
extensionId: {extensionId}

+        {extensionId && 
extensionId: {extensionId}
}
+        {routeId && 
routeId: {routeId}
}
         {pluginId && 
pluginId: {pluginId}
}
         {info && (