-
Notifications
You must be signed in to change notification settings - Fork 3
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
changed send_json and its dependent functions to be non-blocking. #15
base: main
Are you sure you want to change the base?
Conversation
now send_json will timeout with incomplete data, fix #14 |
wayfire/ipc.py
Outdated
return response | ||
else: | ||
# The connection is closed due to a timeout | ||
self.connect_client(self.socket_name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this reconnect here seems very weird to me, we're raising the exception, the user should decide what to do
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have two options:
The worse option, which will lead to issues later:
Exception("Response timeout, client connection closed")
In this case, we would remove the line for connecting the client, which is not ideal. Every time a timeout occurs, the client-side code would need to handle socket = WayfireSocket() again.
The issue here is the excessive checking to see if the client is still connected, all due to a send_json timeout.
It's not part of the user side to keep the client connected after socket = WayfireSocket()
The better option:
Set self.reconnect_on_client_timeout = True in init
Exception("Response timeout, reconnecting client")
if self.reconnect_on_client_timeout:
self.connect_client(self.socket_name)
This way, the user can set socket.reconnect_on_client_timeout = False if they don't want the client to automatically reconnect.
it's not strange btw, the message 'Response timeout' indicates that the request didn't happen
This will reproduce the issue which happens randomly, a quick way to get the error is by using fuzzy tests while this code watch for events...
this will return randomly:
|
No description provided.