Skip to content

Commit

Permalink
Provide universal getLogger() to beforeRouting (#2167)
Browse files Browse the repository at this point in the history
Instead of `logger` and `getChildLogger`
  • Loading branch information
RobinTail authored Nov 14, 2024
1 parent cd422c6 commit bc3db93
Show file tree
Hide file tree
Showing 12 changed files with 192 additions and 133 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
- The `serializer` property of `Documentation` and `Integration` constructor argument removed;
- The `originalError` property of `InputValidationError` and `OutputValidationError` removed (use `cause` instead);
- The `getStatusCodeFromError()` method removed (use the `ensureHttpError().statusCode` instead);
- Both `logger` and `getChildLogger` properties of `beforeRouting` argument are replaced with all-purpose `getLogger`:
- It returns the child logger for the given request (if configured) or the configured logger otherwise.
- Specifying `method` or `methods` for `EndpointsFactory::build()` made optional and when it's omitted:
- If the endpoint is assigned to a route using `DependsOnMethod` instance, the corresponding method is used;
- Otherwise `GET` method is implied by default.
Expand Down
6 changes: 5 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -370,9 +370,13 @@ import { createConfig } from "express-zod-api";
import ui from "swagger-ui-express";

const config = createConfig({
beforeRouting: ({ app, logger, getChildLogger }) => {
beforeRouting: ({ app, getLogger }) => {
const logger = getLogger();
logger.info("Serving the API documentation at https://example.com/docs");
app.use("/docs", ui.serve, ui.setup(documentation));
app.use("/custom", (req, res, next) => {
const childLogger = getLogger(req); // if childLoggerProvider is configured
});
},
});
```
Expand Down
11 changes: 3 additions & 8 deletions src/config-type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 { GetLogger } from "./server-helpers";

export type InputSource = keyof Pick<
Request,
Expand Down Expand Up @@ -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 child logger for the given request (if configured) or the configured logger otherwise */
getLogger: GetLogger;
}) => void | Promise<void>;

export interface HttpConfig {
Expand Down
40 changes: 40 additions & 0 deletions src/migration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,21 @@ import { name as importName } from "../package.json";
const createConfigName = "createConfig";
const createServerName = "createServer";
const serverPropName = "server";
const beforeRoutingPropName = "beforeRouting";
const httpServerPropName = "httpServer";
const httpsServerPropName = "httpsServer";
const originalErrorPropName = "originalError";
const getStatusCodeFromErrorMethod = "getStatusCodeFromError";
const loggerPropName = "logger";
const getChildLoggerPropName = "getChildLogger";

const changedProps = {
[serverPropName]: "http",
[httpServerPropName]: "servers",
[httpsServerPropName]: "servers",
[originalErrorPropName]: "cause",
[loggerPropName]: "getLogger",
[getChildLoggerPropName]: "getLogger",
};

const changedMethods = {
Expand Down Expand Up @@ -199,6 +204,41 @@ const v21 = ESLintUtils.RuleCreator.withoutDocs({
});
}
},
[`${NT.Property}[key.name="${beforeRoutingPropName}"] ${NT.ArrowFunctionExpression} ${NT.Identifier}[name="${loggerPropName}"]`]:
(node: TSESTree.Identifier) => {
const { parent } = node;
const isProp = isPropWithId(parent);
if (isProp && parent.value === node) return; // not for renames
const replacement = `${changedProps[node.name as keyof typeof changedProps]}${isProp ? "" : "()"}`;
ctx.report({
node,
messageId: "change",
data: {
subject: isProp ? "property" : "const",
from: node.name,
to: replacement,
},
fix: (fixer) => fixer.replaceText(node, replacement),
});
},
[`${NT.Property}[key.name="${beforeRoutingPropName}"] ${NT.ArrowFunctionExpression} ${NT.Identifier}[name="${getChildLoggerPropName}"]`]:
(node: TSESTree.Identifier) => {
const { parent } = node;
const isProp = isPropWithId(parent);
if (isProp && parent.value === node) return; // not for renames
const replacement =
changedProps[node.name as keyof typeof changedProps];
ctx.report({
node,
messageId: "change",
data: {
subject: isProp ? "property" : "method",
from: node.name,
to: replacement,
},
fix: (fixer) => fixer.replaceText(node, replacement),
});
},
}),
});

Expand Down
15 changes: 6 additions & 9 deletions src/routing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 { GetLogger } from "./server-helpers";

export interface Routing {
[SEGMENT: string]: Routing | DependsOnMethod | AbstractEndpoint | ServeStatic;
Expand All @@ -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: GetLogger;
config: CommonConfig;
routing: Routing;
parsers?: Parsers;
Expand All @@ -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 },
);
Expand All @@ -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 },
);
Expand All @@ -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"
Expand Down
38 changes: 18 additions & 20 deletions src/server-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,16 @@ type EquippedRequest = Request<
{ [metaSymbol]?: { logger: ActualLogger } }
>;

export type ChildLoggerExtractor = (request: Request) => ActualLogger;
/** @desc Returns child logger for the given request (if configured) or the configured logger otherwise */
export type GetLogger = (request?: Request) => ActualLogger;

interface HandlerCreatorParams {
errorHandler: AbstractResultHandler;
getChildLogger: ChildLoggerExtractor;
getLogger: GetLogger;
}

export const createParserFailureHandler =
({
errorHandler,
getChildLogger,
}: HandlerCreatorParams): ErrorRequestHandler =>
({ errorHandler, getLogger }: HandlerCreatorParams): ErrorRequestHandler =>
async (error, request, response, next) => {
if (!error) return next();
return errorHandler.execute({
Expand All @@ -42,18 +40,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,
Expand Down Expand Up @@ -88,10 +86,10 @@ export const createUploadLogger = (
): Pick<Console, "log"> => ({ log: logger.debug.bind(logger) });

export const createUploadParsers = async ({
getChildLogger,
getLogger,
config,
}: {
getChildLogger: ChildLoggerExtractor;
getLogger: GetLogger;
config: ServerConfig;
}): Promise<RequestHandler[]> => {
const uploader = await loadPeer<typeof fileUpload>("express-fileupload");
Expand All @@ -100,7 +98,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) {
Expand All @@ -126,26 +124,26 @@ export const moveRaw: RequestHandler = (req, {}, next) => {
/** @since v19 prints the actual path of the request, not a configured route, severity decreased to debug level */
export const createLoggingMiddleware =
({
rootLogger,
logger: parent,
config,
}: {
rootLogger: ActualLogger;
logger: ActualLogger;
config: CommonConfig;
}): RequestHandler =>
async (request, response, next) => {
const logger = config.childLoggerProvider
? await config.childLoggerProvider({ request, parent: rootLogger })
: rootLogger;
const logger =
(await config.childLoggerProvider?.({ request, parent })) || parent;
logger.debug(`${request.method}: ${request.path}`);
if (request.res)
(request as EquippedRequest).res!.locals[metaSymbol] = { logger };
next();
};

export const makeChildLoggerExtractor =
(fallback: ActualLogger): ChildLoggerExtractor =>
export const makeGetLogger =
(fallback: ActualLogger): GetLogger =>
(request) =>
(request as EquippedRequest).res?.locals[metaSymbol]?.logger || fallback;
(request as EquippedRequest | undefined)?.res?.locals[metaSymbol]?.logger ||
fallback;

export const installDeprecationListener = (logger: ActualLogger) =>
process.on("deprecation", ({ message, namespace, name, stack }) =>
Expand Down
49 changes: 19 additions & 30 deletions src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
createNotFoundHandler,
createParserFailureHandler,
createUploadParsers,
makeChildLoggerExtractor,
makeGetLogger,
installDeprecationListener,
moveRaw,
installTerminationListener,
Expand All @@ -28,45 +28,44 @@ import { getStartupLogo } from "./startup-logo";
const makeCommonEntities = (config: CommonConfig) => {
if (config.startupLogo !== false) console.log(getStartupLogo());
const errorHandler = config.errorHandler || defaultResultHandler;
const rootLogger = isLoggerInstance(config.logger)
const logger = isLoggerInstance(config.logger)
? config.logger
: new BuiltinLogger(config.logger);
rootLogger.debug("Running", {
logger.debug("Running", {
build: process.env.TSUP_BUILD || "from sources",
env: process.env.NODE_ENV || "development",
});
installDeprecationListener(rootLogger);
const loggingMiddleware = createLoggingMiddleware({ rootLogger, config });
const getChildLogger = makeChildLoggerExtractor(rootLogger);
const commons = { getChildLogger, errorHandler };
installDeprecationListener(logger);
const loggingMiddleware = createLoggingMiddleware({ logger, config });
const getLogger = makeGetLogger(logger);
const commons = { getLogger, errorHandler };
const notFoundHandler = createNotFoundHandler(commons);
const parserFailureHandler = createParserFailureHandler(commons);
return {
...commons,
rootLogger,
logger,
notFoundHandler,
parserFailureHandler,
loggingMiddleware,
};
};

export const attachRouting = (config: AppConfig, routing: Routing) => {
const { rootLogger, getChildLogger, notFoundHandler, loggingMiddleware } =
const { logger, getLogger, notFoundHandler, loggingMiddleware } =
makeCommonEntities(config);
initRouting({
app: config.app.use(loggingMiddleware),
rootLogger,
routing,
getChildLogger,
getLogger,
config,
});
return { notFoundHandler, logger: rootLogger };
return { notFoundHandler, logger };
};

export const createServer = async (config: ServerConfig, routing: Routing) => {
const {
rootLogger,
getChildLogger,
logger,
getLogger,
notFoundHandler,
parserFailureHandler,
loggingMiddleware,
Expand All @@ -86,23 +85,17 @@ 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 });
await config.beforeRouting?.({ app, getLogger });
initRouting({ app, routing, getLogger, config, parsers });
app.use(parserFailureHandler, notFoundHandler);

const makeStarter =
(server: http.Server | https.Server, subject: HttpConfig["listen"]) => () =>
server.listen(subject, () => rootLogger.info("Listening", subject));
server.listen(subject, () => logger.info("Listening", subject));

const created: Array<http.Server | https.Server> = [];
const starters: Array<() => http.Server | https.Server> = [];
Expand All @@ -119,15 +112,11 @@ export const createServer = async (config: ServerConfig, routing: Routing) => {

if (config.gracefulShutdown) {
installTerminationListener({
logger,
servers: created,
logger: rootLogger,
options: config.gracefulShutdown === true ? {} : config.gracefulShutdown,
});
}

return {
app,
logger: rootLogger,
servers: starters.map((starter) => starter()),
};
return { app, logger, servers: starters.map((starter) => starter()) };
};
4 changes: 2 additions & 2 deletions tests/system/system.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
Expand Down
Loading

0 comments on commit bc3db93

Please sign in to comment.