diff --git a/.changeset/ninety-jeans-scream.md b/.changeset/ninety-jeans-scream.md new file mode 100644 index 0000000000..3465a67444 --- /dev/null +++ b/.changeset/ninety-jeans-scream.md @@ -0,0 +1,5 @@ +--- +'@backstage/errors': minor +--- + +Added `ServiceUnavailableError` diff --git a/.changeset/ninety-turkeys-knock.md b/.changeset/ninety-turkeys-knock.md new file mode 100644 index 0000000000..7f26f0eeda --- /dev/null +++ b/.changeset/ninety-turkeys-knock.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-app-api': patch +--- + +Switched startup strategy to initialize all plugins in parallel, as well as hook into the new startup lifecycle hooks. diff --git a/.changeset/olive-carpets-explode.md b/.changeset/olive-carpets-explode.md new file mode 100644 index 0000000000..6741355d58 --- /dev/null +++ b/.changeset/olive-carpets-explode.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-plugin-api': patch +--- + +Added startup hooks to the lifecycle services. diff --git a/.changeset/selfish-olives-end.md b/.changeset/selfish-olives-end.md new file mode 100644 index 0000000000..cb3778f316 --- /dev/null +++ b/.changeset/selfish-olives-end.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-app-api': patch +--- + +Introduced built-in middleware into the default `HttpService` implementation that throws a `ServiceNotAvailable` error when plugins aren't able to serve request. Also introduced a request stalling mechanism that pauses incoming request until plugins have been fully initialized. diff --git a/.changeset/tasty-oranges-sleep.md b/.changeset/tasty-oranges-sleep.md new file mode 100644 index 0000000000..7c99ecc822 --- /dev/null +++ b/.changeset/tasty-oranges-sleep.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-app-api': patch +--- + +Added handling of `ServiceUnavailableError` to error handling middleware. diff --git a/packages/backend-app-api/api-report.md b/packages/backend-app-api/api-report.md index 27b218ecb3..5f452f9e03 100644 --- a/packages/backend-app-api/api-report.md +++ b/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; +// @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; +// @public +export interface LifecycleMiddlewareOptions { + // (undocumented) + lifecycle: LifecycleService; + startupRequestPauseTimeout?: HumanDuration; +} + // @public export const lifecycleServiceFactory: () => ServiceFactory< LifecycleService, diff --git a/packages/backend-app-api/src/http/MiddlewareFactory.test.ts b/packages/backend-app-api/src/http/MiddlewareFactory.test.ts index 64812564ca..5dbb63ec59 100644 --- a/packages/backend-app-api/src/http/MiddlewareFactory.test.ts +++ b/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 () => { diff --git a/packages/backend-app-api/src/http/MiddlewareFactory.ts b/packages/backend-app-api/src/http/MiddlewareFactory.ts index 833bdba540..774e784d1d 100644 --- a/packages/backend-app-api/src/http/MiddlewareFactory.ts +++ b/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; } diff --git a/packages/backend-app-api/src/services/implementations/httpRouter/createLifecycleMiddleware.test.ts b/packages/backend-app-api/src/services/implementations/httpRouter/createLifecycleMiddleware.test.ts new file mode 100644 index 0000000000..31eecf14bb --- /dev/null +++ b/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'), + ); + }); +}); diff --git a/packages/backend-app-api/src/services/implementations/httpRouter/createLifecycleMiddleware.ts b/packages/backend-app-api/src/services/implementations/httpRouter/createLifecycleMiddleware.ts new file mode 100644 index 0000000000..958ae33e99 --- /dev/null +++ b/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); + }; +} diff --git a/packages/backend-app-api/src/services/implementations/httpRouter/httpRouterServiceFactory.test.ts b/packages/backend-app-api/src/services/implementations/httpRouter/httpRouterServiceFactory.test.ts index e52b4c9a36..d0b40df33b 100644 --- a/packages/backend-app-api/src/services/implementations/httpRouter/httpRouterServiceFactory.test.ts +++ b/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), ); }); }); diff --git a/packages/backend-app-api/src/services/implementations/httpRouter/httpRouterServiceFactory.ts b/packages/backend-app-api/src/services/implementations/httpRouter/httpRouterServiceFactory.ts index bbd54b7ade..2ecdeffd77 100644 --- a/packages/backend-app-api/src/services/implementations/httpRouter/httpRouterServiceFactory.ts +++ b/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); }, }; }, diff --git a/packages/backend-app-api/src/services/implementations/httpRouter/index.ts b/packages/backend-app-api/src/services/implementations/httpRouter/index.ts index 9dfb01c5be..928a232131 100644 --- a/packages/backend-app-api/src/services/implementations/httpRouter/index.ts +++ b/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'; diff --git a/packages/backend-app-api/src/services/implementations/lifecycle/lifecycleServiceFactory.ts b/packages/backend-app-api/src/services/implementations/lifecycle/lifecycleServiceFactory.ts index 79658c0d42..2b68a81807 100644 --- a/packages/backend-app-api/src/services/implementations/lifecycle/lifecycleServiceFactory.ts +++ b/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 { + 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, + ); }, }); diff --git a/packages/backend-app-api/src/services/implementations/rootLifecycle/rootLifecycleServiceFactory.test.ts b/packages/backend-app-api/src/services/implementations/rootLifecycle/rootLifecycleServiceFactory.test.ts index 8d2b2af3d5..91cb9031ae 100644 --- a/packages/backend-app-api/src/services/implementations/rootLifecycle/rootLifecycleServiceFactory.test.ts +++ b/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'); + }); }); diff --git a/packages/backend-app-api/src/services/implementations/rootLifecycle/rootLifecycleServiceFactory.ts b/packages/backend-app-api/src/services/implementations/rootLifecycle/rootLifecycleServiceFactory.ts index f379c2ec52..197f99b97a 100644 --- a/packages/backend-app-api/src/services/implementations/rootLifecycle/rootLifecycleServiceFactory.ts +++ b/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 { + 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 { - 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}`); } diff --git a/packages/backend-app-api/src/wiring/BackendInitializer.test.ts b/packages/backend-app-api/src/wiring/BackendInitializer.test.ts index c00969a52c..b05194f594 100644 --- a/packages/backend-app-api/src/wiring/BackendInitializer.test.ts +++ b/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", + ); + }); }); diff --git a/packages/backend-app-api/src/wiring/BackendInitializer.ts b/packages/backend-app-api/src/wiring/BackendInitializer.ts index b2c62c2122..8c2b75e887 100644 --- a/packages/backend-app-api/src/wiring/BackendInitializer.ts +++ b/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; + provides: Set; + init: { + deps: { [name: string]: ServiceOrExtensionPoint }; + func: (deps: { [name: string]: unknown }) => Promise; + }; +} export class BackendInitializer { #startPromise?: Promise; #features = new Array(); - #registerInits = new Array(); #extensionPoints = new Map, unknown>(); #serviceHolder: EnumerableServiceHolder; @@ -130,7 +136,10 @@ export class BackendInitializer { } } - // Initialize all features + const pluginInits = new Map(); + const moduleInits = new Map>(); + + // Enumerate all features for (const feature of this.#features) { for (const r of feature.getRegistrations()) { const provides = new Set>(); @@ -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) { - let registerInitsToOrder = registerInits.slice(); - const orderedRegisterInits = new Array(); - - // TODO: Validate duplicates - - while (registerInitsToOrder.length > 0) { - const toRemove = new Set(); - - 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 { 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 { 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 { + const lifecycleService = await this.#serviceHolder.get( + coreServices.lifecycle, + pluginId, + ); + if (lifecycleService instanceof BackendPluginLifecycleImpl) { + return lifecycleService; + } + throw new Error('Unexpected plugin lifecycle service implementation'); } } diff --git a/packages/backend-app-api/src/wiring/createSpecializedBackend.test.ts b/packages/backend-app-api/src/wiring/createSpecializedBackend.test.ts index ad50455e5f..dc9a0cc085 100644 --- a/packages/backend-app-api/src/wiring/createSpecializedBackend.test.ts +++ b/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: () => {}, + }), }), ], }), diff --git a/packages/backend-app-api/src/wiring/types.ts b/packages/backend-app-api/src/wiring/types.ts index 2a91bf46b9..ffedb40324 100644 --- a/packages/backend-app-api/src/wiring/types.ts +++ b/packages/backend-app-api/src/wiring/types.ts @@ -30,16 +30,6 @@ export interface Backend { stop(): Promise; } -export interface BackendRegisterInit { - id: string; - consumes: Set; - provides: Set; - init: { - deps: { [name: string]: ServiceOrExtensionPoint }; - func: (deps: { [name: string]: unknown }) => Promise; - }; -} - /** * @public */ diff --git a/packages/backend-defaults/src/CreateBackend.test.ts b/packages/backend-defaults/src/CreateBackend.test.ts index 8d56cd0986..946de90b2b 100644 --- a/packages/backend-defaults/src/CreateBackend.test.ts +++ b/packages/backend-defaults/src/CreateBackend.test.ts @@ -35,7 +35,10 @@ describe('createBackend', () => { createServiceFactory({ service: coreServices.rootLifecycle, deps: {}, - factory: async () => ({ addShutdownHook: () => {} }), + factory: async () => ({ + addStartupHook: () => {}, + addShutdownHook: () => {}, + }), }), ], }), @@ -49,12 +52,18 @@ describe('createBackend', () => { createServiceFactory({ service: coreServices.rootLifecycle, deps: {}, - factory: async () => ({ addShutdownHook: () => {} }), + factory: async () => ({ + addStartupHook: () => {}, + addShutdownHook: () => {}, + }), }), createServiceFactory({ service: coreServices.rootLifecycle, deps: {}, - factory: async () => ({ addShutdownHook: () => {} }), + factory: async () => ({ + addStartupHook: () => {}, + addShutdownHook: () => {}, + }), }), ], }), diff --git a/packages/backend-plugin-api/api-report.md b/packages/backend-plugin-api/api-report.md index 7dc6ae97b3..b3cbef993c 100644 --- a/packages/backend-plugin-api/api-report.md +++ b/packages/backend-plugin-api/api-report.md @@ -273,6 +273,10 @@ export interface LifecycleService { hook: LifecycleServiceShutdownHook, options?: LifecycleServiceShutdownOptions, ): void; + addStartupHook( + hook: LifecycleServiceStartupHook, + options?: LifecycleServiceStartupOptions, + ): void; } // @public (undocumented) @@ -283,6 +287,14 @@ export interface LifecycleServiceShutdownOptions { logger?: LoggerService; } +// @public (undocumented) +export type LifecycleServiceStartupHook = () => void | Promise; + +// @public (undocumented) +export interface LifecycleServiceStartupOptions { + logger?: LoggerService; +} + // @public export interface LoggerService { // (undocumented) diff --git a/packages/backend-plugin-api/src/services/definitions/LifecycleService.ts b/packages/backend-plugin-api/src/services/definitions/LifecycleService.ts index 89351194e6..e0641e0457 100644 --- a/packages/backend-plugin-api/src/services/definitions/LifecycleService.ts +++ b/packages/backend-plugin-api/src/services/definitions/LifecycleService.ts @@ -16,6 +16,21 @@ import { LoggerService } from './LoggerService'; +/** + * @public + */ +export type LifecycleServiceStartupHook = () => void | Promise; + +/** + * @public + */ +export interface LifecycleServiceStartupOptions { + /** + * Optional {@link LoggerService} that will be used for logging instead of the default logger. + */ + logger?: LoggerService; +} + /** * @public */ @@ -35,6 +50,20 @@ export interface LifecycleServiceShutdownOptions { * @public */ export interface LifecycleService { + /** + * Register a function to be called when the backend has been initialized. + * + * @remarks + * + * When used with plugin scope it will wait for the plugin itself to have been initialized. + * + * When used with root scope it will wait for all plugins to have been initialized. + */ + addStartupHook( + hook: LifecycleServiceStartupHook, + options?: LifecycleServiceStartupOptions, + ): void; + /** * Register a function to be called when the backend is shutting down. */ diff --git a/packages/backend-plugin-api/src/services/definitions/index.ts b/packages/backend-plugin-api/src/services/definitions/index.ts index 0122bc0be8..ab0f0b84b1 100644 --- a/packages/backend-plugin-api/src/services/definitions/index.ts +++ b/packages/backend-plugin-api/src/services/definitions/index.ts @@ -26,6 +26,8 @@ export type { DiscoveryService } from './DiscoveryService'; export type { HttpRouterService } from './HttpRouterService'; export type { LifecycleService, + LifecycleServiceStartupHook, + LifecycleServiceStartupOptions, LifecycleServiceShutdownHook, LifecycleServiceShutdownOptions, } from './LifecycleService'; diff --git a/packages/errors/api-report.md b/packages/errors/api-report.md index 650a826a30..0d405d8ba6 100644 --- a/packages/errors/api-report.md +++ b/packages/errors/api-report.md @@ -125,6 +125,9 @@ export function serializeError( }, ): SerializedError; +// @public +export class ServiceUnavailableError extends CustomErrorBase {} + // @public export function stringifyError(error: unknown): string; ``` diff --git a/packages/errors/src/errors/common.ts b/packages/errors/src/errors/common.ts index e1a292d7b5..a2ea20e99f 100644 --- a/packages/errors/src/errors/common.ts +++ b/packages/errors/src/errors/common.ts @@ -81,6 +81,13 @@ export class NotModifiedError extends CustomErrorBase {} */ export class NotImplementedError extends CustomErrorBase {} +/** + * The server is not ready to handle the request. + * + * @public + */ +export class ServiceUnavailableError extends CustomErrorBase {} + /** * An error that forwards an underlying cause with additional context in the message. * diff --git a/packages/errors/src/errors/index.ts b/packages/errors/src/errors/index.ts index efd3257fbf..0a59891f87 100644 --- a/packages/errors/src/errors/index.ts +++ b/packages/errors/src/errors/index.ts @@ -25,6 +25,7 @@ export { NotFoundError, NotModifiedError, NotImplementedError, + ServiceUnavailableError, } from './common'; export { CustomErrorBase } from './CustomErrorBase'; export { ResponseError } from './ResponseError';