-
-
Notifications
You must be signed in to change notification settings - Fork 526
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
Websocket Server: Closing handshake failed in v10, v9.1 was ok #1072
Comments
Looks like a bug. Probably websockets should ignore exceptions there. |
Hey all, In case it helps, I got the same issue on FastAPI/Starlette/uvicorn which is using websockets v10.0. Seems to happen if the client kills the connection suddenly. The disconnect is detected (and TCPTransport shows closed=True before returning from the endpoint), but websockets tries to write_eof still Minimal code:
(executed through uvicorn Traceback:
Hope this helps! |
Happens to me as well, when client disconnects suddenly. |
I tried to reproduce this error but I didn't manage. I didn't find any changes in this area between 9.1 and 10.0 either. So I have no idea why it's happening in 10.0 but didn't happen before :-( This change could help. Would someone who hits the issue be able to test if it helps, please? diff --git a/src/websockets/legacy/protocol.py b/src/websockets/legacy/protocol.py
index a31a5c7..9c0819c 100644
--- a/src/websockets/legacy/protocol.py
+++ b/src/websockets/legacy/protocol.py
@@ -1303,7 +1303,7 @@ class WebSocketCommonProtocol(asyncio.Protocol):
self.logger.debug("! timed out waiting for TCP close")
# Half-close the TCP connection if possible (when there's no TLS).
- if self.transport.can_write_eof():
+ if self.transport.can_write_eof() and not self.transport.is_closing():
if self.debug:
self.logger.debug("x half-closing TCP connection")
self.transport.write_eof() |
Is anyone able to test this patch? If you have a way to reproduce the issue, it should take only of few minutes to apply this change and check if the issue goes away 🙏 |
I'll try to test it.
The error indeed happens at line 1308 in protocol.py
1. # Half-close the TCP connection if possible (when there's no TLS).
2. if self.transport.can_write_eof():
3. if self.debug:
4. self.logger.debug("x half-closing TCP connection")
5. self.transport.write_eof()
…On Wed, Nov 3, 2021 at 9:59 PM Aymeric Augustin ***@***.***> wrote:
Is anyone able to test this patch? If you have a way to reproduce the
issue, it should take only of few minutes to apply this change and check if
the issue goes away 🙏
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1072 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAK4BQKKG24TZP64H3WHG23UKGIDDANCNFSM5GD2T3AA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
@aaugustin I can confirm that your patch above fixes the issue. Before
After
Versions
|
In case if someone is interested to compare changes from v9.1 to v10 |
For some more background, this issue occurred for me when upgrading |
Thanks a lot for your help! I'll commit the fix shortly. |
(I'd still love if someone had a way to reproduce on macOS because I swear I tried a dozen combos of clients x ways to break the connection and never reproduced this.) |
I'm not very familiar with the bit of code that I'm looking into right now so not sure how much help I can be. In my example above I triggered an exception:
So it looks like we're seeing the following because the connection is terminated abnormally:
Not sure if that is helpful. I was able to trigger this behaviour easily with the following where the path isn't valid:
|
I also ran into this problem with |
Version 10.1, which includes the fix, is available on PyPI. Thanks everyone for your help! |
aaugustin, Maybe this will help. behavior: websockets client js environment: running in Qt5/Pyside2 with included Chrome browser js: websocket client listens to events 'onopen','onclose','onmessage' and 'onerror'. error output: stack trace: closing handshake failed output: observation: work around: The Pyside2 environment probably affected the issue's timing, but hopefully sheds some light on the overall problem. |
I believe this error has resurfaced in 10.2. 10.1 is working fine, but 10.2 is giving me this error, when a connection is non-gracefully closed:
|
I can confirm that this as resurfaced for me as of 5b3a6d2 in v10.2. That change got rid of the |
I will fix it. Really sorry :-( |
Hi @aaugustin Was wondering if any progress had been made on this one? Would you like me to open a PR reinstating the I also noticed some other changes in the Thanks 🙂 |
I am not clear on how I want to handle this :-( Yesterday's changes are related to the Sans-I/O implementation which isn't involved here. |
OK. Thanks for the update 😢 |
Hello Augustin,
from version 10 occurs an error during closing browser tab connected to my websocket server.
It occurs only in approximately 30% cases.
But everything was ok in version 9.1.
Tested in Google Chrome 94.
Traceback:
Websocket server simplified code:
Can you help me, please?
Thanks a lot,
Jaromir
The text was updated successfully, but these errors were encountered: