Skip to content

bpo-27682: Handle client connection terminations in wsgiref.#9713

Merged
berkerpeksag merged 4 commits into
python:masterfrom
PetterS:bpo-34547
May 1, 2019
Merged

bpo-27682: Handle client connection terminations in wsgiref.#9713
berkerpeksag merged 4 commits into
python:masterfrom
PetterS:bpo-34547

Conversation

@PetterS

@PetterS PetterS commented Oct 5, 2018

Copy link
Copy Markdown
Contributor

Previously, long stack traces with Python TypeErrors and AttributeErrors
were printed (see bug).

The wsgiref server is used quite a lot in the community, notably by the
Django development server. I see these huge stack traces daily.

https://bugs.python.org/issue27682

@PetterS

PetterS commented Oct 6, 2018

Copy link
Copy Markdown
Contributor Author

I have signed the CLA before, but the bot has not updated this PR either way.
Also, the Azure failing CI seems to be incorrect (other PRs were also failing when I submitted this.)

@PetterS

PetterS commented Oct 11, 2018

Copy link
Copy Markdown
Contributor Author

@berkerpeksag You seem to have touched wsgiref last. I know it hasn't been developed a lot the last years, but it is still used quite a lot. Do you think you could have a look at this?

@deeredman1991

Copy link
Copy Markdown

I was able to fix the issue on my end by modifying Python36/Lib/socketserver.py

I modified the write method (at around line 800) of the _SocketWriter() class to the following;

    def write(self, b):
        try: #Not Standard Python
            self._sock.sendall(b)
        except ConnectionAbortedError: #Not Standard Python
            self._sock.close() #Not Standard Python
        with memoryview(b) as view:
            return view.nbytes

I am not sure if this is going to cause other issues or not but; it stopped the error messages/stacktrace

I wanted to let you guys know about it in case it could help...

I am not too familiar with the python source code... If you guys foresee any issues I might have with this modified code off the top of your head; could you give me a heads up?

@berkerpeksag
berkerpeksag self-requested a review October 12, 2018 18:53
@zooba

zooba commented Oct 13, 2018

Copy link
Copy Markdown
Member

I'm keen to see this fixed - it's been an annoyance for quite a while :)

Should we log a message on the connection abort? This is meant to be a reference server, so logging something shouldn't hurt, and I'd hate to be the person who has to diagnose why their aborted connections are being silently handled (or something one step removed that would be obvious if they knew the connections were aborting).

I also wonder if there's a more narrow place we can handle the exception? (It's been a while since I looked closely at the stack traces and don't have any handy now, but if there's one place where it always occurs rather than "anywhere in the WSGI app", it'd be better to handle it there.)

@zooba

zooba commented Oct 13, 2018

Copy link
Copy Markdown
Member

Also, you can ignore the Azure Pipelines failure here. It should work on re-run (and possibly with a merge from master) - there was a problem with the configuration that has since been fixed.

@berkerpeksag berkerpeksag left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that catching this exception in a more specific place would be better. I think SimpleHandler._write() might be a good place to do this. Also, we can reuse the existing warn() to log something to users:

    def _write(self,data):
        from warnings import warn
        try:
            result = self.stdout.write(data)
        except ConnectionAbortedError:
            warn(...)
            return
        else:
            if result is None or result == len(data):
                return
        ...

I'm not sure about catching ConnectionAbortedError in socketserver. I think it's the client's responsibility to catch such an exception, but it might be a good idea to check how other server implementations handle this case.

@berkerpeksag
berkerpeksag requested a review from vadmium October 13, 2018 18:24
@PetterS

PetterS commented Oct 13, 2018

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback!

I am hestitating a little about warning for a ConnectionAbortedError since that is a pretty normal thing for a web server, but its is a large improvement over 100+ lines of stack traces, so I guess we could go with that.

PR updated. It works nicely with Django’s test server, which is my main concern.

@deeredman1991

deeredman1991 commented Oct 13, 2018

Copy link
Copy Markdown

@PetterS What about a flag somewhere like "os.environ['DISPLAY_CONNECTION_ABORTED_WARING'] = True"? Idk if os.environ would be the right place to put it or not but it's an idea...

or instead of making it a "warning" just treat it as a notification.

@PetterS

PetterS commented Oct 13, 2018

Copy link
Copy Markdown
Contributor Author

I don't think we want to introduce a environment variable for this. I am happy with the current change – it writes two lines to the Django test server log when the browser closes the connection. Further, Python already has a system for disabling specific warnings.

@PetterS

PetterS commented Oct 19, 2018

