refactor: apply more review suggestions

Co-authored-by: Fredrik Adelöw <freben@gmail.com>
Co-authored-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Camila Belo <camilaibs@gmail.com>

.changeset/rude-pears-tie.md

@@ -2,4 +2,5 @@
 '@backstage/backend-defaults': patch
 ---
 
-Remove use of the `stoppable` library as Node's native http server [close](https://nodejs.org/api/http.html#serverclosecallback) method already drains requests.
+Remove use of the `stoppable` library on the `DefaultRootHttpRouterService` as Node's native http server [close](https://nodejs.org/api/http.html#serverclosecallback) method already drains requests.
+Also, we pass a new `lifecycleMiddleware` to the `rootHttpRouterServiceFactory` configure function that must be called manually if you don't call `applyDefaults`.

docs/backend-system/core-services/root-http-router.md

@@ -55,12 +55,12 @@ backend:
     # - A standard ISO formatted duration string, e.g. 'P2DT6H' or 'PT1M'.
     # - An object with individual units (in plural) as keys, e.g. `{ days: 2, hours: 6 }`.
     startupRequestPauseTimeout: { seconds: 10 }
-    # (Optional) The maximum time that the server will wait for stop accepting traffic, before returning an error (defaults to 30 seconds).
+    # (Optional) The minimum time that the HTTP server will delay the shutdown of the backend. During this delay health checks will be set to failing, allowing traffic to drain (defaults to 0 seconds).
     # Supported formats:
     # - A string in the format of '1d', '2 seconds' etc. as supported by the `ms` library.
     # - A standard ISO formatted duration string, e.g. 'P2DT6H' or 'PT1M'.
     # - An object with individual units (in plural) as keys, e.g. `{ days: 2, hours: 6 }`.
