-
-
Notifications
You must be signed in to change notification settings - Fork 605
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
Confusing desync with blocking calls #3143
Comments
Hi @FeldrinH, That's an interesting observation. If I understand correctly, "desync" basically means that the UI is not updated after So it boils down to something like this: def test():
time.sleep(5)
button.text += '!'
button = ui.button('Test', on_click=test) With a delay of 4 seconds the UI is correctly updated, but not with 5 seconds. I guess what is happening is that the socket connection disconnects while the server is sleeping, but afterwards the server automatically reconnects. Until the reconnect is done, updates get lost. We'll need to investigate this more thoroughly to see if we can do anything about it. |
Here is another simple example to reproduce lost messages: @ui.page('/', reconnect_timeout=10.0)
def page():
log = ui.log()
ui.timer(1.0, lambda: log.push(f'{time.time():.0f}')) When disabling the network connection for a few seconds, there will be a gap in the logged time stamps. |
Did you want to look into this one yourself @falkoschindler? If not, I'd be happy to take it on. Using your example, I already discovered that Let me know if I should keep going or not. |
@afullerx Feel free to take it on. Questioning Lines 122 to 123 in 2e00244
To ensure every single message is received by the client, I was thinking about adding some UID to each socket message as well as another outbox buffer collecting all sent messages per client (or |
@falkoschindler I think I spoke too soon on this one. While fixing I completely agree that in the absence of perfect knowledge about the connection state, a system of message accounting is the only way to completely ensure delivery. I think it's still worth submitting a pull request to improve I was thinking it should be changed to a simple attribute that would be toggled on and off to reflect the connection state as it's known to |
Hmmm, isn't the connection between NiceGUI and browser based on the websocket which is based on TCP? How could messages be lost in that case? 🤔 |
@me21 The message retransmission described above by @falkoschindler has the same purpose. Acknowledging every message would probably be excessive overhead since most of the time, the connection is fine. |
@me21 You have me thinking though. If there's a chance that a message could fail to be delivered, but the connection could recover without a subsequent reconnection. Then acknowledging every message would be necessary. Hopefully, Socket.IO wouldn't allow this. Edit: Actually, this would still be OK because the client could detect a gap in the message ID's it's receiving and request retransmission of any missed ones or simply perform a reconnection to synchronize. |
For Socket.IO this part of the official documentation might be helpful: https://socket.io/docs/v4/tutorial/handling-disconnections. |
"Connection state recovery" looks interesting: I'm surprised to see this because on another page of the documentation, it says if you want this kind of functionality, you have to implement it yourself. I'll have to take a close look at this later. |
Unfortunately, "Connection state recovery" doesn't appear to be implemented for the Python version: miguelgrinberg/python-socketio#1258 |
I think it's impossible to lose a message without the need to reconnect. The underlying TCP protocol implements a stream of bytes meaning it guarantees all data to be received in the same order it was sent. |
@me21 That's true, but since we're relying on the socket.io library to handle the underlying TCP connection, we're at the mercy of its behavior. The documentation does say the sequence of messages is guaranteed though, just not the delivery. So, I think you're right that it's a non-issue. |
How long should the message history be retained? The longest we can have a non-functional connection before socket.io detects it is sio.eio.ping_interval = max(app.config.reconnect_timeout * 0.8, 4)
sio.eio.ping_timeout = max(app.config.reconnect_timeout * 0.4, 2) ui.pageFor the case of dead_connection_max = sio.eio.ping_interval + sio.eio.ping_timeout
history_duration = dead_connection_max + page.reconnect_timeout For the default case of Auto-index pageIn the case of the auto-index page, the client can reconnect after any amount of time. This leaves us two choices:
Number 2 makes more sense both from a resource standpoint, and from the user standpoint. You wouldn't want a whole bunch of stale notifications popping up after an extended disconnection, for example. You would just want the current UI state. |
I've got the message history/retransmission pretty much done. I hugely overestimated how much time it would take. I'm going to blame it on how easy it is to work in this codebase. 😉 |
Description
Doing blocking calls in some places (e.g. in event handlers) can sometimes cause silent desync between the browser and Python.
For example, take this code:
Here is an example of me playing around with this code:
NiceGUI.-.Google.Chrome.2024-05-30.02-34-09.mp4
Note in particular two things:
Now obviously you're not supposed to use long-running blocking calls in event handlers, because it blocks the async event loop. However, the problem is that if you accidentally introduce a blocking call then it can lead to silent desync which is very confusing and hard to debug (speaking from experience).
In my opinion, there are at least two issues here:
The text was updated successfully, but these errors were encountered: