added the reconnect timeout#2026
Conversation
|
@samhed, please check this PR |
samhed
left a comment
samhed
left a comment
There was a problem hiding this comment.
Thanks for your contribution. I didn't test it yet, but a few comments from briefly checking the code.
samhed
left a comment
samhed
left a comment
There was a problem hiding this comment.
I tested this and unfortunately, this breaks the reconnection functionality. No matter what I set reconnect_max_time to, the first time reconnect() is called, we abort the reconnection attempts. This is because UI.firstReconnectTime is never set.
I get the impression that you're not testing your code?
Ankit8969
left a comment
Ankit8969
left a comment
There was a problem hiding this comment.
I have resolved the comments. Please review.
samhed
left a comment
samhed
left a comment
There was a problem hiding this comment.
Thank you, I tested again, and it's getting closer. However, the state you end up in after reaching the threshold is broken.
Please, test yourself as well, so we don't need these long feedback cycles.
| UI.hideStatus(); | ||
| // Showing this message for long time, because if the connection is unstable and user is not around to see the message when the connection is lost, they will be able to see it when they will come back. (Showing for 3 hours) | ||
| UI.showStatus(_("Maximum reconnect attempts reached. Failed to connect to the server."), 'error', 180 * 60 * 1000); | ||
| UI.updateVisualState('disconnected'); |
There was a problem hiding this comment.
This should be UI.cancelReconnect() to avoid ending up in a broken state after reaching the maximum reconnect attempts.
| if ((Date.now() - UI.firstReconnectTime) >= MAX_TIME_IN_MS) { | ||
| // hiding the previous status message | ||
| UI.hideStatus(); | ||
| // Showing this message for long time, because if the connection is unstable and user is not around to see the message when the connection is lost, they will be able to see it when they will come back. (Showing for 3 hours) |
There was a problem hiding this comment.
It looks like the rest of the file avoids long lines like this one.
2 participants
This update introduces a maximum reconnect timeout for the VNC client. Currently, when the VNC client attempts to reconnect, it may continue retrying indefinitely if the connection keeps failing, which is not ideal.
With this change, we add a new configuration key, reconnect_max_time, in default.json. This setting allows us to define a maximum duration for reconnection attempts. Once the specified time limit is reached, the client will stop trying to reconnect, preventing unnecessary retries and improving overall stability.