Cleaning up broken pipe errors in runserver

[Matthew Somerville]

Hi,

I have created a branch at https://github.com/dracos/django/compare/pipe-cleaning that builds upon a previous patch posted to this list and outputs "Broken pipe" instead of a traceback for such an error. As the history below shows, practically speaking all reports of broken pipe tracebacks in the log are due to the browser cutting off output, and are not helpful to be shown as a full traceback. I get them frequently (e.g. hit refresh before your previous page has finished loading to sometimes get it), and find them annoying.

I am posting here because ticket #4444 - https://code.djangoproject.com/ticket/4444 - is marked wontfix. I am not "imagei" on that ticket, I just came across the ticket recently whilst trying to work out why my Selenium tests on Travis were failing with broken pipes when they were fine last week - https://github.com/travis-ci/travis-ci/issues/2610 (some change to underlying Travis, I assume). Ticket #4444 was opened in May 2007, and marked wontfix in September 2007 saying "The best we could is to have a more explicit error message." In 2008 someone provided a patch to make it a logged error message rather than spew a scary traceback, there was a +1 and a "leave as-is", then silence. There was a brief reopening of the thread in 2012 by two people who also found the broken pipe tracebacks tedious, then nothing further since then. The thread (2008 and 2012) can be found at https://groups.google.com/forum/#!topic/django-developers/W1Nns9k40EQ

(The 'python manage.py runserver --help' is a little bit confusing because it has a --traceback argument and yet you still get a traceback - you have to have read the default options section of the django-admin.py documentation to know that only makes a difference to CommandError, not any other type of error. My branch also adds "CommandError" to the help output of --traceback.)

The 2008 patch doesn't really work any more, but I think I've made the spirit of the same thing on the current Django code. I hope this can be considered for inclusion, as I think it tidies up a common issue with runserver output that will especially confuse people new to Django. Do let me know if I've gone about fixing it in the wrong way, or if I should do something differently. In my testing it certainly makes my runserver logs nicer to follow :)

ATB,

Matthew

[Richard Eames]

I'm +1 for this, for the same reasons; I have a monkey patch for my selenium tests which does the same thing as this PR.

[Tim Graham]

I had a look at the patch.  As I mentioned on the ticket, "I am not really happy with that patch which copies the simple_server.WSGIRequestHandler.handle() method from Python's version in order to override it. The copied version is not in sync with the latest Python and I'd prefer not to be in a position where we'd have to copy changes from there to Django."

Matthew noted, "the change [in WSGIRequestHandler.handle()], made only last month (hence why this PR doesn't currently match it), is the only change to that function since it became part of the standard library in 2006, and is a small potential security issue that doesn't apply to a development server in any case, as that's explicitly listed as being insecure. I don't necessarily think you'd have to keep changes in sync from upstream, as I don't see there would be any changes to this function other than other tiny security issues that don't matter to a development server, but I can see you wouldn't want to be in that potential situation anyway."

I agree with that, however, many people do use runserver in production and several members of the core team feel we should fix security issues with runserver even though the docs say it's not hardened for production use (I'm not one of them). Any more opinions on how to proceed here?

[Steve Jalim]

Naive / over-obvious suggestion: if there's a genuine stalemate, bundling the changes into a third-party app that supplants core runserver (similar to how django-devserver does it) would avoid the need for individuals to monkey-patch while also making it possible to release versions with more significant security fixes based on changes to the underlying Python -- without tying it to core Django at all. It also puts the decision on the developer re whether or not to care about the "fixed" runserver being slightly less secure.

In a way, it's punting the issue, but I'm all for anything that "fixes" the broken pipe handling: if the fixed runserver existed as a third party app today, I'd install it. And better than the issue tanking for another few years.

If if then gets enough traction, that opens a case for it core re-consideration, potentially. Yup? 

S 

[Tim Graham]

An opinion from Emil Stenström posted on the pull request (I'm inclined to agree with it unless there are objections or if someone can propose another solution):

I agree that we should use the latest version of handle() from CPython to make sure we are as protected as we can be, even when using runserver in production. I see this as a one-time copy from the python standard library. There was one change from 2006 - 2014, so we could expect another change somewhere around 2022.

The trade-off here: avoiding confusion for Django beginners (I've got questions about this from at least three different people), vs. copying 14 lines from a relatively stable standard library method, is worth it.

So if we update the the latest CPython version of handle() I'm +1.