From 06ae64ce4c0afd3a4a06197e1ff7bc81a86f1049 Mon Sep 17 00:00:00 2001 From: 0xkenj1 Date: Tue, 30 Jul 2024 12:04:30 -0300 Subject: [PATCH] fix: pr comments --- packages/shared/src/logger.ts | 28 ++++++++++++++-------------- packages/shared/src/types/logger.ts | 2 -- packages/shared/tests/logger.spec.ts | 23 +++++++++++++---------- 3 files changed, 27 insertions(+), 26 deletions(-) diff --git a/packages/shared/src/logger.ts b/packages/shared/src/logger.ts index fc2c678..57cd9ad 100644 --- a/packages/shared/src/logger.ts +++ b/packages/shared/src/logger.ts @@ -1,12 +1,17 @@ import { createLogger, format, transports, Logger as WinstonLogger } from "winston"; -import { ILogger, LogLevel } from "./index.js"; +import { ILogger } from "./index.js"; + +type LogLevel = "error" | "warn" | "info" | "debug"; + +const validLogLevels: LogLevel[] = ["error", "warn", "info", "debug"]; export class Logger implements ILogger { private logger: WinstonLogger; private static instance: Logger | null; - - private constructor(private level: LogLevel) { + private level: LogLevel; + private constructor() { + this.level = this.isValidLogLevel(process.env.LOG_LEVEL) ? process.env.LOG_LEVEL : "info"; this.logger = createLogger({ level: this.level, format: format.combine( @@ -14,7 +19,7 @@ export class Logger implements ILogger { format.timestamp({ format: "YYYY-MM-DD HH:mm:ss" }), format.errors({ stack: true }), format.printf(({ level, message, timestamp, stack }) => { - return `${timestamp} ${level}: ${stack || message}`; + return `${timestamp} ${level}: ${stack ?? message ?? ""}`; }), ), transports: [new transports.Console()], @@ -25,20 +30,15 @@ export class Logger implements ILogger { * @param level The log level to be used by the logger. * @returns The instance of the Logger class. */ - public static getInstance(level?: LogLevel): Logger { + public static getInstance(): Logger { if (!Logger.instance) { - if (!level) { - throw new Error("Initial configuration is required for the first instantiation."); - } - Logger.instance = new Logger(level); - } else { - Logger.instance.warn( - `Logger instance already exists. Returning the existing instance with log level ${Logger.instance.level}.`, - ); + Logger.instance = new Logger(); } - return Logger.instance; } + isValidLogLevel(level: any): level is LogLevel { + return validLogLevels.includes(level); + } info(message: string) { this.logger.info(message); diff --git a/packages/shared/src/types/logger.ts b/packages/shared/src/types/logger.ts index 08a234f..b647e39 100644 --- a/packages/shared/src/types/logger.ts +++ b/packages/shared/src/types/logger.ts @@ -7,5 +7,3 @@ export interface ILogger { warn: (message: string) => void; debug: (message: string) => void; } - -export type LogLevel = "error" | "warn" | "info" | "debug"; diff --git a/packages/shared/tests/logger.spec.ts b/packages/shared/tests/logger.spec.ts index 83137f3..5a7a86a 100644 --- a/packages/shared/tests/logger.spec.ts +++ b/packages/shared/tests/logger.spec.ts @@ -4,21 +4,24 @@ import { Logger } from "../src/logger.js"; describe("Logger Singleton", () => { it("creates a logger instance with the given log level", () => { - const logger = Logger.getInstance("info"); + const logger = Logger.getInstance(); expect(logger).toBeInstanceOf(Logger); expect(() => Logger.getInstance()).not.toThrow(); }); - it("throws an error if no log level is provided on first instantiation", () => { - Logger["instance"] = null; - expect(() => Logger.getInstance()).toThrow( - new Error("Initial configuration is required for the first instantiation."), - ); - }); - it("returns the same instance if called multiple times", () => { - const logger1 = Logger.getInstance("info"); - const logger2 = Logger.getInstance("warn"); + const logger1 = Logger.getInstance(); + const logger2 = Logger.getInstance(); expect(logger1).toBe(logger2); }); + it("sets level correctly based on LOG_LEVEL env var", () => { + let logger1 = Logger.getInstance(); + expect(logger1["level"]).toEqual("info"); + + Logger["instance"] = null; + process.env.LOG_LEVEL = "debug"; + logger1 = Logger.getInstance(); + + expect(logger1["level"]).toEqual("debug"); + }); });