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

When WSGIRequestHandler catches socket.error it silences all other OSErrors in Python 3 #1127

Closed
davidism opened this issue May 29, 2017 · 11 comments · Fixed by #1418
Closed
Labels
Milestone

Comments

@davidism
Copy link
Member

davidism commented May 29, 2017

Reported at pallets/flask#1995. WSGIRequestHander.run_wsgi catches socket.error to silence client disconnects. In Python 2 this was fine, because socket.error was unique. In Python 3, socket.error is OSError, so any OSError or subclass (IOError, etc.) is also silenced.

Possible solution is to check if e.errno is a disconnect-related error and bubble the error up otherwise. Or catch a more specific error such as ConnectionError in Python 3.

This only affects Werkzeug's dev server, other WSGI servers like Gunicorn do not exhibit this behavior.

@mitsuhiko
Copy link
Contributor

I feel like we had this before. I thought we fixed this somewhere already.

@davidism
Copy link
Member Author

It was reported against Flask and we discussed it there, but it's a bug against Werkzeug so I moved it here. Maybe that's what you're remembering? pallets/flask#1995

@aaossa
Copy link
Contributor

aaossa commented Aug 30, 2017

Can I work on this? If so, any suggestion/advice to begin with that consideration?

@untitaker
Copy link
Contributor

@aaossa I'm not sure what you mean with "that consideration". One possible approach to this issue is described in the OP

@aaossa
Copy link
Contributor

aaossa commented Aug 31, 2017

With "that consideration" I meant the suggestion/advice I was asking for. I'll give it a try.

@aaossa
Copy link
Contributor

aaossa commented Oct 10, 2017

I have a possible solution here, should I create a test? @untitaker

@davidism
Copy link
Member Author

davidism commented Oct 10, 2017

I haven't looked at this in a while, but type(e) is socket.error is the same issue we're talking about here: it's not a distinct error on py3. This looks like it just moved the check around, it didn't change the logic.

@aaossa
Copy link
Contributor

aaossa commented Oct 10, 2017

I though that the problem was using except (socket.error ... because it uses isinstance instead of type and that caused the problems in error handling, because isinstance check subclasses and type does not.

EDIT: I tried the example from the original issue and worked without problems, but I haven't tested with a "pure" socket.error.

@davidism
Copy link
Member Author

isinstance is the same as type is in this case, because socket.error = OSError.

@aaossa
Copy link
Contributor

aaossa commented Oct 10, 2017

Then maybe use something like:

# (Not valid syntax)
except socket.timeout, _:
    # ...
except socket.error, exc:
    print("Caught exception socket.error : " + str(exc))

and check the error code (using errno) like this should work? I'm trying to find the errno for socket.error

@davidism
Copy link
Member Author

davidism commented Oct 10, 2017

That's what I said at the top. Also, you're using really old syntax for catching errors there.

@davidism davidism added this to the 0.15 milestone Dec 9, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants