Add warning log for disabled Logger mailer due to missing config #3
Conversation
Ndpnt
force-pushed
the
warn-logger-mailer-not-enabled
branch
from
f72f9dd to
5702dc9
Compare
src/utils/logger.js
Outdated
| } else { | ||
| logger.warn('Configuration key "logger.sendMailOnError" was not found; the mailer feature of the Logger module won\'t be enabled'); | ||
| } | ||
| } else { | ||
| logger.warn('Environment variable "SMTP_PASSWORD" was not found; the mailer feature of the Logger module won\'t be enabled'); | ||
| } |
There was a problem hiding this comment.
For readability, I would suggest reverting the condition:
if (!precondition) {
logger.warn('Precondition not met');
} else { // of course, if possible, do an early exit instead
// do normal job
}
src/utils/logger.js
Outdated
| rejectionHandlers: transports, | ||
| }); | ||
| } else { | ||
| logger.warn('Configuration key "logger.sendMailOnError" was not found; the mailer feature of the Logger module won\'t be enabled'); |
There was a problem hiding this comment.
Why is that now a warning? I would assume that if users does not set up this config key, they don't want the mailer logger to be set up 🙂
src/utils/logger.js
Outdated
| logger.warn('Configuration key "logger.sendMailOnError" was not found; the mailer feature of the Logger module won\'t be enabled'); | ||
| } | ||
| } else { | ||
| logger.warn('Environment variable "SMTP_PASSWORD" was not found; the mailer feature of the Logger module won\'t be enabled'); |
There was a problem hiding this comment.
I believe we should log this warning only if logger.sendMailOnError is set up: if there is no intention to send an email anyway, then the missing env variable is not an issue.
CHANGELOG.md
Outdated
|
|
||
| ### Added | ||
|
|
||
| - Add warning log for disabled Logger mailer due to missing config |
There was a problem hiding this comment.
This is accurate; I just suggest focusing on the impact for end users rather than on the technical description: “log emails cannot be sent” rather than “disabled Logger mailer”.
| - Add warning log for disabled Logger mailer due to missing config | |
| - Log a warning in case log emails cannot be sent because of a missing config |
src/utils/logger.js
Outdated
| @@ -37,4 +21,31 @@ const logger = winston.createLogger({ | |||
| rejectionHandlers: transports, | |||
| }); | |||
|
|
|||
| if (process.env.SMTP_PASSWORD) { | |||
There was a problem hiding this comment.
SMTP servers can be set up to send without password authentication, for example if they are on the same private network. It thus seems a potential legit setup for SMTP_PASSWORD to be undefined. In order to keep this relevant check and warning, I suggest that we check explicitly if it is undefined rather than falsy, so that users having no password authentication can set SMTP_PASSWORD to the empty string 🙂
No description provided.