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
Unintuitive error handling in wsgiref when a crash happens in write() or close() #73369
Comments
TLDR: When an error happens in the wsgiref's write() or close(), stack traces get inundated with irrelevant yet legitimate errors which make it hard to track down the real issue. Couple of examples of this happening in practice: https://stackoverflow.com/questions/27518844/error-when-serving-html-with-wsgi ---- How to reproduce: The file I've attached reproduces the error on python 3.4, 3.5 and 3.6. The handler returns a string instead of bytes, which fails an early assert in handlers.py: write(data). BaseHandler.run() triggers, gets as far as finish_response(), which triggers the above AssertionError. It falls into the except: block which attempts to handle_error(). But before doing that, it triggers self.close(), which sets result/headers/status/environ to None, bytes_sent to 0 and headers_sent to False. Now when handle_error() triggers, I probably skipped some steps because the traceback is truly a mess. I think this could be improved, if only so that it doesn't get so confusing anymore. |
Looks like the error handling is broken by bpo-16220, which calls the “BaseHandler.close” method before the exception is caught for the error handler. Perhaps it is better to just close the iterator without messing with the other attributes in the exception case. I tried various cases in Python 2.6 (without the bpo-16220 change) and the error handling seems better (not masked by double exceptions, no sending a 500 response after the start of the app’s response). The same problem exists in Python 2, except only the later exception is reported, and the original exception is forgotten. |
This may or may not be the same as what you're suggesting, Martin. But is another option to make close() a no-op if it is called a second time? Otherwise, it seems we'd need to make sure that no code path can result in close() being called twice (even during exception handling, etc). |
There are actually two “close” methods in the WSGI package: ServerHandler’s implementation extends the BaseHandler implementation. Making the “close” methods do nothing if called a second time would avoid the error about “self.status” being None, but won’t help very much with other problems, such as:
To be clear, what I had in mind last week was to adjust “finish_response” to look something like: class BaseHandler:
...
def finish_response(self):
try:
...
except:
# Limited version of “close” method on exception
if hasattr(self.result, 'close'):
self.result.close()
raise
self.close() # Full “close” method when no exception For the record: This is related to bpo-27682, specifically about handling ConnectionAbortedError and other network errors. |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: