fix: accept only config as string or human duration

Signed-off-by: Camila Belo <camilaibs@gmail.com>

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

@@ -50,8 +50,16 @@ The `app-config.yaml` file provides configurable options that can be adjusted to
 backend:
   lifecycle:
     # (Optional) The maximum time that paused requests will wait for the service to start, before returning an error (defaults to 5 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 }`.
     startupRequestPauseTimeout: { seconds: 10 }
     # (Optional) The maximum time that the server will wait for stop accepting traffic, before returning an error (defaults to 30 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 }
 ```
 

packages/backend-defaults/config.d.ts

@@ -31,16 +31,24 @@ export interface Config {
     lifecycle?: {
       /**
        * The maximum time that paused requests will wait for the service to start, before returning an error.
-       * If you pass a number or string, it will be interpreted as milliseconds.
-       * Defaults to 5 seconds.
+       * Defaults to 5 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 }`.
        */
-      startupRequestPauseTimeout?: number | string | HumanDuration;
+      startupRequestPauseTimeout?: string | HumanDuration;
       /**
        * The maximum time that the server will wait for stop accepting traffic, before returning an error.
-       * If you pass a number or string, it will be interpreted as milliseconds.
        * Defaults to 30 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 }`.
        */
-      shutdownRequestDelayTimeout?: number | string | HumanDuration;
+      serverShutdownDelay?: string | HumanDuration;
     };
 
     /** Address that the backend should listen to. */

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

@@ -91,13 +91,13 @@ describe('createLifecycleMiddleware', () => {
     return expect(beforeShutdownPromise).resolves.toBeUndefined();
   });
 
