Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions app/ui.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ const UI = {
inhibitReconnect: true,
reconnectCallback: null,
reconnectPassword: null,
firstReconnectTime: 0,

async start(options={}) {
UI.customSettings = options.settings || {};
Expand Down Expand Up @@ -961,6 +962,9 @@ const UI = {
UI.closePowerPanel();
}
},
resetFirstReconnection(){
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing space before {

UI.firstReconnectTime = 0;
},

/* ------^-------
* /POWER
Expand Down Expand Up @@ -1121,6 +1125,23 @@ const UI = {

reconnect() {
UI.reconnectCallback = null;
const maxTime = UI.getSetting('reconnect_max_time') ?? 600000; // default to 10 minutes => 10 * 60 * 1000 ms
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this should be configured to seconds instead of milliseconds?

I.e:

const maxTime = UI.getSetting('reconnect_max_time') ?? 600; // default to 10 minutes => 10 * 60 seconds

...

// Check if we've exceeded the max reconnect time
if ((Date.now() - UI.firstReconnectTime) >= maxTime * 1000) {


// Initialize first reconnect time if it's the first attempt
if (UI.firstReconnectTime === null) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UI.firstReconnectTime is never null, meaning your logic never works. Did you mean to check for 0?

UI.firstReconnectTime = Date.now();
}
const elapsedTime = Date.now() - UI.firstReconnectTime;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems you're not using this variable.


// Check if we've exceeded the max reconnect time
if ((Date.now() - UI.firstReconnectTime) >= maxTime) {
// 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)
UI.showStatus(_("Maximum reconnect attempts reached. Failed to connect to the server."), 'error', 180*60*1000);
UI.updateVisualState('disconnected');
return;
}

// if reconnect has been disabled in the meantime, do nothing.
if (UI.inhibitReconnect) {
Expand Down Expand Up @@ -1154,6 +1175,8 @@ const UI = {
}
UI.showStatus(msg);
UI.updateVisualState('connected');
// Here we can reset the retry count
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

misleading comment

UI.resetFirstReconnection();

UI.updateBeforeUnload();

Expand Down
4 changes: 3 additions & 1 deletion defaults.json
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
{}
{
"reconnect_max_time": 600000
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is that this should be empty. You will still get the same behavior since you added a default when calling UI.getSetting().

defaults.json could be used by integrators of the noVNC app to provide other defaults.