frontend-app-api: isolate invalid feature flag registrations

Wrap each feature flag registration in a try/catch so that a single invalid flag name (e.g. containing a slash) is reported through the error collector instead of crashing the entire app at bootstrap. Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com> Made-with: Cursor
2026-04-23 00:38:05 +02:00
parent 3b8c0557c6
commit b6ca666812
6 changed files with 355 additions and 19 deletions
@@ -0,0 +1,5 @@
+---
+'@backstage/frontend-app-api': patch
+---
+
+Invalid feature flag declarations no longer crash the app during bootstrap. They are now reported through the error collector and skipped, letting the rest of the app load normally.
@@ -148,6 +148,13 @@ export type AppErrorTypes = {
      bootstrapPluginId: string;
    };
  };
+  FEATURE_FLAG_INVALID: {
+    context: {
+      pluginId: string;
+      flagName: string;
+      error: Error;
+    };
+  };
  ROUTE_DUPLICATE: {
    context: {
      routeId: string;
@@ -0,0 +1,295 @@
+/*
+ * 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 {
+  createFrontendPlugin,
+  createFrontendModule,
+  featureFlagsApiRef,
+  createApiRef,
+  type FeatureFlagsApi,
+} from '@backstage/frontend-plugin-api';
+import {
+  registerFeatureFlagDeclarationsInHolder,
+  wrapFeatureFlagApiFactory,
+} from './apiFactories';
+import { createErrorCollector } from './createErrorCollector';
+
+function createValidatingMockFeatureFlagsApi(): FeatureFlagsApi & {
+  flags: Array<{ name: string; pluginId: string }>;
+} {
+  const flagNameRegex = /^[a-z]+[a-z0-9-]+$/;
+  const flags = new Array<{ name: string; pluginId: string }>();
+  return {
+    flags,
+    registerFlag(flag) {
+      if (!flagNameRegex.test(flag.name)) {
+        throw new Error(
+          `The '${flag.name}' feature flag must start with a lowercase letter and only contain lowercase letters, numbers and hyphens.`,
+        );
+      }
+      flags.push(flag);
+    },
+    getRegisteredFlags() {
+      return flags;
+    },
+    isActive() {
+      return false;
+    },
+    save() {},
+  };
+}
+
+describe('registerFeatureFlagDeclarationsInHolder', () => {
+  it('should isolate invalid flags and still register valid ones from a plugin', () => {
+    const featureFlagsApi = createValidatingMockFeatureFlagsApi();
+    const collector = createErrorCollector();
+
+    const plugin = createFrontendPlugin({
+      pluginId: 'test',
+      featureFlags: [
+        { name: 'valid-flag' },
+        { name: 'bad/flag' },
+        { name: 'another-valid' },
+      ],
+      extensions: [],
+    });
+
+    registerFeatureFlagDeclarationsInHolder(
+      {
+        get: ref =>
+          (ref.id === featureFlagsApiRef.id
+            ? featureFlagsApi
+            : undefined) as any,
+      },
+      [plugin],
+      collector,
+    );
+
+    expect(featureFlagsApi.flags).toEqual([
+      expect.objectContaining({ name: 'valid-flag', pluginId: 'test' }),
+      expect.objectContaining({ name: 'another-valid', pluginId: 'test' }),
+    ]);
+
+    const errors = collector.collectErrors()!;
+    expect(errors).toHaveLength(1);
+    expect(errors[0]).toMatchObject({
+      code: 'FEATURE_FLAG_INVALID',
+      context: { pluginId: 'test', flagName: 'bad/flag' },
+    });
+    expect(errors[0].message).toContain("Plugin 'test'");
+    expect(errors[0].message).toContain("'bad/flag'");
+  });
+
+  it('should report each invalid flag independently across multiple plugins', () => {
+    const featureFlagsApi = createValidatingMockFeatureFlagsApi();
+    const collector = createErrorCollector();
+
+    const pluginA = createFrontendPlugin({
+      pluginId: 'alpha',
+      featureFlags: [{ name: 'ok-flag' }, { name: 'UPPER' }],
+      extensions: [],
+    });
+    const pluginB = createFrontendPlugin({
+      pluginId: 'beta',
+      featureFlags: [{ name: 'x/y' }, { name: 'good-one' }],
+      extensions: [],
+    });
+
+    registerFeatureFlagDeclarationsInHolder(
+      {
+        get: ref =>
+          (ref.id === featureFlagsApiRef.id
+            ? featureFlagsApi
+            : undefined) as any,
+      },
+      [pluginA, pluginB],
+      collector,
+    );
+
+    expect(featureFlagsApi.flags).toEqual([
+      expect.objectContaining({ name: 'ok-flag', pluginId: 'alpha' }),
+      expect.objectContaining({ name: 'good-one', pluginId: 'beta' }),
+    ]);
+
+    const errors = collector.collectErrors()!;
+    expect(errors).toHaveLength(2);
+    expect(errors).toEqual(
+      expect.arrayContaining([
+        expect.objectContaining({
+          code: 'FEATURE_FLAG_INVALID',
+          context: expect.objectContaining({
+            pluginId: 'alpha',
+            flagName: 'UPPER',
+          }),
+        }),
+        expect.objectContaining({
+          code: 'FEATURE_FLAG_INVALID',
+          context: expect.objectContaining({
+            pluginId: 'beta',
+            flagName: 'x/y',
+          }),
+        }),
+      ]),
+    );
+  });
+
+  it('should isolate invalid flags declared by a frontend module', () => {
+    const featureFlagsApi = createValidatingMockFeatureFlagsApi();
+    const collector = createErrorCollector();
+
+    const mod = createFrontendModule({
+      pluginId: 'my-plugin',
+      featureFlags: [{ name: 'mod-valid' }, { name: 'mod/invalid' }],
+      extensions: [],
+    });
+
+    registerFeatureFlagDeclarationsInHolder(
+      {
+        get: ref =>
+          (ref.id === featureFlagsApiRef.id
+            ? featureFlagsApi
+            : undefined) as any,
+      },
+      [mod],
+      collector,
+    );
+
+    expect(featureFlagsApi.flags).toEqual([
+      expect.objectContaining({ name: 'mod-valid', pluginId: 'my-plugin' }),
+    ]);
+
+    const errors = collector.collectErrors()!;
+    expect(errors).toHaveLength(1);
+    expect(errors[0]).toMatchObject({
+      code: 'FEATURE_FLAG_INVALID',
+      context: { pluginId: 'my-plugin', flagName: 'mod/invalid' },
+    });
+  });
+
+  it('should isolate non-validation errors thrown by registerFlag', () => {
+    const featureFlagsApi: FeatureFlagsApi = {
+      registerFlag() {
+        throw new Error('database connection lost');
+      },
+      getRegisteredFlags: () => [],
+      isActive: () => false,
+      save() {},
+    };
+    const collector = createErrorCollector();
+
+    const plugin = createFrontendPlugin({
+      pluginId: 'broken',
+      featureFlags: [{ name: 'any-flag' }],
+      extensions: [],
+    });
+
+    registerFeatureFlagDeclarationsInHolder(
+      {
+        get: ref =>
+          (ref.id === featureFlagsApiRef.id
+            ? featureFlagsApi
+            : undefined) as any,
+      },
+      [plugin],
+      collector,
+    );
+
+    const errors = collector.collectErrors()!;
+    expect(errors).toHaveLength(1);
+    expect(errors[0]).toMatchObject({
+      code: 'FEATURE_FLAG_INVALID',
+      context: expect.objectContaining({
+        pluginId: 'broken',
+        flagName: 'any-flag',
+      }),
+    });
+    expect(errors[0].message).toContain('database connection lost');
+  });
+});
+
+describe('wrapFeatureFlagApiFactory', () => {
+  it('should not wrap factories for non-feature-flag APIs', () => {
+    const otherApiRef = createApiRef<{}>({ id: 'test.other' });
+    const factory = {
+      api: otherApiRef,
+      deps: {},
+      factory: () => ({}),
+    };
+    const collector = createErrorCollector();
+
+    const result = wrapFeatureFlagApiFactory(factory, [], collector);
+    expect(result).toBe(factory);
+  });
+
+  it('should wrap the feature flags factory and isolate invalid flags', () => {
+    const featureFlagsApi = createValidatingMockFeatureFlagsApi();
+    const collector = createErrorCollector();
+
+    const plugin = createFrontendPlugin({
+      pluginId: 'test',
+      featureFlags: [{ name: 'good-flag' }, { name: 'bad/flag' }],
+      extensions: [],
+    });
+
+    const factory = {
+      api: featureFlagsApiRef,
+      deps: {},
+      factory: () => featureFlagsApi,
+    };
+
+    const wrapped = wrapFeatureFlagApiFactory(factory, [plugin], collector);
+    expect(wrapped).not.toBe(factory);
+
+    const result = wrapped.factory({});
+    expect(result).toBe(featureFlagsApi);
+    expect(featureFlagsApi.flags).toEqual([
+      expect.objectContaining({ name: 'good-flag', pluginId: 'test' }),
+    ]);
+
+    const errors = collector.collectErrors()!;
+    expect(errors).toHaveLength(1);
+    expect(errors[0]).toMatchObject({
+      code: 'FEATURE_FLAG_INVALID',
+      context: { pluginId: 'test', flagName: 'bad/flag' },
+    });
+  });
+
+  it('should not throw from the wrapped factory when flags are invalid', () => {
+    const featureFlagsApi = createValidatingMockFeatureFlagsApi();
+    const collector = createErrorCollector();
+
+    const plugin = createFrontendPlugin({
+      pluginId: 'crashy',
+      featureFlags: [{ name: 'inspt/show-test-banner' }],
+      extensions: [],
+    });
+
+    const factory = {
+      api: featureFlagsApiRef,
+      deps: {},
+      factory: () => featureFlagsApi,
+    };
+
+    const wrapped = wrapFeatureFlagApiFactory(factory, [plugin], collector);
+
+    expect(() => wrapped.factory({})).not.toThrow();
+    expect(featureFlagsApi.flags).toEqual([]);
+
+    const errors = collector.collectErrors()!;
+    expect(errors).toHaveLength(1);
+    expect(errors[0].code).toBe('FEATURE_FLAG_INVALID');
+  });
+});
@@ -51,10 +51,11 @@ export type ApiFactoryEntry = {
 export function registerFeatureFlagDeclarationsInHolder(
  apis: ApiHolder,
  features: FrontendFeature[],
+  collector: ErrorCollector,
 ) {
  const featureFlagApi = apis.get(featureFlagsApiRef);
  if (featureFlagApi) {
-    registerFeatureFlagDeclarations(featureFlagApi, features);
+    registerFeatureFlagDeclarations(featureFlagApi, features, collector);
  }
 }

@@ -65,6 +66,7 @@ export function registerFeatureFlagDeclarationsInHolder(
 export function wrapFeatureFlagApiFactory(
  factory: AnyApiFactory,
  features: FrontendFeature[],
+  collector: ErrorCollector,
 ) {
  if (factory.api.id !== featureFlagsApiRef.id) {
    return factory;
@@ -76,7 +78,7 @@ export function wrapFeatureFlagApiFactory(
      const featureFlagApi = factory.factory(
        deps,
      ) as typeof featureFlagsApiRef.T;
-      registerFeatureFlagDeclarations(featureFlagApi, features);
+      registerFeatureFlagDeclarations(featureFlagApi, features, collector);
      return featureFlagApi;
    },
  } as AnyApiFactory;
@@ -137,7 +139,11 @@ export function syncFinalApiFactories(options: {
    return true;
  });
  const changedFactories = changedEntries.map(entry =>
-    wrapFeatureFlagApiFactory(entry.factory, options.features),
+    wrapFeatureFlagApiFactory(
+      entry.factory,
+      options.features,
+      options.collector,
+    ),
  );
  options.appApiRegistry.setAll(changedFactories);
  options.apiResolver.invalidate(
@@ -174,25 +180,45 @@ const EMPTY_API_HOLDER: ApiHolder = {
 function registerFeatureFlagDeclarations(
  featureFlagApi: typeof featureFlagsApiRef.T,
  features: FrontendFeature[],
+  collector: ErrorCollector,
 ) {
  for (const feature of features) {
    if (OpaqueFrontendPlugin.isType(feature)) {
-      OpaqueFrontendPlugin.toInternal(feature).featureFlags.forEach(flag =>
-        featureFlagApi.registerFlag({
-          name: flag.name,
-          description: flag.description,
-          pluginId: feature.id,
-        }),
-      );
+      const pluginId = feature.id;
+      for (const flag of OpaqueFrontendPlugin.toInternal(feature)
+        .featureFlags) {
+        try {
+          featureFlagApi.registerFlag({
+            name: flag.name,
+            description: flag.description,
+            pluginId,
+          });
+        } catch (error) {
+          collector.report({
+            code: 'FEATURE_FLAG_INVALID',
+            message: `Plugin '${pluginId}' declared invalid feature flag '${flag.name}': ${error}`,
+            context: { pluginId, flagName: flag.name, error: error as Error },
+          });
+        }
+      }
    }
    if (isInternalFrontendModule(feature)) {
-      toInternalFrontendModule(feature).featureFlags.forEach(flag =>
-        featureFlagApi.registerFlag({
-          name: flag.name,
-          description: flag.description,
-          pluginId: feature.pluginId,
-        }),
-      );
+      const pluginId = feature.pluginId;
+      for (const flag of toInternalFrontendModule(feature).featureFlags) {
+        try {
+          featureFlagApi.registerFlag({
+            name: flag.name,
+            description: flag.description,
+            pluginId,
+          });
+        } catch (error) {
+          collector.report({
+            code: 'FEATURE_FLAG_INVALID',
+            message: `Plugin '${pluginId}' declared invalid feature flag '${flag.name}': ${error}`,
+            context: { pluginId, flagName: flag.name, error: error as Error },
+          });
+        }
+      }
    }
  }
 }
@@ -97,6 +97,9 @@ export type AppErrorTypes = {
      bootstrapPluginId: string;
    };
  };
+  FEATURE_FLAG_INVALID: {
+    context: { pluginId: string; flagName: string; error: Error };
+  };
  // routing
  ROUTE_DUPLICATE: {
    context: { routeId: string };
@@ -342,7 +342,7 @@ export function prepareSpecializedApp(
  if (providedApis) {
    // Reused session state already carries a fully prepared API holder, so the
    // bootstrap path only needs to register feature flag declarations on top.
-    registerFeatureFlagDeclarationsInHolder(providedApis, features);
+    registerFeatureFlagDeclarationsInHolder(providedApis, features, collector);
  } else {
    // Bootstrap materializes only the immediately visible API factories. Any
    // predicate-gated API roots are revisited during finalization.
@@ -355,7 +355,7 @@ export function prepareSpecializedApp(
    });
    const apiFactories = Array.from(
      bootstrapApiFactoryEntries.values(),
-      entry => wrapFeatureFlagApiFactory(entry.factory, features),
+      entry => wrapFeatureFlagApiFactory(entry.factory, features, collector),
    );
    appApiRegistry.registerAll(apiFactories);
  }