[GitHub] [guacamole-client] necouchman commented on a change in pull request #547: GUACAMOLE-1126: Set a maximum of 4 auto-reconnect attempts

[GitBox <gi...@apache.org>]

necouchman commented on a change in pull request #547:
URL: https://github.com/apache/guacamole-client/pull/547#discussion_r450348942



##########
File path: guacamole/src/main/webapp/app/client/controllers/clientController.js
##########
@@ -792,7 +808,13 @@ angular.module('client').controller('clientController', ['$scope', '$routeParams
             var errorName = (status in TUNNEL_ERRORS) ? status.toString(16).toUpperCase() : "DEFAULT";
 
             // Determine whether the reconnect countdown applies
-            var countdown = (status in TUNNEL_AUTO_RECONNECT) ? RECONNECT_COUNTDOWN : null;
+            if (status in TUNNEL_AUTO_RECONNECT && AUTO_RECONNECT_TRY < AUTO_RECONNECT_MAXTRY ) {
+                var countdown = RECONNECT_COUNTDOWN;
+                AUTO_RECONNECT_TRY++;
+            } else {
+                var countdown = null;
+                AUTO_RECONNECT_TRY = 0;
+            }

Review comment:
       Since the countdown is already part of the notification, I would think it would be pretty easy to add the number of tries and max tries pretty easily.

##########
File path: guacamole/src/main/webapp/app/client/controllers/clientController.js
##########
@@ -44,6 +44,16 @@ angular.module('client').controller('clientController', ['$scope', '$routeParams
     var requestService         = $injector.get('requestService');
     var tunnelService          = $injector.get('tunnelService');
     var userPageService        = $injector.get('userPageService');
+	
+	/**
+	 * Counter and limiter for maximum auto-reconnect attempts that we should 
+	 * automatically try to establish a connection when CLIENT_AUTO_RECONNECT or 
+	 * TUNNEL_AUTO_RECONNECT error types occur.
+	 *
+	 * @type Number
+	 */
+	var AUTO_RECONNECT_TRY     = 0;
+	var AUTO_RECONNECT_MAXTRY  = 4;

Review comment:
       So, rather than setting a constant, here, I would say:
   - Add an attribute to the Connection for tracking whether auto-reconnect is disabled or not (AUTO_RECONNECT_DISABLE, for example), which defaults to having auto-reconnect enabled.
   - Add an attribute to the Connection for setting the maximum number of retries, which should default to unlimited.
   - Might even be worth making the retry interval configurable with a similar attribute, such that it defaults to Guacamole's global default, but could be set per-connection to something other than 15 seconds.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org