-    shutdownRequestPauseTimeout: { seconds: 20 }
+    serverShutdownTimeout: { seconds: 20 }
 ```
 
 ### Via Code

packages/backend-defaults/config.d.ts

@@ -40,8 +40,8 @@ export interface Config {
        */
       startupRequestPauseTimeout?: string | HumanDuration;
       /**
-       * The maximum time that the server will wait for stop accepting traffic, before returning an error.
-       * Defaults to 30 seconds.
+       * The minimum time that the HTTP server will delay the shutdown of the backend. During this delay health checks will be set to failing, allowing traffic to drain.
+       * Defaults to 0 seconds.
        * Supported formats:
        * - A string in the format of '1d', '2 seconds' etc. as supported by the `ms`
        *   library.

packages/backend-defaults/src/entrypoints/rootHealth/rootHealthServiceFactory.test.ts

@@ -69,7 +69,7 @@ describe('DefaultRootHealthService', () => {
       await expect(service.getReadiness()).resolves.toEqual({
         status: 503,
         payload: {
-          message: 'Backend has not started yet',
+          message: 'Backend is shuttting down',
           status: 'error',
         },
       });

packages/backend-defaults/src/entrypoints/rootHealth/rootHealthServiceFactory.ts

@@ -23,14 +23,14 @@ import {
 
 /** @internal */
 export class DefaultRootHealthService implements RootHealthService {
-  #isRunning = false;
+  #state: 'init' | 'up' | 'down' = 'init';
 
   constructor(readonly options: { lifecycle: RootLifecycleService }) {
     options.lifecycle.addStartupHook(() => {
-      this.#isRunning = true;
+      this.#state = 'up';
     });
     options.lifecycle.addBeforeShutdownHook(() => {
-      this.#isRunning = false;
+      this.#state = 'down';
     });
   }
 
@@ -39,12 +39,18 @@ export class DefaultRootHealthService implements RootHealthService {
   }
 
   async getReadiness(): Promise<{ status: number; payload?: any }> {
-    if (!this.#isRunning) {
+    if (this.#state === 'init') {
       return {
         status: 503,
         payload: { message: 'Backend has not started yet', status: 'error' },
       };
     }
+    if (this.#state === 'down') {
+      return {
+        status: 503,
+        payload: { message: 'Backend is shuttting down', status: 'error' },
+      };
+    }
 
     return { status: 200, payload: { status: 'ok' } };
   }

packages/backend-defaults/src/entrypoints/rootHttpRouter/createLifecycleMiddleware.ts

@@ -20,7 +20,7 @@ import { HumanDuration, durationToMilliseconds } from '@backstage/types';
 import { RequestHandler } from 'express';
 
 export const DEFAULT_STARTUP_REQUEST_PAUSE_TIMEOUT = { seconds: 5 };
-export const DEFAULT_SERVER_SHUTDOWN_TIMEOUT = { seconds: 30 };
+export const DEFAULT_SERVER_SHUTDOWN_TIMEOUT = { seconds: 0 };
 
 /**
  * Options for {@link createLifecycleMiddleware}.
@@ -37,7 +37,7 @@ export interface LifecycleMiddlewareOptions {
   /**
    * The maximum time that the server will wait for stop accepting traffic, before returning an error.
    *
-   * Defaults to 30 seconds.
+   * Defaults to 0 seconds.
    */
   serverShutdownDelay?: HumanDuration;
 }

packages/backend-defaults/src/entrypoints/rootHttpRouter/rootHttpRouterServiceFactory.test.ts

@@ -231,6 +231,9 @@ describe('rootHttpRouterServiceFactory', () => {
               backend: {
                 baseUrl: 'http://localhost',
                 listen: { host: '', port: 0 },
+                lifecycle: {
+                  serverShutdownDelay: '30s',
+                },
               },
             },
           }),
@@ -281,7 +284,7 @@ describe('rootHttpRouterServiceFactory', () => {
 
     // Immediately start failing the readiness health check
     await request(app).get('/.backstage/health/v1/readiness').expect(503, {
-      message: 'Backend has not started yet',
+      message: 'Backend is shuttting down',
       status: 'error',
     });
 
@@ -297,7 +300,7 @@ describe('rootHttpRouterServiceFactory', () => {
       .expect(200, { status: 'ok' });
 
     await request(app).get('/.backstage/health/v1/readiness').expect(503, {
-      message: 'Backend has not started yet',
+      message: 'Backend is shuttting down',
       status: 'error',
     });
 
@@ -320,7 +323,7 @@ describe('rootHttpRouterServiceFactory', () => {
       .expect(200, { status: 'ok' });
 
     await request(app).get('/.backstage/health/v1/readiness').expect(503, {
-      message: 'Backend has not started yet',
+      message: 'Backend is shuttting down',
       status: 'error',
     });
 

packages/backend-defaults/src/entrypoints/rootLifecycle/rootLifecycleServiceFactory.ts

@@ -66,7 +66,7 @@ export class BackendLifecycleImpl implements RootLifecycleService {
   }
 
   #hasBeforeShutdown = false;
-  #beforeShutdownTasks: Array<{ hook: () => void }> = [];
+  #beforeShutdownTasks: Array<{ hook: () => void | Promise<void> }> = [];
 
   addBeforeShutdownHook(hook: () => void): void {
     if (this.#hasBeforeShutdown) {

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

@@ -509,7 +509,7 @@ export interface RootHttpRouterService {
 // @public
 export interface RootLifecycleService extends LifecycleService {
   // (undocumented)
-  addBeforeShutdownHook(hook: () => void): void;
+  addBeforeShutdownHook(hook: () => void | Promise<void>): void;
 }
 
 // @public

packages/backend-plugin-api/src/services/definitions/RootLifecycleService.ts

@@ -24,5 +24,5 @@ import { LifecycleService } from './LifecycleService';
  * @public
  */
 export interface RootLifecycleService extends LifecycleService {
-  addBeforeShutdownHook(hook: () => void): void;
+  addBeforeShutdownHook(hook: () => void | Promise<void>): void;
 }