-
Notifications
You must be signed in to change notification settings - Fork 771
Release 1.0.0 caused an AttributeError: 'NoneType' object has no attribute 'settimeout' on websocketapp.close() #694
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
Comments
Today we are seeing sporadic failures on our CI system. They seem to stem from the same regression.
|
Can you provide a minimum reproducible example as outlined in the contributing guidelines? I suspected that issues might arise due to this change from commit 13e83b4, but because exceptions were only passed before, I thought this change would help clarify where the exceptions were coming from so that the code could be made more robust in the long run. The unit tests for this project are currently all passing, so I need to better understand what leads to these exceptions. I do agree that short-term issues might be a problem and I will work to address them and build them into unit tests, or possibly revert to the old solution of passing all exceptions. |
The failure @eirikur-grid mentioned originates in a fairly primitive smoketest script, I'll share the code here: import sys
import time
import socketio
def test():
sio = socketio.Client(logger=True, engineio_logger=True)
connected = False
namespace = "/change-notifications"
def connect():
nonlocal connected
connected = True
print("Connected")
def disconnect():
print("Disconnecting")
sio.on("connect", connect, namespace=namespace)
sio.on("disconnect", disconnect, namespace=namespace)
sio.connect(
"ws://localhost:5001", namespaces=[namespace], socketio_path="socket.io5", transports=["websocket"]
)
for tries in range(10):
if not connected:
print(f"Not connected yet, sleeping for 1s")
time.sleep(1)
else:
break
print("About to disconnect")
sio.disconnect()
sio.wait()
print("Done disconnecting")
return connected
if __name__ == "__main__":
if test():
sys.exit(0)
else:
sys.exit(1) The output from this script does suggest that it may not be using the socketio client correctly, at least the disconnection log output from the loggers is not confidence-inspiring:
|
I'm sorry that I do not have a reproducible test which only uses |
I don't have example to reproduce but I confirm that, at least, the commit 13e83b4 lets AttributeError exception raised to WekSocketApp clients whereas it wasn't the ase before (see my initial comment). |
Unfortunately I can't get the sample code from @steinnes to work. I get the error
I'm a bit confused now because the error being triggered in websocket-client is from Thanks for the quick responses. We should be able to resolve this before long. |
Ok I had an old version of socketio installed causing the
@PFaurel if the issue you are encountering is different from what steinnes and eirikur-grid have shared, can you provide a traceback to help with troubleshooting? It sounds like you are encountering a |
We are using the circleci/python:3.9.1 docker image in our CI workflow. |
Here are the exact versions that got pulled in by pip when we encountered the issue
|
@eirikur-grid thanks, I can recreate the issue now using a virtual env, even with the latest python-socketio v5.3.0. Unfortunately it's unclear to me at first glance whether the issue is in websocket-client or elsewhere. I might not have time to dig into this much deeper in the next 24 hours or so, but I will provide updates if I find something. Maybe other websocket-client users can provide more insight if the issue is widespread. @miguelgrinberg since you're the author of python-engineio and python-socketio, do you have insight to whether the issue in this thread might be due to websocket-client or the engineio/socketio code? The v1.0.0 release of websocket-client (released about 12 hours ago) may have impacted socket-io/engine-io functionality, so if your unit tests reveal a problem in websocket-client, I can look into it. |
The stack we have:
I confirm that self.sock is set to None in _recv() function (_core.py file) :
WebSocketConnectionClosedException is always raised when we call websocketapp.close().
|
I just realized this issue might be due to PR #690, which could cause sock to be None. When I patch websocket-client to revert this PR, it looks like it might resolve this issue. Can @PFaurel, @eirikur-grid, or @steinnes provide feedback if reverting PR #690 fixes this issue for you? If we confirm this is the issue, I can release a version 1.0.1 with this fix. |
@engn33r the unit tests in python-engineio and python-socketio do not depend on websocket-client, so they would not reveal any problems if your package has a breaking change. Looking at the error I don't really see how this can be triggered from the socketio side, but let me know if there is anything I can do to help figure this out. |
@miguelgrinberg thank you for your insights, I agree that the issue stems from one of a recent change in websocket-client. If any issues appear in your projects like this, you can direct them here. I hope to have this resolved soon. |
A short update: I was mistaken about PR #690 being a quick fix. @PFaurel was spot on in the first report of this issue - the quickest fix would be to change the My initial finding is that the |
I reverted to the simple I will create release 1.0.1 with this fix. If this problem is not fixed in release 1.0.1, let me know and I can reopen this issue. |
Hi,
With the 1.0.0 release, a major update in WebSocket.close() function in _core.py was done and it causes an AttributeError when we try to close WebSocketApp.
This issue is a side effect of an update in websocket-client/_core.py file
Initially, the code of WebSocket.close() function was:
Now, exceptions are no more catched ... except for Mac OS (!?!) :
I found one reason that could cause self.sock set to None :
when frame is decoded (line 479: frame = self.recv_frame()),
if there is any error during decoding, the web socket is closed and set to None:
I suppose our issue isn't new but was hidden by the try / except pass until 1.0.0 release.
Could you test self.sock before trying to set timeout and call shutdown() ?
The text was updated successfully, but these errors were encountered: