Merge pull request #17953 from backstage/rugvip/startup

backend-app-api: parallelize plugin startup, introduce startup hook and middleware

packages/backend-app-api/api-report.md

@@ -18,6 +18,7 @@ import { Handler } from 'express';
 import { HelmetOptions } from 'helmet';
 import * as http from 'http';
 import { HttpRouterService } from '@backstage/backend-plugin-api';
+import { HumanDuration } from '@backstage/types';
 import { IdentityService } from '@backstage/backend-plugin-api';
 import { JsonObject } from '@backstage/types';
 import { LifecycleService } from '@backstage/backend-plugin-api';
@@ -78,6 +79,11 @@ export function createHttpServer(
   },
 ): Promise<ExtendedHttpServer>;
 
+// @public
+export function createLifecycleMiddleware(
+  options: LifecycleMiddlewareOptions,
+): RequestHandler;
+
 // @public (undocumented)
 export function createSpecializedBackend(
   options: CreateSpecializedBackendOptions,
@@ -170,6 +176,13 @@ export const identityServiceFactory: (
   options?: IdentityFactoryOptions | undefined,
 ) => ServiceFactory<IdentityService, 'plugin'>;
 
+// @public
+export interface LifecycleMiddlewareOptions {
+  // (undocumented)
+  lifecycle: LifecycleService;
+  startupRequestPauseTimeout?: HumanDuration;
+}
+
 // @public
 export const lifecycleServiceFactory: () => ServiceFactory<
   LifecycleService,

packages/backend-app-api/src/http/MiddlewareFactory.test.ts

@@ -21,6 +21,7 @@ import {
   NotAllowedError,
   NotFoundError,
   NotModifiedError,
+  ServiceUnavailableError,
 } from '@backstage/errors';
 import express from 'express';
 import createError from 'http-errors';
@@ -143,6 +144,9 @@ describe('MiddlewareFactory', () => {
       app.use('/ConflictError', () => {
         throw new ConflictError();
       });
+      app.use('/ServiceUnavailableError', () => {
+        throw new ServiceUnavailableError();
+      });
       app.use(middleware.error());
 
       const r = request(app);
@@ -165,6 +169,10 @@ describe('MiddlewareFactory', () => {
       expect((await r.get('/ConflictError')).body.error.name).toBe(
         'ConflictError',
       );
+      expect((await r.get('/ServiceUnavailableError')).status).toBe(503);
+      expect((await r.get('/ServiceUnavailableError')).body.error.name).toBe(
+        'ServiceUnavailableError',
+      );
     });
 
     it('logs all 500 errors', async () => {

packages/backend-app-api/src/http/MiddlewareFactory.ts

@@ -36,6 +36,7 @@ import {
   NotAllowedError,
   NotFoundError,
   NotModifiedError,
+  ServiceUnavailableError,
   serializeError,
 } from '@backstage/errors';
 import { NotImplementedError } from '@backstage/errors';
@@ -260,6 +261,8 @@ function getStatusCode(error: Error): number {
       return 409;
     case NotImplementedError.name:
       return 501;
+    case ServiceUnavailableError.name:
+      return 503;
     default:
       break;
   }

packages/backend-app-api/src/services/implementations/httpRouter/createLifecycleMiddleware.test.ts

@@ -0,0 +1,81 @@
+/*
+ * Copyright 2022 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 { createLifecycleMiddleware } from './createLifecycleMiddleware';
+import { BackendLifecycleImpl } from '../rootLifecycle/rootLifecycleServiceFactory';
+import { mockServices } from '@backstage/backend-test-utils';
+import { ServiceUnavailableError } from '@backstage/errors';
+
+describe('createLifecycleMiddleware', () => {
+  it('should pause requests when plugin is not ready', async () => {
+    const lifecycle = new BackendLifecycleImpl(mockServices.rootLogger());
+
+    const middleware = createLifecycleMiddleware({ lifecycle });
+
+    const next = jest.fn();
+    middleware({} as any, {} as any, next);
+    expect(next).not.toHaveBeenCalled();
+    await lifecycle.startup();
+
+    // pending call
+    expect(next).toHaveBeenCalledWith();
+
+    // new call
+    const next2 = jest.fn();
+    middleware({} as any, {} as any, next2);
+    expect(next2).toHaveBeenCalledWith();
+  });
+
+  it('should throw ServiceUnavailableError after shutdown', async () => {
+    const lifecycle = new BackendLifecycleImpl(mockServices.rootLogger());
+    const middleware = createLifecycleMiddleware({ lifecycle });
+
+    const next = jest.fn();
+    middleware({} as any, {} as any, next);
+    expect(next).not.toHaveBeenCalled();
+    await lifecycle.shutdown();
+
+    // pending call
+    expect(next).toHaveBeenCalledWith(
+      new ServiceUnavailableError('Service is shutting down'),
+    );
+
+    // new call
+    const next2 = jest.fn();
+    middleware({} as any, {} as any, next2);
+    expect(next2).toHaveBeenCalledWith(
+      new ServiceUnavailableError('Service is shutting down'),
+    );
+  });
+
+  it('should throw ServiceUnavailableError after timeout', async () => {
+    const lifecycle = new BackendLifecycleImpl(mockServices.rootLogger());
+    const middleware = createLifecycleMiddleware({
+      lifecycle,
+      startupRequestPauseTimeout: { milliseconds: 1 },
+    });
+
+    const next = jest.fn();
+    middleware({} as any, {} as any, next);
+    expect(next).not.toHaveBeenCalled();
+
+    await new Promise(r => setTimeout(r, 2));
+
+    expect(next).toHaveBeenCalledWith(
+      new ServiceUnavailableError('Service has not started up yet'),
+    );
+  });
+});

packages/backend-app-api/src/services/implementations/httpRouter/createLifecycleMiddleware.ts

@@ -0,0 +1,127 @@
+/*
+ * Copyright 2022 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 { LifecycleService } from '@backstage/backend-plugin-api';
+import { ServiceUnavailableError } from '@backstage/errors';
+import { HumanDuration } from '@backstage/types';
+import { RequestHandler } from 'express';
+
+export const DEFAULT_TIMEOUT = { seconds: 5 };
+
+function durationToMs(duration: HumanDuration): number {
+  const {
+    years = 0,
+    months = 0,
+    weeks = 0,
+    days = 0,
+    hours = 0,
+    minutes = 0,
+    seconds = 0,
+    milliseconds = 0,
+  } = duration;
+
+  const totalDays = years * 365 + months * 30 + weeks * 7 + days;
+  const totalHours = totalDays * 24 + hours;
+  const totalMinutes = totalHours * 60 + minutes;
+  const totalSeconds = totalMinutes * 60 + seconds;
+  const totalMilliseconds = totalSeconds * 1000 + milliseconds;
+
+  return totalMilliseconds;
+}
+
+/**
+ * Options for {@link createLifecycleMiddleware}.
+ * @public
+ */
+export interface LifecycleMiddlewareOptions {
+  lifecycle: LifecycleService;
+  /**
+   * The maximum time that paused requests will wait for the service to start, before returning an error.
+   *
+   * Defaults to 5 seconds.
+   */
+  startupRequestPauseTimeout?: HumanDuration;
+}
+
+/**
+ * Creates a middleware that pauses requests until the service has started.
+ *
+ * @remarks
+ *
+ * Requests that arrive before the service has started will be paused until startup is complete.
+ * If the service does not start within the provided timeout, the request will be rejected with a
+ * {@link @backstage/errors#ServiceUnavailableError}.
+ *
+ * If the service is shutting down, all requests will be rejected with a
+ * {@link @backstage/errors#ServiceUnavailableError}.
+ *
+ * @public
+ */
+export function createLifecycleMiddleware(
+  options: LifecycleMiddlewareOptions,
+): RequestHandler {
+  const { lifecycle, startupRequestPauseTimeout = DEFAULT_TIMEOUT } = options;
+
+  let state: 'init' | 'up' | 'down' = 'init';
+  const waiting = new Set<{
+    next: (err?: Error) => void;
+    timeout: NodeJS.Timeout;
+  }>();
+
+  lifecycle.addStartupHook(async () => {
+    if (state === 'init') {
+      state = 'up';
+      for (const item of waiting) {
+        clearTimeout(item.timeout);
+        item.next();
+      }
+      waiting.clear();
+    }
+  });
+
+  lifecycle.addShutdownHook(async () => {
+    state = 'down';
+
+    for (const item of waiting) {
+      clearTimeout(item.timeout);
+      item.next(new ServiceUnavailableError('Service is shutting down'));
+    }
+    waiting.clear();
+  });
+
+  const timeoutMs = durationToMs(startupRequestPauseTimeout);
+
+  return (_req, _res, next) => {
+    if (state === 'up') {
+      next();
+      return;
+    } else if (state === 'down') {
+      next(new ServiceUnavailableError('Service is shutting down'));
+      return;
+    }
+
+    const item = {
+      next,
+      timeout: setTimeout(() => {
+        if (waiting.delete(item)) {
+          next(new ServiceUnavailableError('Service has not started up yet'));
+        }
+      }, timeoutMs),
+    };
+
+    waiting.add(item);
+  };
+}

packages/backend-app-api/src/services/implementations/httpRouter/httpRouterServiceFactory.test.ts

@@ -26,24 +26,32 @@ describe('httpRouterFactory', () => {
       {
         rootHttpRouter,
         plugin: { getId: () => 'test1' },
+        lifecycle: { addStartupHook() {}, addShutdownHook() {} },
       },
       undefined,
     );
     router1.use(handler1);
     expect(rootHttpRouter.use).toHaveBeenCalledTimes(1);
-    expect(rootHttpRouter.use).toHaveBeenCalledWith('/api/test1', handler1);
+    expect(rootHttpRouter.use).toHaveBeenCalledWith(
+      '/api/test1',
+      expect.any(Function),
+    );
 
     const handler2 = () => {};
     const router2 = await factory.factory(
       {
         rootHttpRouter,
         plugin: { getId: () => 'test2' },
+        lifecycle: { addStartupHook() {}, addShutdownHook() {} },
       },
       undefined,
     );
     router2.use(handler2);
     expect(rootHttpRouter.use).toHaveBeenCalledTimes(2);
-    expect(rootHttpRouter.use).toHaveBeenCalledWith('/api/test2', handler2);
+    expect(rootHttpRouter.use).toHaveBeenCalledWith(
+      '/api/test2',
+      expect.any(Function),
+    );
   });
 
   it('should use custom path generator', async () => {
@@ -57,6 +65,7 @@ describe('httpRouterFactory', () => {
       {
         rootHttpRouter,
         plugin: { getId: () => 'test1' },
+        lifecycle: { addStartupHook() {}, addShutdownHook() {} },
       },
       undefined,
     );
@@ -64,7 +73,7 @@ describe('httpRouterFactory', () => {
     expect(rootHttpRouter.use).toHaveBeenCalledTimes(1);
     expect(rootHttpRouter.use).toHaveBeenCalledWith(
       '/some/test1/path',
-      handler1,
+      expect.any(Function),
     );
 
     const handler2 = () => {};
@@ -72,6 +81,7 @@ describe('httpRouterFactory', () => {
       {
         rootHttpRouter,
         plugin: { getId: () => 'test2' },
+        lifecycle: { addStartupHook() {}, addShutdownHook() {} },
       },
       undefined,
     );
@@ -79,7 +89,7 @@ describe('httpRouterFactory', () => {
     expect(rootHttpRouter.use).toHaveBeenCalledTimes(2);
     expect(rootHttpRouter.use).toHaveBeenCalledWith(
       '/some/test2/path',
-      handler2,
+      expect.any(Function),
     );
   });
 });

packages/backend-app-api/src/services/implementations/httpRouter/httpRouterServiceFactory.ts

@@ -15,10 +15,12 @@
  */
 
 import {
-  createServiceFactory,
   coreServices,
+  createServiceFactory,
 } from '@backstage/backend-plugin-api';
 import { Handler } from 'express';
+import PromiseRouter from 'express-promise-router';
+import { createLifecycleMiddleware } from './createLifecycleMiddleware';
 
 /**
  * @public
@@ -36,14 +38,21 @@ export const httpRouterServiceFactory = createServiceFactory(
     service: coreServices.httpRouter,
     deps: {
       plugin: coreServices.pluginMetadata,
+      lifecycle: coreServices.lifecycle,
       rootHttpRouter: coreServices.rootHttpRouter,
     },
-    async factory({ plugin, rootHttpRouter }) {
+    async factory({ plugin, rootHttpRouter, lifecycle }) {
       const getPath = options?.getPath ?? (id => `/api/${id}`);
       const path = getPath(plugin.getId());
+
+      const router = PromiseRouter();
+      rootHttpRouter.use(path, router);
+
+      router.use(createLifecycleMiddleware({ lifecycle }));
+
       return {
         use(handler: Handler) {
-          rootHttpRouter.use(path, handler);
+          router.use(handler);
         },
       };
     },

packages/backend-app-api/src/services/implementations/httpRouter/index.ts

@@ -16,3 +16,5 @@
 
 export { httpRouterServiceFactory } from './httpRouterServiceFactory';
 export type { HttpRouterFactoryOptions } from './httpRouterServiceFactory';
+export { createLifecycleMiddleware } from './createLifecycleMiddleware';
+export type { LifecycleMiddlewareOptions } from './createLifecycleMiddleware';

packages/backend-app-api/src/services/implementations/lifecycle/lifecycleServiceFactory.ts

@@ -14,12 +14,75 @@
  * limitations under the License.
  */
 import {
-  createServiceFactory,
-  coreServices,
+  LifecycleService,
   LifecycleServiceShutdownHook,
   LifecycleServiceShutdownOptions,
+  LifecycleServiceStartupHook,
+  LifecycleServiceStartupOptions,
+  LoggerService,
+  PluginMetadataService,
+  RootLifecycleService,
+  coreServices,
+  createServiceFactory,
 } from '@backstage/backend-plugin-api';
 
+/** @internal */
+export class BackendPluginLifecycleImpl implements LifecycleService {
+  constructor(
+    private readonly logger: LoggerService,
+    private readonly rootLifecycle: RootLifecycleService,
+    private readonly pluginMetadata: PluginMetadataService,
+  ) {}
+
+  #hasStarted = false;
+  #startupTasks: Array<{
+    hook: LifecycleServiceStartupHook;
+    options?: LifecycleServiceStartupOptions;
+  }> = [];
+
+  addStartupHook(
+    hook: LifecycleServiceStartupHook,
+    options?: LifecycleServiceStartupOptions,
+  ): void {
+    if (this.#hasStarted) {
+      throw new Error('Attempted to add startup hook after startup');
+    }
+    this.#startupTasks.push({ hook, options });
+  }
+
+  async startup(): Promise<void> {
+    if (this.#hasStarted) {
+      return;
+    }
+    this.#hasStarted = true;
+
+    this.logger.debug(
+      `Running ${this.#startupTasks.length} plugin startup tasks...`,
+    );
+    await Promise.all(
+      this.#startupTasks.map(async ({ hook, options }) => {
+        const logger = options?.logger ?? this.logger;
+        try {
+          await hook();
+          logger.debug(`Plugin startup hook succeeded`);
+        } catch (error) {
+          logger.error(`Plugin startup hook failed, ${error}`);
+        }
+      }),
+    );
+  }
+
+  addShutdownHook(
+    hook: LifecycleServiceShutdownHook,
+    options?: LifecycleServiceShutdownOptions,
+  ): void {
+    const plugin = this.pluginMetadata.getId();
+    this.rootLifecycle.addShutdownHook(hook, {
+      logger: options?.logger?.child({ plugin }) ?? this.logger,
+    });
+  }
+}
+
 /**
  * Allows plugins to register shutdown hooks that are run when the process is about to exit.
  * @public
@@ -32,16 +95,10 @@ export const lifecycleServiceFactory = createServiceFactory({
     pluginMetadata: coreServices.pluginMetadata,
   },
   async factory({ rootLifecycle, logger, pluginMetadata }) {
-    const plugin = pluginMetadata.getId();
-    return {
-      addShutdownHook(
-        hook: LifecycleServiceShutdownHook,
-        options?: LifecycleServiceShutdownOptions,
-      ): void {
-        rootLifecycle.addShutdownHook(hook, {
-          logger: options?.logger?.child({ plugin }) ?? logger,
-        });
-      },
-    };
+    return new BackendPluginLifecycleImpl(
+      logger,
+      rootLifecycle,
+      pluginMetadata,
+    );
   },
 });

packages/backend-app-api/src/services/implementations/rootLifecycle/rootLifecycleServiceFactory.test.ts

@@ -44,4 +44,17 @@ describe('lifecycleService', () => {
     });
     await expect(service.shutdown()).resolves.toBeUndefined();
   });
+
+  it('should reject hooks after trigger', async () => {
+    const service = new BackendLifecycleImpl(getVoidLogger());
+    await service.startup();
+    expect(() => {
+      service.addStartupHook(() => {});
+    }).toThrow('Attempted to add startup hook after startup');
+
+    await service.shutdown();
+    expect(() => {
+      service.addShutdownHook(() => {});
+    }).toThrow('Attempted to add shutdown hook after shutdown');
+  });
 });

packages/backend-app-api/src/services/implementations/rootLifecycle/rootLifecycleServiceFactory.ts

@@ -17,16 +17,55 @@
 import {
   createServiceFactory,
   coreServices,
+  LifecycleServiceStartupHook,
+  LifecycleServiceStartupOptions,
   LifecycleServiceShutdownHook,
   LifecycleServiceShutdownOptions,
   RootLifecycleService,
   LoggerService,
 } from '@backstage/backend-plugin-api';
 
+/** @internal */
 export class BackendLifecycleImpl implements RootLifecycleService {
   constructor(private readonly logger: LoggerService) {}
 
-  #isCalled = false;
+  #hasStarted = false;
+  #startupTasks: Array<{
+    hook: LifecycleServiceStartupHook;
+    options?: LifecycleServiceStartupOptions;
+  }> = [];
+
+  addStartupHook(
+    hook: LifecycleServiceStartupHook,
+    options?: LifecycleServiceStartupOptions,
+  ): void {
+    if (this.#hasStarted) {
+      throw new Error('Attempted to add startup hook after startup');
+    }
+    this.#startupTasks.push({ hook, options });
+  }
+
+  async startup(): Promise<void> {
+    if (this.#hasStarted) {
+      return;
+    }
+    this.#hasStarted = true;
+
+    this.logger.debug(`Running ${this.#startupTasks.length} startup tasks...`);
+    await Promise.all(
+      this.#startupTasks.map(async ({ hook, options }) => {
+        const logger = options?.logger ?? this.logger;
+        try {
+          await hook();
+          logger.debug(`Startup hook succeeded`);
+        } catch (error) {
+          logger.error(`Startup hook failed, ${error}`);
+        }
+      }),
+    );
+  }
+
+  #hasShutdown = false;
   #shutdownTasks: Array<{
     hook: LifecycleServiceShutdownHook;
     options?: LifecycleServiceShutdownOptions;
@@ -36,22 +75,27 @@ export class BackendLifecycleImpl implements RootLifecycleService {
     hook: LifecycleServiceShutdownHook,
     options?: LifecycleServiceShutdownOptions,
   ): void {
+    if (this.#hasShutdown) {
+      throw new Error('Attempted to add shutdown hook after shutdown');
+    }
     this.#shutdownTasks.push({ hook, options });
   }
 
   async shutdown(): Promise<void> {
-    if (this.#isCalled) {
+    if (this.#hasShutdown) {
       return;
     }
-    this.#isCalled = true;
+    this.#hasShutdown = true;
 
-    this.logger.info(`Running ${this.#shutdownTasks.length} shutdown tasks...`);
+    this.logger.debug(
+      `Running ${this.#shutdownTasks.length} shutdown tasks...`,
+    );
     await Promise.all(
       this.#shutdownTasks.map(async ({ hook, options }) => {
         const logger = options?.logger ?? this.logger;
         try {
           await hook();
-          logger.info(`Shutdown hook succeeded`);
+          logger.debug(`Shutdown hook succeeded`);
         } catch (error) {
           logger.error(`Shutdown hook failed, ${error}`);
         }

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

@@ -17,9 +17,13 @@
 import {
   createServiceRef,
   createServiceFactory,
+  coreServices,
+  createBackendPlugin,
+  createBackendModule,
 } from '@backstage/backend-plugin-api';
 import { BackendInitializer } from './BackendInitializer';
 import { ServiceRegistry } from './ServiceRegistry';
+import { rootLifecycleServiceFactory } from '../services/implementations';
 
 const rootRef = createServiceRef<{ x: number }>({
   id: '1',
@@ -30,6 +34,16 @@ const pluginRef = createServiceRef<{ x: number }>({
   id: '2',
 });
 
+class MockLogger {
+  debug() {}
+  info() {}
+  warn() {}
+  error() {}
+  child() {
+    return this;
+  }
+}
+
 describe('BackendInitializer', () => {
   it('should initialize root scoped services', async () => {
     const rootFactory = jest.fn();
@@ -46,6 +60,12 @@ describe('BackendInitializer', () => {
         deps: {},
         factory: pluginFactory,
       })(),
+      rootLifecycleServiceFactory(),
+      createServiceFactory({
+        service: coreServices.rootLogger,
+        deps: {},
+        factory: () => new MockLogger(),
+      })(),
     ]);
 
     const init = new BackendInitializer(registry);
@@ -54,4 +74,105 @@ describe('BackendInitializer', () => {
     expect(rootFactory).toHaveBeenCalled();
     expect(pluginFactory).not.toHaveBeenCalled();
   });
+
+  it('should forward errors when plugins fail to start', async () => {
+    const init = new BackendInitializer(new ServiceRegistry([]));
+    init.add(
+      createBackendPlugin({
+        pluginId: 'test',
+        register(reg) {
+          reg.registerInit({
+            deps: {},
+            async init() {
+              throw new Error('NOPE');
+            },
+          });
+        },
+      })(),
+    );
+    await expect(init.start()).rejects.toThrow(
+      "Plugin 'test' startup failed; caused by Error: NOPE",
+    );
+  });
+
+  it('should forward errors when modules fail to start', async () => {
+    const init = new BackendInitializer(new ServiceRegistry([]));
+    init.add(
+      createBackendModule({
+        pluginId: 'test',
+        moduleId: 'mod',
+        register(reg) {
+          reg.registerInit({
+            deps: {},
+            async init() {
+              throw new Error('NOPE');
+            },
+          });
+        },
+      })(),
+    );
+    await expect(init.start()).rejects.toThrow(
+      "Module 'mod' for plugin 'test' startup failed; caused by Error: NOPE",
+    );
+  });
+
+  it('should reject duplicate plugins', async () => {
+    const init = new BackendInitializer(new ServiceRegistry([]));
+    init.add(
+      createBackendPlugin({
+        pluginId: 'test',
+        register(reg) {
+          reg.registerInit({
+            deps: {},
+            async init() {},
+          });
+        },
+      })(),
+    );
+    init.add(
+      createBackendPlugin({
+        pluginId: 'test',
+        register(reg) {
+          reg.registerInit({
+            deps: {},
+            async init() {},
+          });
+        },
+      })(),
+    );
+    await expect(init.start()).rejects.toThrow(
+      "Plugin 'test' is already registered",
+    );
+  });
+
+  it('should reject duplicate modules', async () => {
+    const init = new BackendInitializer(new ServiceRegistry([]));
+    init.add(
+      createBackendModule({
+        pluginId: 'test',
+        moduleId: 'mod',
+        register(reg) {
+          reg.registerInit({
+            deps: {},
+            async init() {},
+          });
+        },
+      })(),
+    );
+    init.add(
+      createBackendModule({
+        pluginId: 'test',
+        moduleId: 'mod',
+        register(reg) {
+          reg.registerInit({
+            deps: {},
+            async init() {},
+          });
+        },
+      })(),
+    );
+    await expect(init.start()).rejects.toThrow(
+      "Module 'mod' for plugin 'test' is already registered",
+    );
+  });
 });

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

@@ -21,19 +21,25 @@ import {
   ServiceRef,
 } from '@backstage/backend-plugin-api';
 import { BackendLifecycleImpl } from '../services/implementations/rootLifecycle/rootLifecycleServiceFactory';
-import {
-  BackendRegisterInit,
-  EnumerableServiceHolder,
-  ServiceOrExtensionPoint,
-} from './types';
+import { BackendPluginLifecycleImpl } from '../services/implementations/lifecycle/lifecycleServiceFactory';
+import { EnumerableServiceHolder, ServiceOrExtensionPoint } from './types';
 // Direct internal import to avoid duplication
 // eslint-disable-next-line @backstage/no-forbidden-package-imports
 import { InternalBackendFeature } from '@backstage/backend-plugin-api/src/wiring/types';
+import { ForwardedError } from '@backstage/errors';
+
+export interface BackendRegisterInit {
+  consumes: Set<ServiceOrExtensionPoint>;
+  provides: Set<ServiceOrExtensionPoint>;
+  init: {
+    deps: { [name: string]: ServiceOrExtensionPoint };
+    func: (deps: { [name: string]: unknown }) => Promise<void>;
+  };
+}
 
 export class BackendInitializer {
   #startPromise?: Promise<void>;
   #features = new Array<InternalBackendFeature>();
-  #registerInits = new Array<BackendRegisterInit>();
   #extensionPoints = new Map<ExtensionPoint<unknown>, unknown>();
   #serviceHolder: EnumerableServiceHolder;
 
@@ -130,7 +136,10 @@ export class BackendInitializer {
       }
     }
 
-    // Initialize all features
+    const pluginInits = new Map<string, BackendRegisterInit>();
+    const moduleInits = new Map<string, Map<string, BackendRegisterInit>>();
+
+    // Enumerate all features
     for (const feature of this.#features) {
       for (const r of feature.getRegistrations()) {
         const provides = new Set<ExtensionPoint<unknown>>();
@@ -147,24 +156,84 @@ export class BackendInitializer {
           }
         }
 
-        this.#registerInits.push({
-          id: r.type === 'plugin' ? r.pluginId : `${r.pluginId}.${r.moduleId}`,
-          provides,
-          consumes: new Set(Object.values(r.init.deps)),
-          init: r.init,
-        });
+        if (r.type === 'plugin') {
+          if (pluginInits.has(r.pluginId)) {
+            throw new Error(`Plugin '${r.pluginId}' is already registered`);
+          }
+          pluginInits.set(r.pluginId, {
+            provides,
+            consumes: new Set(Object.values(r.init.deps)),
+            init: r.init,
+          });
+        } else {
+          let modules = moduleInits.get(r.pluginId);
+          if (!modules) {
+            modules = new Map();
+            moduleInits.set(r.pluginId, modules);
+          }
+          if (modules.has(r.moduleId)) {
+            throw new Error(
+              `Module '${r.moduleId}' for plugin '${r.pluginId}' is already registered`,
+            );
+          }
+          modules.set(r.moduleId, {
+            provides,
+            consumes: new Set(Object.values(r.init.deps)),
+            init: r.init,
+          });
+        }
       }
     }
 
-    const orderedRegisterResults = this.#resolveInitOrder(this.#registerInits);
+    const allPluginIds = [
+      ...new Set([...pluginInits.keys(), ...moduleInits.keys()]),
+    ];
 
-    for (const registerInit of orderedRegisterResults) {
-      const deps = await this.#getInitDeps(
-        registerInit.init.deps,
-        registerInit.id,
-      );
-      await registerInit.init.func(deps);
-    }
+    // All plugins are initialized in parallel
+    await Promise.all(
+      allPluginIds.map(async pluginId => {
+        // Modules are initialized before plugins, so that they can provide extension to the plugin
+        const modules = moduleInits.get(pluginId) ?? [];
+        await Promise.all(
+          Array.from(modules).map(async ([moduleId, moduleInit]) => {
+            const moduleDeps = await this.#getInitDeps(
+              moduleInit.init.deps,
+              pluginId,
+            );
+            await moduleInit.init.func(moduleDeps).catch(error => {
+              throw new ForwardedError(
+                `Module '${moduleId}' for plugin '${pluginId}' startup failed`,
+                error,
+              );
+            });
+          }),
+        );
+
+        // Once all modules have been initialized, we can initialize the plugin itself
+        const pluginInit = pluginInits.get(pluginId);
+        // We allow modules to be installed without the accompanying plugin, so the plugin may not exist
+        if (pluginInit) {
+          const pluginDeps = await this.#getInitDeps(
+            pluginInit.init.deps,
+            pluginId,
+          );
+          await pluginInit.init.func(pluginDeps).catch(error => {
+            throw new ForwardedError(
+              `Plugin '${pluginId}' startup failed`,
+              error,
+            );
+          });
+        }
+
+        // Once the plugin and all modules have been initialized, we can signal that the plugin has stared up successfully
+        const lifecycleService = await this.#getPluginLifecycleImpl(pluginId);
+        await lifecycleService.startup();
+      }),
+    );
+
+    // Once all plugins and modules have been initialized, we can signal that the backend has started up successfully
+    const lifecycleService = await this.#getRootLifecycleImpl();
+    await lifecycleService.startup();
 
     // Once the backend is started, any uncaught errors or unhandled rejections are caught
     // and logged, in order to avoid crashing the entire backend on local failures.
@@ -186,56 +255,38 @@ export class BackendInitializer {
     }
   }
 
-  #resolveInitOrder(registerInits: Array<BackendRegisterInit>) {
-    let registerInitsToOrder = registerInits.slice();
-    const orderedRegisterInits = new Array<BackendRegisterInit>();
-
-    // TODO: Validate duplicates
-
-    while (registerInitsToOrder.length > 0) {
-      const toRemove = new Set<unknown>();
-
-      for (const registerInit of registerInitsToOrder) {
-        const unInitializedDependents = [];
-
-        for (const provided of registerInit.provides) {
-          if (
-            registerInitsToOrder.some(
-              init => init !== registerInit && init.consumes.has(provided),
-            )
-          ) {
-            unInitializedDependents.push(provided);
-          }
-        }
-
-        if (unInitializedDependents.length === 0) {
-          orderedRegisterInits.push(registerInit);
-          toRemove.add(registerInit);
-        }
-      }
-
-      registerInitsToOrder = registerInitsToOrder.filter(r => !toRemove.has(r));
-    }
-
-    return orderedRegisterInits;
-  }
-
   async stop(): Promise<void> {
     if (!this.#startPromise) {
       return;
     }
     await this.#startPromise;
 
+    const lifecycleService = await this.#getRootLifecycleImpl();
+    await lifecycleService.shutdown();
+  }
+
+  // Bit of a hacky way to grab the lifecycle services, potentially find a nicer way to do this
+  async #getRootLifecycleImpl(): Promise<BackendLifecycleImpl> {
     const lifecycleService = await this.#serviceHolder.get(
       coreServices.rootLifecycle,
       'root',
     );
-
-    // TODO(Rugvip): Find a better way to do this
     if (lifecycleService instanceof BackendLifecycleImpl) {
-      await lifecycleService.shutdown();
-    } else {
-      throw new Error('Unexpected lifecycle service implementation');
+      return lifecycleService;
     }
+    throw new Error('Unexpected root lifecycle service implementation');
+  }
+
+  async #getPluginLifecycleImpl(
+    pluginId: string,
+  ): Promise<BackendPluginLifecycleImpl> {
+    const lifecycleService = await this.#serviceHolder.get(
+      coreServices.lifecycle,
+      pluginId,
+    );
+    if (lifecycleService instanceof BackendPluginLifecycleImpl) {
+      return lifecycleService;
+    }
+    throw new Error('Unexpected plugin lifecycle service implementation');
   }
 }

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

@@ -32,12 +32,18 @@ describe('createSpecializedBackend', () => {
           createServiceFactory({
             service: coreServices.rootLifecycle,
             deps: {},
-            factory: async () => ({ addShutdownHook: () => {} }),
+            factory: async () => ({
+              addStartupHook: () => {},
+              addShutdownHook: () => {},
+            }),
           }),
           createServiceFactory({
             service: coreServices.rootLifecycle,
             deps: {},
-            factory: async () => ({ addShutdownHook: () => {} }),
+            factory: async () => ({
+              addStartupHook: () => {},
+              addShutdownHook: () => {},
+            }),
           }),
         ],
       }),

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

@@ -30,16 +30,6 @@ export interface Backend {
   stop(): Promise<void>;
 }
 
-export interface BackendRegisterInit {
-  id: string;
-  consumes: Set<ServiceOrExtensionPoint>;
-  provides: Set<ServiceOrExtensionPoint>;
-  init: {
-    deps: { [name: string]: ServiceOrExtensionPoint };
-    func: (deps: { [name: string]: unknown }) => Promise<void>;
-  };
-}
-
 /**
  * @public
  */