Provide universal getLogger() to beforeRouting

src/config-type.ts

@@ -8,7 +8,7 @@ import { AbstractLogger, ActualLogger } from "./logger-helpers";
 import { Method } from "./method";
 import { AbstractResultHandler } from "./result-handler";
 import { ListenOptions } from "node:net";
-import { ChildLoggerExtractor } from "./server-helpers";
+import { LoggerProvider } from "./server-helpers";
 
 export type InputSource = keyof Pick<
   Request,
@@ -131,13 +131,8 @@ interface GracefulOptions {
 
 type BeforeRouting = (params: {
   app: IRouter;
-  /**
-   * @desc Root logger, same for all requests
-   * @todo reconsider the naming in v21
-   * */
-  logger: ActualLogger;
-  /** @desc Returns a child logger if childLoggerProvider is configured (otherwise root logger) */
-  getChildLogger: ChildLoggerExtractor;
+  /** @desc Returns a child logger for the given request if childLoggerProvider is configured (otherwise root logger) */
+  getLogger: LoggerProvider;
 }) => void | Promise<void>;
 
 export interface HttpConfig {

src/routing.ts

@@ -4,11 +4,10 @@ import { ContentType, contentTypes } from "./content-type";
 import { assertJsonCompatible } from "./deep-checks";
 import { DependsOnMethod } from "./depends-on-method";
 import { AbstractEndpoint } from "./endpoint";
-import { ActualLogger } from "./logger-helpers";
 import { AuxMethod, Method } from "./method";
 import { walkRouting } from "./routing-walker";
 import { ServeStatic } from "./serve-static";
-import { ChildLoggerExtractor } from "./server-helpers";
+import { LoggerProvider } from "./server-helpers";
 
 export interface Routing {
   [SEGMENT: string]: Routing | DependsOnMethod | AbstractEndpoint | ServeStatic;
@@ -18,15 +17,13 @@ export type Parsers = Record<ContentType, RequestHandler[]>;
 
 export const initRouting = ({
   app,
-  rootLogger,
-  getChildLogger,
+  getLogger,
   config,
   routing,
   parsers,
 }: {
   app: IRouter;
-  rootLogger: ActualLogger;
-  getChildLogger: ChildLoggerExtractor;
+  getLogger: LoggerProvider;
   config: CommonConfig;
   routing: Routing;
   parsers?: Parsers;
@@ -43,7 +40,7 @@ export const initRouting = ({
           try {
             assertJsonCompatible(endpoint.getSchema("input"), "in");
           } catch (reason) {
-            rootLogger.warn(
+            getLogger().warn(
               "The final input schema of the endpoint contains an unsupported JSON payload type.",
               { path, method, reason },
             );
@@ -54,7 +51,7 @@ export const initRouting = ({
             try {
               assertJsonCompatible(endpoint.getSchema(variant), "out");
             } catch (reason) {
-              rootLogger.warn(
+              getLogger().warn(
                 `The final ${variant} response schema of the endpoint contains an unsupported JSON payload type.`,
                 { path, method, reason },
               );
@@ -75,7 +72,7 @@ export const initRouting = ({
       };
       const matchingParsers = parsers?.[requestType] || [];
       const handler: RequestHandler = async (request, response) => {
-        const logger = getChildLogger(request);
+        const logger = getLogger(request);
         if (config.cors) {
           const headers =
             typeof config.cors === "function"

src/server-helpers.ts

@@ -19,18 +19,15 @@ type EquippedRequest = Request<
   { [metaSymbol]?: { logger: ActualLogger } }
 >;
 
-export type ChildLoggerExtractor = (request: Request) => ActualLogger;
+export type LoggerProvider = (request?: Request) => ActualLogger;
 
 interface HandlerCreatorParams {
   errorHandler: AbstractResultHandler;
-  getChildLogger: ChildLoggerExtractor;
+  getLogger: LoggerProvider;
 }
 
 export const createParserFailureHandler =
-  ({
-    errorHandler,
-    getChildLogger,
-  }: HandlerCreatorParams): ErrorRequestHandler =>
+  ({ errorHandler, getLogger }: HandlerCreatorParams): ErrorRequestHandler =>
   async (error, request, response, next) => {
     if (!error) return next();
     return errorHandler.execute({
@@ -42,18 +39,18 @@ export const createParserFailureHandler =
       input: null,
       output: null,
       options: {},
-      logger: getChildLogger(request),
+      logger: getLogger(request),
     });
   };
 
 export const createNotFoundHandler =
-  ({ errorHandler, getChildLogger }: HandlerCreatorParams): RequestHandler =>
+  ({ errorHandler, getLogger }: HandlerCreatorParams): RequestHandler =>
   async (request, response) => {
     const error = createHttpError(
       404,
       `Can not ${request.method} ${request.path}`,
     );
-    const logger = getChildLogger(request);
+    const logger = getLogger(request);
     try {
       errorHandler.execute({
         request,
@@ -88,10 +85,10 @@ export const createUploadLogger = (
 ): Pick<Console, "log"> => ({ log: logger.debug.bind(logger) });
 
 export const createUploadParsers = async ({
-  getChildLogger,
+  getLogger,
   config,
 }: {
-  getChildLogger: ChildLoggerExtractor;
+  getLogger: LoggerProvider;
   config: ServerConfig;
 }): Promise<RequestHandler[]> => {
   const uploader = await loadPeer<typeof fileUpload>("express-fileupload");
@@ -100,7 +97,7 @@ export const createUploadParsers = async ({
   };
   const parsers: RequestHandler[] = [];
   parsers.push(async (request, response, next) => {
-    const logger = getChildLogger(request);
+    const logger = getLogger(request);
     try {
       await beforeUpload?.({ request, logger });
     } catch (error) {
@@ -142,10 +139,11 @@ export const createLoggingMiddleware =
     next();
   };
 
-export const makeChildLoggerExtractor =
-  (fallback: ActualLogger): ChildLoggerExtractor =>
+export const makeLoggerProvider =
+  (fallback: ActualLogger): LoggerProvider =>
   (request) =>
-    (request as EquippedRequest).res?.locals[metaSymbol]?.logger || fallback;
+    (request && (request as EquippedRequest).res?.locals[metaSymbol]?.logger) ||
+    fallback;
 
 export const installDeprecationListener = (logger: ActualLogger) =>
   process.on("deprecation", ({ message, namespace, name, stack }) =>

src/server.ts

@@ -18,7 +18,7 @@ import {
   createNotFoundHandler,
   createParserFailureHandler,
   createUploadParsers,
-  makeChildLoggerExtractor,
+  makeLoggerProvider,
   installDeprecationListener,
   moveRaw,
   installTerminationListener,
@@ -37,8 +37,8 @@ const makeCommonEntities = (config: CommonConfig) => {
   });
   installDeprecationListener(rootLogger);
   const loggingMiddleware = createLoggingMiddleware({ rootLogger, config });
-  const getChildLogger = makeChildLoggerExtractor(rootLogger);
-  const commons = { getChildLogger, errorHandler };
+  const getLogger = makeLoggerProvider(rootLogger);
+  const commons = { getLogger, errorHandler };
   const notFoundHandler = createNotFoundHandler(commons);
   const parserFailureHandler = createParserFailureHandler(commons);
   return {
@@ -51,13 +51,12 @@ const makeCommonEntities = (config: CommonConfig) => {
 };
 
 export const attachRouting = (config: AppConfig, routing: Routing) => {
-  const { rootLogger, getChildLogger, notFoundHandler, loggingMiddleware } =
+  const { rootLogger, getLogger, notFoundHandler, loggingMiddleware } =
     makeCommonEntities(config);
   initRouting({
     app: config.app.use(loggingMiddleware),
-    rootLogger,
     routing,
-    getChildLogger,
+    getLogger,
     config,
   });
   return { notFoundHandler, logger: rootLogger };
@@ -66,7 +65,7 @@ export const attachRouting = (config: AppConfig, routing: Routing) => {
 export const createServer = async (config: ServerConfig, routing: Routing) => {
   const {
     rootLogger,
-    getChildLogger,
+    getLogger,
     notFoundHandler,
     parserFailureHandler,
     loggingMiddleware,
@@ -86,18 +85,12 @@ export const createServer = async (config: ServerConfig, routing: Routing) => {
     json: [config.jsonParser || express.json()],
     raw: [config.rawParser || express.raw(), moveRaw],
     upload: config.upload
-      ? await createUploadParsers({ config, getChildLogger })
+      ? await createUploadParsers({ config, getLogger })
       : [],
   };
 
-  if (config.beforeRouting) {
-    await config.beforeRouting({
-      app,
-      logger: rootLogger,
-      getChildLogger,
-    });
-  }
-  initRouting({ app, routing, rootLogger, getChildLogger, config, parsers });
+  if (config.beforeRouting) await config.beforeRouting({ app, getLogger });
+  initRouting({ app, routing, getLogger, config, parsers });
   app.use(parserFailureHandler, notFoundHandler);
 
   const makeStarter =

tests/system/system.spec.ts

@@ -112,10 +112,10 @@ describe("App in production mode", async () => {
   const config = createConfig({
     http: { listen: port },
     compression: { threshold: 1 },
-    beforeRouting: ({ app, getChildLogger }) => {
+    beforeRouting: ({ app, getLogger }) => {
       depd("express")("Sample deprecation message");
       app.use((req, {}, next) => {
-        const childLogger = getChildLogger(req);
+        const childLogger = getLogger(req);
         assert("isChild" in childLogger && childLogger.isChild);
         next();
       });

tests/unit/routing.spec.ts

@@ -67,10 +67,9 @@ describe("Routing", () => {
       const rootLogger = new BuiltinLogger({ level: "silent" });
       initRouting({
         app: appMock as unknown as IRouter,
-        getChildLogger: () => rootLogger,
+        getLogger: () => rootLogger,
         config: configMock as CommonConfig,
         routing,
-        rootLogger,
       });
       expect(appMock.get).toHaveBeenCalledTimes(2);
       expect(appMock.post).toHaveBeenCalledTimes(2);
@@ -98,10 +97,9 @@ describe("Routing", () => {
       const rootLogger = new BuiltinLogger({ level: "silent" });
       initRouting({
         app: appMock as unknown as IRouter,
-        getChildLogger: () => rootLogger,
+        getLogger: () => rootLogger,
         config: configMock as CommonConfig,
         routing,
-        rootLogger,
       });
       expect(staticMock).toHaveBeenCalledWith(__dirname, { dotfiles: "deny" });
       expect(appMock.use).toHaveBeenCalledTimes(1);
@@ -140,10 +138,9 @@ describe("Routing", () => {
       const rootLogger = new BuiltinLogger({ level: "silent" });
       initRouting({
         app: appMock as unknown as IRouter,
-        getChildLogger: () => rootLogger,
+        getLogger: () => rootLogger,
         config: configMock as CommonConfig,
         routing,
-        rootLogger,
       });
       expect(appMock.get).toHaveBeenCalledTimes(1);
       expect(appMock.post).toHaveBeenCalledTimes(1);
@@ -179,10 +176,9 @@ describe("Routing", () => {
       expect(() =>
         initRouting({
           app: appMock as unknown as IRouter,
-          getChildLogger: () => rootLogger,
+          getLogger: () => rootLogger,
           config: configMock as CommonConfig,
           routing,
-          rootLogger,
         }),
       ).toThrowErrorMatchingSnapshot();
     });
@@ -225,10 +221,9 @@ describe("Routing", () => {
       const rootLogger = new BuiltinLogger({ level: "silent" });
       initRouting({
         app: appMock as unknown as IRouter,
-        getChildLogger: () => rootLogger,
+        getLogger: () => rootLogger,
         config: configMock as CommonConfig,
         routing,
-        rootLogger,
       });
       expect(appMock.options).toHaveBeenCalledTimes(1);
       expect(appMock.options.mock.calls[0][0]).toBe("/hello");
@@ -265,10 +260,9 @@ describe("Routing", () => {
       const rootLogger = new BuiltinLogger({ level: "silent" });
       initRouting({
         app: appMock as unknown as IRouter,
-        getChildLogger: () => rootLogger,
+        getLogger: () => rootLogger,
         config: configMock as CommonConfig,
         routing,
-        rootLogger,
       });
       expect(appMock.get).toHaveBeenCalledTimes(1);
       expect(appMock.get.mock.calls[0][0]).toBe("/v1/user/:id");
@@ -295,10 +289,9 @@ describe("Routing", () => {
       const rootLogger = new BuiltinLogger({ level: "silent" });
       initRouting({
         app: appMock as unknown as IRouter,
-        getChildLogger: () => rootLogger,
+        getLogger: () => rootLogger,
         config: configMock as CommonConfig,
         routing,
-        rootLogger,
       });
       expect(appMock.get).toHaveBeenCalledTimes(2);
       expect(appMock.get.mock.calls[0][0]).toBe("/v1/user/:id");
@@ -316,9 +309,8 @@ describe("Routing", () => {
       const rootLogger = new BuiltinLogger({ level: "silent" });
       expect(() =>
         initRouting({
-          rootLogger,
           app: appMock as unknown as IRouter,
-          getChildLogger: () => rootLogger,
+          getLogger: () => rootLogger,
           config: configMock as CommonConfig,
           routing: {
             v1: {
@@ -329,9 +321,8 @@ describe("Routing", () => {
       ).toThrowErrorMatchingSnapshot();
       expect(() =>
         initRouting({
-          rootLogger,
           app: appMock as unknown as IRouter,
-          getChildLogger: () => rootLogger,
+          getLogger: () => rootLogger,
           config: configMock as CommonConfig,
           routing: {
             "v1/user/retrieve": endpointMock,
@@ -362,12 +353,10 @@ describe("Routing", () => {
           },
         },
       };
-      const rootLogger = makeLoggerMock();
-      const childLogger = makeLoggerMock();
+      const getLoggerMock = vi.fn(() => makeLoggerMock());
       initRouting({
-        rootLogger,
         app: appMock as unknown as IRouter,
-        getChildLogger: () => childLogger,
+        getLogger: getLoggerMock,
         config: configMock as CommonConfig,
         routing,
       });
@@ -380,15 +369,16 @@ describe("Routing", () => {
       const responseMock = makeResponseMock();
       const nextMock = vi.fn();
       await routeHandler(requestMock, responseMock, nextMock);
+      expect(getLoggerMock).toHaveBeenCalledWith(requestMock);
       expect(nextMock).toHaveBeenCalledTimes(0);
       expect(handlerMock).toHaveBeenCalledTimes(1);
-      expect(childLogger._getLogs().error).toHaveLength(0);
+      expect(
+        getLoggerMock.mock.results.pop()!.value._getLogs().error,
+      ).toHaveLength(0);
       expect(handlerMock).toHaveBeenCalledWith({
-        input: {
-          test: 123,
-        },
+        input: { test: 123 },
         options: {},
-        logger: childLogger,
+        logger: getLoggerMock.mock.results.pop()!.value,
       });
       expect(responseMock._getStatusCode()).toBe(200);
       expect(responseMock._getJSONData()).toEqual({
@@ -415,9 +405,8 @@ describe("Routing", () => {
       const configMock = { cors: false, startupLogo: false };
       const rootLogger = makeLoggerMock();
       initRouting({
-        rootLogger,
         app: appMock as unknown as IRouter,
-        getChildLogger: () => rootLogger,
+        getLogger: () => rootLogger,
         config: configMock as CommonConfig,
         routing: { path: endpoint },
       });