Copy link
Copy Markdown
Contributor Author

@vadmium Did you have a chance to look at this? Thanks in advance!

@vadmium

vadmium commented Oct 19, 2018

Copy link
Copy Markdown
Member

Hi, I haven't had a chance to have a good look at the code yet. Maybe the weekend after this one. I agree that exceptions like connection abort, that are caused by the remote client or network, should be handled more gracefully and quietly.

@vadmium

vadmium commented Nov 4, 2018

Copy link
Copy Markdown
Member

Using the warnings module rather than the WSGI stderr logging functionality seems a bit surprising. I agree that it is nice that you can enable and disable warnings and convert them to exceptions.

If you turn on multiple warnings (python -Walways), I expect you might see the same warning repeated as each write call is attempted. If the application is sending a large response, or doing significant computation, it might be nice to abort sooner rather than letting it go through the response.

I think the earlier version that caught the exception higher up (revision 9dc6eb5) is a better behaviour. But in these cases I normally silence all exceptions derived from ConnectionError. On Linux clients and servers, I have never seen ECONNABORTED, but tend to see EPIPE and ECONNRESET instead.

Assuming the more general problem with the double exceptions gets fixed (see https://bugs.python.org/issue29183), then the exception log should revert back to something like the first 16 lines from Petter’s post in https://bugs.python.org/issue34547. If people prefer something is logged, but think the 16-line backtrace is still too much, perhaps just log the exception message in a single line to the WSGI’s stderr?

@PetterS

PetterS commented Nov 4, 2018

Copy link
Copy Markdown
Contributor Author

Personally, I don't feel that this is worth a warning, either. I can revert back to 9dc6eb5 if that can be the consensus.

@PetterS PetterS changed the title bpo-34547: Handle client connection terminations in wsgiref. bpo-27682: Handle client connection terminations in wsgiref. Nov 4, 2018
@PetterS

PetterS commented Nov 4, 2018

Copy link
Copy Markdown
Contributor Author

The bug I created was closed as a duplicate, so I repointed this PR to the other issue.

birthdaysgift added a commit to birthdaysgift/mimic that referenced this pull request Nov 19, 2018
The problem that I intented to describe in this TODO is a bug of python
standart library (wsgi).
More info about it can be found here:
https://bugs.python.org/issue27682
https://bugs.python.org/issue29183
python/cpython#9713
@PetterS

PetterS commented Nov 20, 2018

Copy link
Copy Markdown
Contributor Author

@vadmium What do you think is the way forward here? Reverting to 9dc6eb5?

@vadmium

vadmium commented Nov 26, 2018

Copy link
Copy Markdown
Member

I think it is more important to fix the double exceptions first, like I suggested at https://bugs.python.org/issue29183#msg329223. That would bring the behaviour back to how it used to be when wsgiref was added to Python.

If people still want to change how ConnectionAbortedError (ECONNABORTED) is logged, going back to revision 9dc6eb5 is a good starting point. But I would encourage you to include similar exceptions such as EPIPE and ECONNRESET.

And am in two (or three) minds about whether it is better to hide these conditions completely, log a backtrace showing where the server discovered the broken connection, or perhaps log a short message on a single line. Sometimes the verbose logs get in the way of more interesting log messages; other times they are useful for diagnosing your code. This tradeoff is similar to other servers based on socketserver (for example “python -m http.server”) that log the full exception backtrace, and more generally the default handling of BrokenPipeError and KeyboardInterrupt in the Python interpreter.

@aeltawela

Copy link
Copy Markdown

this issue is running since 2016, any plan to fix it in the next releases?

@PetterS

PetterS commented Dec 16, 2018

Copy link
Copy Markdown
Contributor Author

this issue is running since 2016, any plan to fix it in the next releases?

Yes, I still have a plan to try and get this submitted. I got feedback for my initial version, changed it, but the latest feedback from vadmium suggests that I should go back to my original version 9dc6eb5 and take it from there.

Previously, long stack traces with Python TypeErrors and AttributeErrors
were printed (see bug).

The wsgiref server is used quite a lot in the community, notably by the
Django development server. I see these huge stack traces daily.
@PetterS

PetterS commented Jan 6, 2019

Copy link
Copy Markdown
Contributor Author

I have now reverted to the original state of this PR. It does not address everything (double exceptions in general, EPIPE, ECONNRESET ,…). Still, I think this would be good to merge. It is an annoyance for Django developers on Windows.

@aeltawela

Copy link
Copy Markdown

It is cool that the changes are ready, but may I know in which release will be these changes included?

@PetterS

PetterS commented Feb 20, 2019

Copy link
Copy Markdown
Contributor Author

@ahmedabt A maintainer will have to look at this and merge it first. Like us, they are also volunteers, so there is no timeline.

@vadmium Since this has been going on for six months and is pretty useful as is, any change that we can merge this in order for it to make 3.8 and postpone additional changes until later?

@SakisHous

Copy link
Copy Markdown

I encounter the same problem with Python 3.7.3 and Django 2.2.1 in Windows 10. Should I wait for Python 3.8?

@PetterS

PetterS commented May 15, 2019

Copy link
Copy Markdown
Contributor Author

I encounter the same problem with Python 3.7.3 and Django 2.2.1 in Windows 10. Should I wait for Python 3.8?

This will be backported to the most recent version of 3.7.

@tirkarthi

Copy link
Copy Markdown
Member

It has been backported and will be part of 3.7.4.

@berkerpeksag

Copy link
Copy Markdown
Member

3.7.3 was released on March 25. This fix will be released with 3.7.4.

@changeling

Copy link
Copy Markdown

Really looking forward to June 24. @berkerpeksag and @PetterS, thank y'all again for cleaning this up. It's been a long running thorn in folks' sides.

For anyone not familiar with the release schedule, here's the relevant link including 3.7.4 projected release date:

PEP 537 -- Python 3.7 Release Schedule

@afvca

afvca commented Jul 9, 2019

Copy link
Copy Markdown

I believe this is still not fixed...Upgraded to Python 3.7.4 but still getting these warnings when running the development server in Windows 10.

@wkoot

wkoot commented Jul 10, 2019

Copy link
Copy Markdown

Works well for me on 3.7.4 with Windows 10, running from subshell in PyCharm/IntelliJ IDEA.

@afvca

afvca commented Jul 10, 2019

Copy link
Copy Markdown

Well, turns out I was testing with Mozilla Firefox and was getting these errors.

Socket error: [WinError 10053] An established connection was aborted by the software in your host machine

Tested with Google Chrome and it is working fine. Might be a Firefox bug..

@KANE-99

KANE-99 commented Jul 21, 2019

Copy link
Copy Markdown

Well, turns out I was testing with Mozilla Firefox and was getting these errors.

Socket error: [WinError 10053] An established connection was aborted by the software in your host machine

Tested with Google Chrome and it is working fine. Might be a Firefox bug..

I Encountered exactly opposite, Works fine without any issue with Firefox while got this ConnectionAbortedError error on Chrome.

@PetterS

PetterS commented Jul 22, 2019

Copy link
Copy Markdown
Contributor Author

This is the discussion of a merged pull request. Could we raise these additional issues in the bug please?

I will comment there. The remaining log message probably make sense to fix in Django.

@boshng95

boshng95 commented Aug 4, 2019

Copy link
Copy Markdown

The upgrade from Python 3.7.3 to 3.7.4 remove the error message of AttributeError & TypeError. However, there is still one error message unable to resolve.

For my case, I had upgraded my Python to 3.7.4 & Django 2.2.4 with Windows 10 & run the server using Firefox Developer Edition. Still having an error as shown below:

ConnectionAbortedError: [WinError 10053] An established connection was aborted by the software in your host machine

@thuanvn

thuanvn commented Aug 19, 2019

Copy link
Copy Markdown

I had same situation with boshng95. just ignore & go ahead for now

@JoeKhoa

JoeKhoa commented Jan 8, 2020

Copy link
Copy Markdown

same error in Python.3.8 and Django2.2.5
Exception happened during processing of request from ('127.0.0.1', 53745)
Traceback (most recent call last):
File "C:\Users\Lilti\Miniconda3\envs\mydjangoenv\lib\socketserver.py", line 650, in process_request_thread
self.finish_request(request, client_address)
File "C:\Users\Lilti\Miniconda3\envs\mydjangoenv\lib\socketserver.py", line 360, in finish_request
self.RequestHandlerClass(request, client_address, self)
File "C:\Users\Lilti\Miniconda3\envs\mydjangoenv\lib\socketserver.py", line 720, in init
self.handle()
File "C:\Users\Lilti\Miniconda3\envs\mydjangoenv\lib\site-packages\django\core\servers\basehttp.py", line 171, in handle
self.handle_one_request()
File "C:\Users\Lilti\Miniconda3\envs\mydjangoenv\lib\site-packages\django\core\servers\basehttp.py", line 179, in handle_one_request
self.raw_requestline = self.rfile.readline(65537)
File "C:\Users\Lilti\Miniconda3\envs\mydjangoenv\lib\socket.py", line 669, in readinto
return self._sock.recv_into(b)
ConnectionAbortedError: [WinError 10053] An established connection was aborted by the software in your host machine

@hackzaid

Copy link
Copy Markdown

Any update on this issue, running python 3.8.4 but same error;
ConnectionAbortedError: [WinError 10053] An established connection was aborted by the software in your host machine

Tested my Django app on chrome works fine, firefox not working

@khorammfar

Copy link
Copy Markdown

i have same error in python 3.8.6rc1 Django 3.1 !!! window and linux
how you guys fix this ?

@PetterS

PetterS commented Nov 25, 2020

Copy link
Copy Markdown
Contributor Author

Alright, I don't use Windows alot any more, but perhaps I can find the time to look into this again.

@stonetalons

Copy link
Copy Markdown

This issue do annoy me A LOT! I have tried the following solution in socketserver.py :
def write(self, b):
try: #Not Standard Python
self._sock.sendall(b)
except ConnectionAbortedError: #Not Standard Python
self._sock.close() #Not Standard Python
with memoryview(b) as view:
return view.nbytes
while it doesn't work.(My environment: Python:3.6.4,django 3.1.2)
So...I rewirite the readinto function in socket.py

        except error as e:
            if e.args[0] in _blocking_errnos:
                return None
            if e.args[0]==10053:
                return None
            raise

OK my terminal won‘t report 10053 error anymore.#EMBARRASSED FACE
(STILL LOOKING FOR A REAL SOLUTION!)

@PetterS

PetterS commented Dec 3, 2020

Copy link
Copy Markdown
Contributor Author

This PR changed wsgiref/handlers.py but the new stack trace above does not mention this file. Has something in Django changed so that it does not use wsgiref any more? Compare to the stack trace in https://bugs.python.org/issue27682

@PetterS

PetterS commented Dec 3, 2020

Copy link
Copy Markdown
Contributor Author

But I think this error can be fixed by adding ConnectionAbortedError here in Django: https://github.com/django/django/blob/c6581a40be3bb4c1e13861f0adbb3fe01f09107f/django/core/servers/basehttp.py#L55

@PetterS

PetterS commented Dec 3, 2020

Copy link
Copy Markdown
Contributor Author

FYI I created django/django#13743

felixxm pushed a commit to PetterS/django that referenced this pull request Dec 14, 2020
@PetterS

PetterS commented Dec 14, 2020

Copy link
Copy Markdown
Contributor Author

For people seeing this in Django: it should be better in the next version.

@krzysiaczeks

krzysiaczeks commented Feb 19, 2021

Copy link
Copy Markdown

I,ve got same error in Windows 8.1, Python 3.9.1, Django 3.1.7. No matter Firefox or Chrome :/

    [19/Feb/2021 11:11:54] "GET / HTTP/1.1" 200 2222
    Not Found: /favicon.ico
    ----------------------------------------
    Exception occurred during processing of request from ('127.0.0.1', 50643)
    Traceback (most recent call last):
      File "C:\Users\Krzyś\AppData\Local\Programs\Python\Python39\lib\socketserver.py", line 650, in process_request_thread
        self.finish_request(request, client_address)
      File "C:\Users\Krzyś\AppData\Local\Programs\Python\Python39\lib\socketserver.py", line 360, in finish_request
        self.RequestHandlerClass(request, client_address, self)
      File "C:\Users\Krzyś\AppData\Local\Programs\Python\Python39\lib\socketserver.py", line 720, in __init__
        self.handle()
      File "C:\Users\Krzyś\Documents\Django [szkolenia]\djangostocks\venv\lib\site-packages\django\core\servers\basehttp.py", line 174, in handle
        self.handle_one_request()
      File "C:\Users\Krzyś\Documents\Django [szkolenia]\djangostocks\venv\lib\site-packages\django\core\servers\basehttp.py", line 182, in handle_one_request
        self.raw_requestline = self.rfile.readline(65537)
      File "C:\Users\Krzyś\AppData\Local\Programs\Python\Python39\lib\socket.py", line 704, in readinto
        return self._sock.recv_into(b)
    ConnectionAbortedError: [WinError 10053] Nawiązane połączenie zostało przerwane przez oprogramowanie zainstalowane w komputerze-hoście
    ----------------------------------------

@Charlink3

Copy link
Copy Markdown

It has been backported and will be part of 3.7.4.

Hello, I have the same problem And I have the same version 3.7.4, this issue happened a week ago, Do you solve the problem, can you help me?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.