refator: move lifecycle middleware to root http

Signed-off-by: Camila Belo <camilaibs@gmail.com>
2024-12-04 17:57:45 +01:00
parent 7ad9d1a0ee
commit ad39962dc9
11 changed files with 365 additions and 17 deletions
@@ -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.
@@ -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
@@ -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",
@@ -134,6 +134,8 @@ export interface RootHttpRouterConfigureContext {
  // (undocumented)
  lifecycle: LifecycleService;
  // (undocumented)
+  lifecycleMiddleware: RequestHandler;
+  // (undocumented)
  logger: LoggerService;
  // (undocumented)
  middleware: MiddlewareFactory;
@@ -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 }));

@@ -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();
+  });
+});
@@ -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);
+  };
+}
@@ -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<ExtendedHttpServer> {
  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<void>((resolve, reject) => {
@@ -61,7 +53,7 @@ export async function createHttpServer(

    stop() {
      return new Promise<void>((resolve, reject) => {
-        stopServer((error?: Error) => {
+        server.close(error => {
          if (error) {
            reject(error);
          } else {
@@ -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();
+  });
 });
@@ -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();

@@ -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