Skip to content

logger: change NONE loglevel to sys.maxsize #4258

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

Merged
merged 2 commits into from
Dec 20, 2021

Conversation

bastimeyer
Copy link
Member

Resolves #4235

The reason why there are still log messages being written to stdout/stderr when any "silent" CLI argument is set (--json, --quiet, --stream-url or even --loglevel=none) is because of how Streamlink's NONE log level gets interpreted in Python's logging module.

Each logger instance (subclasses of logging.Logger or streamlink.logger.StreamlinkLogger) can optionally have their own log level. When a logger instance needs to check whether a certain message can be written to the output or not, it first tries to read its own log level value, and if it is set to logging.UNSET (which is the default and its value is 0), then it continues with its parent logger instance until it reaches logging.root (RootLogger). This RootLogger does have a default log level of 30, aka. "warning".
https://github.com/python/cpython/blame/3.10/Lib/logging/__init__.py#L1926-L1927

logging.Logger.getEffectiveLevel():
https://github.com/python/cpython/blame/3.10/Lib/logging/__init__.py#L1701-L1713

    def getEffectiveLevel(self):
        """
        Get the effective level for this logger.
        Loop through this logger and its parents in the logger hierarchy,
        looking for a non-zero logging level. Return the first one found.
        """
        logger = self
        while logger:
            if logger.level:
                return logger.level
            logger = logger.parent
        return NOTSET

Since child loggers usually don't have their own log level set, the loggers like the one in the YouTube plugin for example look up the level of Streamlink's own root logger streamlink.logger.root. If the level of Streamlink's root logger however is set to NONE (0), then the level of the global RootLogger will be used ("warning"), which means everything equal to or above "warning", like error log messages for example, will still be logged.


The fix is to change the streamlink.logger.NONE value from 0 to sys.maxsize, so that getEffectiveLevel() doesn't continue with the next parent logger if one of the logger instance's level is set to NONE. This has been broken since the reimplementation of the logger module in 2018.

Please review carefully before merging, because I don't want to unintentionally break stuff.

The other changes are all optional. See the commit messages for details.


#4235 (comment)

$ streamlink youtube.com/c/LudwigAhgren -j
{
  "error": "No playable streams found on this URL: youtube.com/c/LudwigAhgren"
}

- Change NONE from 0 to sys.maxsize:
  This fixes an issue with `logging.Logger.getEffectiveLevel()` which
  would otherwise choose `logging.root` as logger instance when
  Streamlink's loglevel was set to "none", whose default log level is
  set to "warning", so errors did still get logged.
- Rearrange global scope vars
- Add support for uppercase "TRACE" log level name
- Add typings to baseConfig()
- Add tests for log streams and child loggers
Copy link
Member

@gravyboat gravyboat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me @bastimeyer, good investigation in to the root cause. I'll leave this open today to see if others have any concerns, if not let's merge this in.

@back-to back-to merged commit 6c3bd9a into streamlink:master Dec 20, 2021
@bastimeyer bastimeyer deleted the logger/common-log-level branch December 20, 2021 16:50
Billy2011 added a commit to Billy2011/streamlink-27 that referenced this pull request Dec 20, 2021
Billy2011 added a commit to Billy2011/streamlink-27 that referenced this pull request Dec 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

output with --json flag is not parseable if handled error occurs
3 participants