#8 Winston Logger ECS Deployment Fix #9
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| LOG_DIR=logs | ||
|
|
||
| # leave unset (defaults to false in production) | ||
| ENABLE_FILE_LOGGING=true |
There was a problem hiding this comment.
Consider setting ENABLE_FILE_LOGGING to false by default in the example environment file to prevent accidental file logging in production environments.
| DEFAULT_NODE_ENV: 'development', | ||
| DEFAULT_LOG_LEVEL: 'info', | ||
| DEFAULT_LOG_DIR: 'logs', | ||
| DEFAULT_ENABLE_FILE_LOGGING: false, |
There was a problem hiding this comment.
Consider making DEFAULT_ENABLE_FILE_LOGGING configurable through environment variables or configuration files to allow flexibility in different deployment environments.
| } | ||
|
|
||
| private createWinstonLogger(): Logger { | ||
| const baseTransports: any[] = [ |
There was a problem hiding this comment.
Avoid using any[] for baseTransports. Consider specifying a more precise type for the array elements, such as TransportStream[], to improve type safety and maintainability.
| level: process.env.LOG_LEVEL || CONFIG.APP.DEFAULT_LOG_LEVEL, | ||
| format: format.combine( | ||
| format.colorize(), | ||
| format.printf(({ timestamp, level, message, context, ...meta }) => { |
There was a problem hiding this comment.
The format.printf function is used to format log messages. Ensure that all possible types of message and context are handled to avoid unexpected runtime errors. Consider adding validation or default handling for unexpected types.
| ]; | ||
|
|
||
| // Add file transports conditionally based on environment | ||
| const shouldEnableFileLogging = this.shouldEnableFileLogging(); |
There was a problem hiding this comment.
The method shouldEnableFileLogging is called to determine if file logging should be enabled. Ensure this method is implemented correctly and returns a boolean value. Consider adding error handling or logging if the method fails or returns an unexpected value.
| try { | ||
| baseTransports.push( | ||
| new transports.File({ | ||
| filename: `${process.env.LOG_DIR || CONFIG.APP.DEFAULT_LOG_DIR}/error.log`, |
There was a problem hiding this comment.
The file path for the log files is constructed using environment variables or default configuration. Ensure that process.env.LOG_DIR and CONFIG.APP.DEFAULT_LOG_DIR are correctly set and accessible. Consider validating these paths to prevent potential runtime errors.
| }), | ||
| ); | ||
| } catch (error) { | ||
| // If file transport creation fails (e.g., read-only filesystem), |
There was a problem hiding this comment.
The error handling for file transport creation logs a warning to the console. Consider using the existing logger to log this warning instead of console.warn to maintain consistency in logging practices.
| */ | ||
| private shouldEnableFileLogging(): boolean { | ||
| const nodeEnv = process.env.NODE_ENV; | ||
| const enableFileLogging = process.env.ENABLE_FILE_LOGGING; |
There was a problem hiding this comment.
Consider adding validation for the ENABLE_FILE_LOGGING environment variable to ensure it only accepts 'true' or 'false' values. This can prevent unexpected behavior if the variable is set to an invalid value.
|
|
||
| // In production, only enable file logging if explicitly requested | ||
| if (nodeEnv === 'production') { | ||
| return enableFileLogging === 'true'; |
There was a problem hiding this comment.
The condition enableFileLogging === 'true' is a string comparison. Ensure that ENABLE_FILE_LOGGING is always set as a string in the environment variables to avoid potential issues.
| logging: { | ||
| level: process.env.LOG_LEVEL || 'info', | ||
| directory: process.env.LOG_DIR || 'logs', | ||
| enableFileLogging: process.env.ENABLE_FILE_LOGGING === 'true', |
There was a problem hiding this comment.
Consider using a more explicit check for the environment variable, such as process.env.ENABLE_FILE_LOGGING?.toLowerCase() === 'true', to ensure that the comparison is case-insensitive and handles undefined values gracefully.
Please set NODE_ENV=production or ENABLE_FILE_LOGGING=false to avoid the Winston logger writting logs to File system.