diff --git a/docs/backend-system/core-services/root-http-router.md b/docs/backend-system/core-services/root-http-router.md index a2316b957b..530ea09544 100644 --- a/docs/backend-system/core-services/root-http-router.md +++ b/docs/backend-system/core-services/root-http-router.md @@ -42,6 +42,21 @@ createBackendPlugin({ ## Configuring the service +### Via `app-config.yaml` + +The `app-config.yaml` file provides configurable options that can be adjusted to meet your `RootHttpRouterService` specific requirements: + +```yaml +backend: + lifecycle: + # (Optional) The maximum time that paused requests will wait for the service to start, before returning an error (defaults to 5 seconds). + startupRequestPauseTimeout: { seconds: 10 } + # (Optional) The maximum time that the server will wait for stop accepting traffic, before returning an error (defaults to 30 seconds). + shutdownRequestPauseTimeout: { seconds: 20 } +``` + +### Via Code + There's additional options that you can pass to configure the root HTTP Router service. These options are passed when you call `createBackend`. - `indexPath` - optional path to forward all unmatched requests to. Defaults to `/api/app` which is the `app-backend` plugin responsible for serving the frontend application through the backend. diff --git a/packages/backend-defaults/config.d.ts b/packages/backend-defaults/config.d.ts index 0f7f9906f8..82fb19c14f 100644 --- a/packages/backend-defaults/config.d.ts +++ b/packages/backend-defaults/config.d.ts @@ -28,6 +28,19 @@ export interface Config { */ baseUrl: string; + lifecycle?: { + /** + * The maximum time that paused requests will wait for the service to start, before returning an error. + * Defaults to 5 seconds. + */ + startupRequestPauseTimeout?: HumanDuration; + /** + * The maximum time that the server will wait for stop accepting traffic, before returning an error. + * Defaults to 30 seconds. + */ + shutdownRequestPauseTimeout?: HumanDuration; + }; + /** Address that the backend should listen to. */ listen?: | string diff --git a/packages/backend-defaults/package.json b/packages/backend-defaults/package.json index d8240c429c..88d3ca79b1 100644 --- a/packages/backend-defaults/package.json +++ b/packages/backend-defaults/package.json @@ -175,7 +175,6 @@ "pg-format": "^1.0.4", "raw-body": "^2.4.1", "selfsigned": "^2.0.0", - "stoppable": "^1.1.0", "tar": "^6.1.12", "triple-beam": "^1.4.1", "uuid": "^11.0.0", diff --git a/packages/backend-defaults/report-rootHttpRouter.api.md b/packages/backend-defaults/report-rootHttpRouter.api.md index 41830ebf17..4489fed107 100644 --- a/packages/backend-defaults/report-rootHttpRouter.api.md +++ b/packages/backend-defaults/report-rootHttpRouter.api.md @@ -134,6 +134,8 @@ export interface RootHttpRouterConfigureContext { // (undocumented) lifecycle: LifecycleService; // (undocumented) + lifecycleMiddleware: RequestHandler; + // (undocumented) logger: LoggerService; // (undocumented) middleware: MiddlewareFactory; diff --git a/packages/backend-defaults/src/entrypoints/httpRouter/httpRouterServiceFactory.ts b/packages/backend-defaults/src/entrypoints/httpRouter/httpRouterServiceFactory.ts index 0557cb636d..a448827e16 100644 --- a/packages/backend-defaults/src/entrypoints/httpRouter/httpRouterServiceFactory.ts +++ b/packages/backend-defaults/src/entrypoints/httpRouter/httpRouterServiceFactory.ts @@ -22,7 +22,6 @@ import { HttpRouterServiceAuthPolicy, } from '@backstage/backend-plugin-api'; import { - createLifecycleMiddleware, createCookieAuthRefreshMiddleware, createCredentialsBarrier, createAuthIntegrationRouter, @@ -43,12 +42,11 @@ export const httpRouterServiceFactory = createServiceFactory({ deps: { plugin: coreServices.pluginMetadata, config: coreServices.rootConfig, - lifecycle: coreServices.lifecycle, rootHttpRouter: coreServices.rootHttpRouter, auth: coreServices.auth, httpAuth: coreServices.httpAuth, }, - async factory({ auth, httpAuth, config, plugin, rootHttpRouter, lifecycle }) { + async factory({ auth, httpAuth, config, plugin, rootHttpRouter }) { const router = PromiseRouter(); rootHttpRouter.use(`/api/${plugin.getId()}`, router); @@ -59,7 +57,6 @@ export const httpRouterServiceFactory = createServiceFactory({ }); router.use(createAuthIntegrationRouter({ auth })); - router.use(createLifecycleMiddleware({ lifecycle })); router.use(credentialsBarrier.middleware); router.use(createCookieAuthRefreshMiddleware({ auth, httpAuth })); diff --git a/packages/backend-defaults/src/entrypoints/rootHttpRouter/createLifecycleMiddleware.test.ts b/packages/backend-defaults/src/entrypoints/rootHttpRouter/createLifecycleMiddleware.test.ts new file mode 100644 index 0000000000..0ac8f704bf --- /dev/null +++ b/packages/backend-defaults/src/entrypoints/rootHttpRouter/createLifecycleMiddleware.test.ts @@ -0,0 +1,108 @@ +/* + * 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'), + ); + }); + + it('should delay service shutdown for the default timeout duration', async () => { + jest.useFakeTimers(); + const defaultTimeout = 30000; + const lifecycle = new BackendLifecycleImpl(mockServices.rootLogger()); + createLifecycleMiddleware({ lifecycle }); + const beforeShutdownPromise = lifecycle.beforeShutdown().then(() => { + jest.useRealTimers(); + }); + jest.advanceTimersByTime(defaultTimeout); + return expect(beforeShutdownPromise).resolves.toBeUndefined(); + }); + + it('should delay service shutdown for the configured timeout duration', async () => { + jest.useFakeTimers(); + const configuredTimeout = 20000; + const lifecycle = new BackendLifecycleImpl(mockServices.rootLogger()); + createLifecycleMiddleware({ + lifecycle, + shutdownRequestPauseTimeout: { milliseconds: configuredTimeout }, + }); + const beforeShutdownPromise = lifecycle.beforeShutdown().then(() => { + jest.useRealTimers(); + }); + jest.advanceTimersByTime(configuredTimeout); + return expect(beforeShutdownPromise).resolves.toBeUndefined(); + }); +}); diff --git a/packages/backend-defaults/src/entrypoints/rootHttpRouter/createLifecycleMiddleware.ts b/packages/backend-defaults/src/entrypoints/rootHttpRouter/createLifecycleMiddleware.ts new file mode 100644 index 0000000000..615da13d83 --- /dev/null +++ b/packages/backend-defaults/src/entrypoints/rootHttpRouter/createLifecycleMiddleware.ts @@ -0,0 +1,124 @@ +/* + * 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 { RootLifecycleService } from '@backstage/backend-plugin-api'; +import { ServiceUnavailableError } from '@backstage/errors'; +import { HumanDuration, durationToMilliseconds } from '@backstage/types'; +import { RequestHandler } from 'express'; + +export const DEFAULT_STARTUP_REQUEST_PAUSE_TIMEOUT = { seconds: 5 }; +export const DEFAULT_SHUTDOWN_REQUEST_PAUSE_TIMEOUT = { seconds: 30 }; + +/** + * Options for {@link createLifecycleMiddleware}. + * @public + */ +export interface LifecycleMiddlewareOptions { + lifecycle: RootLifecycleService; + /** + * The maximum time that paused requests will wait for the service to start, before returning an error. + * + * Defaults to 5 seconds. + */ + startupRequestPauseTimeout?: HumanDuration; + /** + * The maximum time that the server will wait for stop accepting traffic, before returning an error. + * + * Defaults to 30 seconds. + */ + shutdownRequestPauseTimeout?: 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, shutdownRequestPauseTimeout } = + 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.addBeforeShutdownHook(async () => { + const timeoutMs = durationToMilliseconds( + shutdownRequestPauseTimeout ?? DEFAULT_SHUTDOWN_REQUEST_PAUSE_TIMEOUT, + ); + return await new Promise(resolve => { + setTimeout(resolve, timeoutMs); + }); + }); + + lifecycle.addShutdownHook(async () => { + state = 'down'; + for (const item of waiting) { + clearTimeout(item.timeout); + item.next(new ServiceUnavailableError('Service is shutting down')); + } + waiting.clear(); + }); + + return (_req, _res, next) => { + if (state === 'up') { + next(); + return; + } else if (state === 'down') { + next(new ServiceUnavailableError('Service is shutting down')); + return; + } + + const timeoutMs = durationToMilliseconds( + startupRequestPauseTimeout ?? DEFAULT_STARTUP_REQUEST_PAUSE_TIMEOUT, + ); + + 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-defaults/src/entrypoints/rootHttpRouter/http/createHttpServer.ts b/packages/backend-defaults/src/entrypoints/rootHttpRouter/http/createHttpServer.ts index 5c526ba1e2..9449291628 100644 --- a/packages/backend-defaults/src/entrypoints/rootHttpRouter/http/createHttpServer.ts +++ b/packages/backend-defaults/src/entrypoints/rootHttpRouter/http/createHttpServer.ts @@ -16,7 +16,6 @@ import * as http from 'http'; import * as https from 'https'; -import stoppableServer from 'stoppable'; import { RequestListener } from 'http'; import { LoggerService } from '@backstage/backend-plugin-api'; import { HttpServerOptions, ExtendedHttpServer } from './types'; @@ -33,13 +32,6 @@ export async function createHttpServer( deps: { logger: LoggerService }, ): Promise { const server = await createServer(listener, options, deps); - - const stopper = stoppableServer(server, 0); - // The stopper here is actually the server itself, so if we try - // to call stopper.stop() down in the stop implementation, we'll - // be calling ourselves. - const stopServer = stopper.stop.bind(stopper); - return Object.assign(server, { start() { return new Promise((resolve, reject) => { @@ -61,7 +53,7 @@ export async function createHttpServer( stop() { return new Promise((resolve, reject) => { - stopServer((error?: Error) => { + server.close(error => { if (error) { reject(error); } else { diff --git a/packages/backend-defaults/src/entrypoints/rootHttpRouter/rootHttpRouterServiceFactory.test.ts b/packages/backend-defaults/src/entrypoints/rootHttpRouter/rootHttpRouterServiceFactory.test.ts index f058c87900..f04df36a9b 100644 --- a/packages/backend-defaults/src/entrypoints/rootHttpRouter/rootHttpRouterServiceFactory.test.ts +++ b/packages/backend-defaults/src/entrypoints/rootHttpRouter/rootHttpRouterServiceFactory.test.ts @@ -21,7 +21,12 @@ import { import { Express } from 'express'; import request from 'supertest'; import { rootHttpRouterServiceFactory } from './rootHttpRouterServiceFactory'; -import { ServiceFactory, coreServices } from '@backstage/backend-plugin-api'; +import { + ServiceFactory, + coreServices, + createServiceFactory, +} from '@backstage/backend-plugin-api'; +import { BackendLifecycleImpl } from '../rootLifecycle/rootLifecycleServiceFactory'; async function createExpressApp(...dependencies: ServiceFactory[]) { let app: Express | undefined = undefined; @@ -198,4 +203,71 @@ describe('rootHttpRouterServiceFactory', () => { 'Invalid header value in at backend.health.headers, must be a non-empty string', ); }); + + it('should wait the server shutdown', async () => { + jest.useFakeTimers(); + + let app: Express | undefined = undefined; + const lifecycleMock = new BackendLifecycleImpl(mockServices.rootLogger()); + + const tester = ServiceFactoryTester.from( + rootHttpRouterServiceFactory({ + configure(options) { + console.log('configure'); + options.app.use(options.lifecycleMiddleware); + options.app.get('/test', (_req, res) => { + res.status(200).send({ status: 'ok' }).end(); + }); + app = options.app; + }, + }), + { + dependencies: [ + mockServices.rootConfig.factory({ + data: { + app: { baseUrl: 'http://localhost' }, + backend: { + baseUrl: 'http://localhost', + listen: { host: '', port: 0 }, + }, + }, + }), + createServiceFactory({ + service: coreServices.rootLifecycle, + deps: {}, + factory() { + return lifecycleMock; + }, + }), + ], + }, + ); + + await tester.getSubject(); + + // Trigger creation of the http service, accessing the app instance through the configure callback + const lifecycle = await tester.getService(coreServices.rootLifecycle); + + await (lifecycle as any).startup(); // Trigger startup by calling the private startup method + + await request(app!).get('/test').expect(200, { + status: 'ok', + }); + + const beforeShutdownPromise = (lifecycle as any) + .beforeShutdown() + .then(() => { + return (lifecycle as any).shutdown(); + }); + + await request(app!).get('/test').expect(200, { + status: 'ok', + }); + + jest.advanceTimersByTime(30000); + + await request(app!).get('/test').expect(500, {}); + + return await expect(beforeShutdownPromise).resolves.toBeUndefined(); + }); }); diff --git a/packages/backend-defaults/src/entrypoints/rootHttpRouter/rootHttpRouterServiceFactory.ts b/packages/backend-defaults/src/entrypoints/rootHttpRouter/rootHttpRouterServiceFactory.ts index 755535dc70..fc2d5bb3a3 100644 --- a/packages/backend-defaults/src/entrypoints/rootHttpRouter/rootHttpRouterServiceFactory.ts +++ b/packages/backend-defaults/src/entrypoints/rootHttpRouter/rootHttpRouterServiceFactory.ts @@ -21,6 +21,8 @@ import { LifecycleService, LoggerService, } from '@backstage/backend-plugin-api'; +import { HumanDuration } from '@backstage/types'; +import { readDurationFromConfig } from '@backstage/config'; import express, { RequestHandler, Express } from 'express'; import type { Server } from 'node:http'; import { @@ -30,6 +32,7 @@ import { } from './http'; import { DefaultRootHttpRouter } from './DefaultRootHttpRouter'; import { createHealthRouter } from './createHealthRouter'; +import { createLifecycleMiddleware } from './createLifecycleMiddleware'; /** * @public @@ -43,6 +46,7 @@ export interface RootHttpRouterConfigureContext { logger: LoggerService; lifecycle: LifecycleService; healthRouter: RequestHandler; + lifecycleMiddleware: RequestHandler; applyDefaults: () => void; } @@ -90,6 +94,27 @@ const rootHttpRouterServiceFactoryWithOptions = ( const routes = router.handler(); const healthRouter = createHealthRouter({ config, health }); + + let startupRequestPauseTimeout: HumanDuration | undefined; + if (config.has('backend.lifecycle.startupRequestPauseTimeout')) { + startupRequestPauseTimeout = readDurationFromConfig(config, { + key: 'backend.lifecycle.startupRequestPauseTimeout', + }); + } + + let shutdownRequestPauseTimeout: HumanDuration | undefined; + if (config.has('backend.lifecycle.shutdownRequestPauseTimeout')) { + shutdownRequestPauseTimeout = readDurationFromConfig(config, { + key: 'backend.lifecycle.shutdownRequestPauseTimeout', + }); + } + + const lifecycleMiddleware = createLifecycleMiddleware({ + lifecycle, + startupRequestPauseTimeout, + shutdownRequestPauseTimeout, + }); + const server = await createHttpServer( app, readHttpServerOptions(config.getOptionalConfig('backend')), @@ -105,6 +130,7 @@ const rootHttpRouterServiceFactoryWithOptions = ( logger, lifecycle, healthRouter, + lifecycleMiddleware, applyDefaults() { if (process.env.NODE_ENV === 'development') { app.set('json spaces', 2); @@ -114,13 +140,14 @@ const rootHttpRouterServiceFactoryWithOptions = ( app.use(middleware.compression()); app.use(middleware.logging()); app.use(healthRouter); + app.use(lifecycleMiddleware); app.use(routes); app.use(middleware.notFound()); app.use(middleware.error()); }, }); - lifecycle.addBeforeShutdownHook(() => server.stop()); + lifecycle.addShutdownHook(() => server.stop()); await server.start(); diff --git a/yarn.lock b/yarn.lock index 463f04a644..353786ab44 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3651,7 +3651,6 @@ __metadata: pg-format: ^1.0.4 raw-body: ^2.4.1 selfsigned: ^2.0.0 - stoppable: ^1.1.0 supertest: ^7.0.0 tar: ^6.1.12 triple-beam: ^1.4.1