-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: getLogLevel return type #44258
fix: getLogLevel return type #44258
Conversation
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
Ah yes had the same issue as something set the default to try {
$loglevel = (int)$this->config->getValue('loglevel', ILogger::WARN);
} catch (\TypeError $e) {
// can we use the logger here? if not then use errorlog
} The reason would be to prevent confusion why the loglevel is not respected, got some similar issues in the past leading to wasted hours of debugging 😅 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works but I think the logging (see commit) would make sense
$configLogLevel = $this->config->getValue('loglevel', ILogger::WARN); | ||
return min(is_int($configLogLevel) ? $configLogLevel : ILogger::WARN, ILogger::FATAL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As @susnux pointed out we should log an error or cast the value to int.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ref #44262
Fixes #43534