-  it('should delay service shutdown for the configured timeout duration', async () => {
+  it('should delay service shutdown for the configured timeout duration - time in human duration', async () => {
     jest.useFakeTimers();
     const configuredTimeout = 20000;
     const lifecycle = new BackendLifecycleImpl(mockServices.rootLogger());
     createLifecycleMiddleware({
       lifecycle,
-      shutdownRequestDelayTimeout: { milliseconds: configuredTimeout },
+      serverShutdownDelay: { milliseconds: configuredTimeout },
     });
     const beforeShutdownPromise = lifecycle.beforeShutdown().then(() => {
       jest.useRealTimers();

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_SHUTDOWN_REQUEST_DELAY_TIMEOUT = { seconds: 30 };
+export const DEFAULT_SERVER_SHUTDOWN_TIMEOUT = { seconds: 30 };
 
 /**
  * Options for {@link createLifecycleMiddleware}.
@@ -39,7 +39,7 @@ export interface LifecycleMiddlewareOptions {
    *
    * Defaults to 30 seconds.
    */
-  shutdownRequestDelayTimeout?: HumanDuration;
+  serverShutdownDelay?: HumanDuration;
 }
 
 /**
@@ -59,7 +59,7 @@ export interface LifecycleMiddlewareOptions {
 export function createLifecycleMiddleware(
   options: LifecycleMiddlewareOptions,
 ): RequestHandler {
-  const { lifecycle, startupRequestPauseTimeout, shutdownRequestDelayTimeout } =
+  const { lifecycle, startupRequestPauseTimeout, serverShutdownDelay } =
     options;
 
   let state: 'init' | 'up' | 'down' = 'init';
@@ -81,7 +81,7 @@ export function createLifecycleMiddleware(
 
   lifecycle.addBeforeShutdownHook(async () => {
     const timeoutMs = durationToMilliseconds(
-      shutdownRequestDelayTimeout ?? DEFAULT_SHUTDOWN_REQUEST_DELAY_TIMEOUT,
+      serverShutdownDelay ?? DEFAULT_SERVER_SHUTDOWN_TIMEOUT,
     );
     return await new Promise(resolve => {
       setTimeout(resolve, timeoutMs);

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

@@ -20,55 +20,13 @@ import {
 } from '@backstage/backend-test-utils';
 import { Express } from 'express';
 import request from 'supertest';
-import {
-  getConfigInHumanDuration,
-  rootHttpRouterServiceFactory,
-} from './rootHttpRouterServiceFactory';
+import { rootHttpRouterServiceFactory } from './rootHttpRouterServiceFactory';
 import {
   ServiceFactory,
   coreServices,
   createServiceFactory,
 } from '@backstage/backend-plugin-api';
 import { BackendLifecycleImpl } from '../rootLifecycle/rootLifecycleServiceFactory';
-import { ConfigReader } from '@backstage/config';
-
-describe('getConfigInHumanDuration', () => {
-  const values = ['20000', 20000, { milliseconds: 20000 }];
-
-  it.each(values)(
-    'should parse the lifecycle startup request timeout from a %s config value',
-    async value => {
-      const key = 'backend.lifecycle.startupRequestPauseTimeout';
-      const config = new ConfigReader({
-        backend: {
-          lifecycle: {
-            startupRequestPauseTimeout: value,
-          },
-        },
-      });
-      expect(getConfigInHumanDuration(config, key)).toMatchObject({
-        milliseconds: 20000,
-      });
-    },
-  );
-
-  it.each(values)(
-    'should parse the lifecycle shutdown delay timeout from a %s config value',
-    async value => {
-      const key = 'backend.lifecycle.shutdownRequestDelayTimeout';
-      const config = new ConfigReader({
-        backend: {
-          lifecycle: {
-            shutdownRequestDelayTimeout: value,
-          },
-        },
-      });
-      expect(getConfigInHumanDuration(config, key)).toMatchObject({
-        milliseconds: 20000,
-      });
-    },
-  );
-});
 
 async function createExpressApp(...dependencies: ServiceFactory[]) {
   let app: Express | undefined = undefined;

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

@@ -21,8 +21,6 @@ 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 {
@@ -33,25 +31,8 @@ import {
 import { DefaultRootHttpRouter } from './DefaultRootHttpRouter';
 import { createHealthRouter } from './createHealthRouter';
 import { createLifecycleMiddleware } from './createLifecycleMiddleware';
-
-export function getConfigInHumanDuration(
-  config: RootConfigService,
-  key: string,
-): HumanDuration | undefined {
-  const value = config.getOptional(key);
-  if (typeof value === 'undefined') {
-    return undefined;
-  }
-  if (typeof value === 'number') {
-    return { milliseconds: value };
-  }
-  if (typeof value === 'string') {
-    return {
-      milliseconds: parseInt(value, 10),
-    };
-  }
-  return readDurationFromConfig(config, { key });
-}
+import { readDurationFromConfig } from '@backstage/config';
+import { HumanDuration } from '@backstage/types';
 
 /**
  * @public
@@ -114,20 +95,24 @@ const rootHttpRouterServiceFactoryWithOptions = (
 
       const healthRouter = createHealthRouter({ config, health });
 
-      const startupRequestPauseTimeout = getConfigInHumanDuration(
-        config,
-        'backend.lifecycle.startupRequestPauseTimeout',
-      );
+      let startupRequestPauseTimeout: HumanDuration | undefined;
+      if (config.has('backend.lifecycle.startupRequestPauseTimeout')) {
+        startupRequestPauseTimeout = readDurationFromConfig(config, {
+          key: 'backend.lifecycle.startupRequestPauseTimeout',
+        });
+      }
 
-      const shutdownRequestDelayTimeout = getConfigInHumanDuration(
-        config,
-        'backend.lifecycle.shutdownRequestDelayTimeout',
-      );
+      let serverShutdownDelay: HumanDuration | undefined;
+      if (config.has('backend.lifecycle.serverShutdownDelay')) {
+        serverShutdownDelay = readDurationFromConfig(config, {
+          key: 'backend.lifecycle.serverShutdownDelay',
+        });
+      }
 
       const lifecycleMiddleware = createLifecycleMiddleware({
         lifecycle,
         startupRequestPauseTimeout,
-        shutdownRequestDelayTimeout,
+        serverShutdownDelay,
       });
 
       const server = await createHttpServer(