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

HTTPServer server.py assumes sys.stderr != None #72481

Closed
grismar mannequin opened this issue Sep 28, 2016 · 11 comments
Closed

HTTPServer server.py assumes sys.stderr != None #72481

grismar mannequin opened this issue Sep 28, 2016 · 11 comments
Labels
easy stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@grismar
Copy link
Mannequin

grismar mannequin commented Sep 28, 2016

BPO 28294
Nosy @gvanrossum, @orsenthil, @bitdancer, @Mariatta, @Grismar
Files
  • issue28294.patch
  • 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 2016-10-02.00:10:14.401>
    created_at = <Date 2016-09-28.07:08:24.924>
    labels = ['easy', 'type-bug', 'library']
    title = 'HTTPServer server.py assumes sys.stderr != None'
    updated_at = <Date 2016-10-02.12:19:57.853>
    user = 'https://github.com/grismar'

    bugs.python.org fields:

    activity = <Date 2016-10-02.12:19:57.853>
    actor = 'grismar'
    assignee = 'none'
    closed = True
    closed_date = <Date 2016-10-02.00:10:14.401>
    closer = 'r.david.murray'
    components = ['Library (Lib)']
    creation = <Date 2016-09-28.07:08:24.924>
    creator = 'grismar'
    dependencies = []
    files = ['44890']
    hgrepos = []
    issue_num = 28294
    keywords = ['patch', 'easy', 'needs review']
    message_count = 11.0
    messages = ['277593', '277617', '277686', '277705', '277739', '277782', '277813', '277822', '277823', '277834', '277888']
    nosy_count = 5.0
    nosy_names = ['gvanrossum', 'orsenthil', 'r.david.murray', 'Mariatta', 'grismar']
    pr_nums = []
    priority = 'normal'
    resolution = 'wont fix'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue28294'
    versions = ['Python 3.5']

    @grismar
    Copy link
    Mannequin Author

    grismar mannequin commented Sep 28, 2016

    On line 556 of server.py, the sys.stderr.write assumes that sys.stderr is assigned and not None.

    However, when developing Windows services using the pypiwin32 library (as an example), sys.stderr == None

    A workaround is to override the log_message method of the BaseHTTPRequestHandler, but it seems to me that the method should not assume the availability of stderr?

    @grismar grismar mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Sep 28, 2016
    @bitdancer
    Copy link
    Member

    That would be masking an error, I think. What might be reasonable would be to convert it to use the logging module in 3.7.

    @orsenthil
    Copy link
    Member

    Using logging, instead of sys.stderr would be a welcome change in this module.

    @orsenthil orsenthil added the easy label Sep 29, 2016
    @Mariatta
    Copy link
    Member

    I'm interested to work on a patch for this, if no one started yet.

    @Mariatta
    Copy link
    Member

    use the logging module instead of sys.stderr

    @gvanrossum
    Copy link
    Member

    I wonder if we shouldn't close this as "won't fix" and tell the OP to override the log_message() method. It's kind of curious to complain about *this* dependency on sys.stderr existing and working, since there are probably 1000s of other places in the stdlib that also depend on that.

    @bitdancer
    Copy link
    Member

    That would be my answer if we aren't switching to logging. I would consider that an enhancement, but are there reasons to not do that?

    @grismar
    Copy link
    Mannequin Author

    grismar mannequin commented Oct 1, 2016

    Closing and not fixing is fair enough - I did not realize that this would be an issue that occurs in many places in stdlib.

    I realize this is not a help forum, so I will ask elsewhere to see if there's some way to redirect all of sys.stderr in scenarios like these (running a service), because tracking down an issue like this takes a lot of time and finding the issue buried in a standard library caught me off guard.

    @gvanrossum
    Copy link
    Member

    A problem with switching to logging is the API design.

    There are three functions, log_request(), log_error() and
    log_message(). The former two call the latter. The API explicitly
    encourages overriding log_message() to customize logging. But it has
    no way to know whether it's called from log_error(), log_request(), or
    directly.

    So how is it to choose logging levels? I imagine log_request() should
    log at the INFO level, log_error() at the ERROR level, and let's say
    that direct calls to log_message() should also log at the INFO level
    (from looking at the few calls). So how should log_error() distinguish
    itself to log_message() without breaking apps that override the
    latter?

    @bitdancer
    Copy link
    Member

    OK, lets close this won't fix, then. Especially since aiohttp does use logging already....

    Sorry, Mariatta. Thanks for the patch, but we aren't going to use it.

    @grismar
    Copy link
    Mannequin Author

    grismar mannequin commented Oct 2, 2016

    Breaking the API isn't good, but it will only break if log_message doesn't *receive* all messages, because that's what people who override it count on.

    If there's some way of detecting who called log_message, you could use the appropriate log level on logging without compromising the API. But the only way I see without changing the signature of log_message is through inspect functions like getouterframes and that seems way nastier than these functions merit...

    @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
    Labels
    easy stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants