Skip to content
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

Jump to bottom

logging.disable('WARN') breaks AsyncIO #86810

Closed
OverLordGoldDragon mannequin opened this issue Dec 15, 2020 · 10 comments
Closed

logging.disable('WARN') breaks AsyncIO #86810

OverLordGoldDragon mannequin opened this issue Dec 15, 2020 · 10 comments
Labels
3.8 only security fixes 3.9 only security fixes 3.10 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@OverLordGoldDragon
Copy link
Mannequin

OverLordGoldDragon mannequin commented Dec 15, 2020

BPO 42644
Nosy @asvetlov, @masklinn, @1st1, @Carreau, @miss-islington, @OverLordGoldDragon
PRs
  • bpo-42644: Validate values in logging.disable() #23786
  • [3.9] bpo-42644: Validate values in logging.disable() (GH-23786) #23796
  • [3.8] bpo-42644: Validate values in logging.disable() (GH-23786) #23797
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = <Date 2020-12-16.10:51:05.586>
created_at = <Date 2020-12-15.10:00:46.185>
labels = ['3.8', 'type-bug', 'library', '3.9', '3.10']
title = "logging.disable('WARN') breaks AsyncIO"
updated_at = <Date 2020-12-16.10:51:05.586>
user = 'https://github.com/OverLordGoldDragon'

    bugs.python.org fields:

    activity = <Date 2020-12-16.10:51:05.586>
actor = 'asvetlov'
assignee = 'none'
closed = True
closed_date = <Date 2020-12-16.10:51:05.586>
closer = 'asvetlov'
components = ['Library (Lib)']
creation = <Date 2020-12-15.10:00:46.185>
creator = 'OverLordGoldDragon'
dependencies = []
files = []
hgrepos = []
issue_num = 42644
keywords = ['patch']
message_count = 10.0
messages = ['383040', '383053', '383056', '383061', '383083', '383085', '383086', '383130', '383133', '383134']
nosy_count = 6.0
nosy_names = ['asvetlov', 'xmorel', 'yselivanov', 'mbussonn', 'miss-islington', 'OverLordGoldDragon']
pr_nums = ['23786', '23796', '23797']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue42644'
versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']
    @OverLordGoldDragon
    Copy link
    Mannequin Author

    OverLordGoldDragon mannequin commented Dec 15, 2020 

    import logging
with logging.disable('WARN'): pass

    See: ipython/ipython#12713
    I'm not actually sure what's happening here, just reporting.

    @OverLordGoldDragon OverLordGoldDragon mannequin added topic-asyncio type-crash A hard crash of the interpreter, possibly with a core dump 3.7 (EOL) end of life labels Dec 15, 2020
    @asvetlov
    Copy link
    Contributor

    Reporting what? Please elaborate.

    @OverLordGoldDragon
    Copy link
    Mannequin Author

    OverLordGoldDragon mannequin commented Dec 15, 2020

    Everything's in the linked issue; in summary, the command kills IPython, and an IPython maintainer showed code suggesting the problem's rooted in CPython's improper handling of 'WARN' as alias for logging.WARN.

    @masklinn
    Copy link
    Mannequin

    masklinn mannequin commented Dec 15, 2020

    The problem seems to be in the user code? As you were told by "Carreau", loggin.disable takes a logging level (an integer), since you're giving it a string which it dutifully stores, it blows up at the next logging call which happens to be in asyncio.

    This is not an asyncio bug, nor is it a crash anymore than passing a broken key function to sorted is a Python crash.

    At most it's an enancement request: logging.disable could raise a TypeError immediately or convert the value to a loglevel the way setLevel does instead of storing the broken value as-is (all it does currently is store the value it receives on the root logger without any check).

    @Carreau
    Copy link
    Mannequin

    Carreau mannequin commented Dec 15, 2020

    I should have been more careful in my explanation in the upstream issue to have a complete report in here.

    I think that patching logging.disable to raise a type error immediately would be welcome; as the effect of storing a wrong type make any asyncio application fails in place that can be relatively remote from the location of the erroneous value, and could be quite hard to debug in other circumstances.

    @masklinn
    Copy link
    Mannequin

    masklinn mannequin commented Dec 15, 2020

    I think that patching logging.disable to raise a type error immediately would be welcome

    FWIW logging has a built-in checker / converter[0] which is already used in a bunch of places (e.g. the aforementioned setLevel), it could just be added inside disable in the same way it's used in setLevel[1] and would uniformly convert "level names" to proper levels or raise an error. That seems like a really easy patch / contribution.

    [0] https://github.com/python/cpython/blob/3.9/Lib/logging/__init__.py#L189

    [1] https://github.com/python/cpython/blob/3.9/Lib/logging/__init__.py#L906

    @masklinn
    Copy link
    Mannequin

    masklinn mannequin commented Dec 15, 2020

    Oh I now see you've created a PR to do essentially that, nm.

    @asvetlov asvetlov added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error and removed topic-asyncio type-crash A hard crash of the interpreter, possibly with a core dump labels Dec 16, 2020
    @asvetlov
    Copy link
    Contributor

    New changeset b32d8b4 by Matthias Bussonnier in branch 'master':
    bpo-42644: Validate values in logging.disable() (bpo-23786)
    b32d8b4

    @miss-islington
    Copy link
    Contributor

    New changeset 0a24a57 by Miss Islington (bot) in branch '3.8':
    bpo-42644: Validate values in logging.disable() (GH-23786)
    0a24a57

    @miss-islington
    Copy link
    Contributor

    New changeset db63da7 by Miss Islington (bot) in branch '3.9':
    bpo-42644: Validate values in logging.disable() (GH-23786)
    db63da7

    @asvetlov asvetlov added 3.8 only security fixes 3.9 only security fixes 3.10 only security fixes and removed 3.7 (EOL) end of life labels Dec 16, 2020
    @asvetlov asvetlov added the 3.8 only security fixes label Dec 16, 2020
    @asvetlov asvetlov added 3.9 only security fixes 3.10 only security fixes and removed 3.7 (EOL) end of life labels Dec 16, 2020
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.8 only security fixes 3.9 only security fixes 3.10 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @asvetlov @miss-islington