Attempt to reconnect, with timeout

Question

I have a situation in which a program, communicating over the network, can timeout or otherwise fail to establish a connection through no fault of my application or its user. If and when this happens, an exception is thrown. The solution is obvious; simply try again a couple more times, and if multiple tries don't work then this is really a problem.

I've accomplished this by wrapping the original try-catch in a while loop with a retry counter, producing the following basic pattern:

var retryCount = 0;
while(true)
{
   try
   {
      AttemptToConnect();
      break;
   }
   catch(TimeoutException tex)
   {
      if(++retryCount < 3) continue;

      throw; //or handle error and break/return
   }
}

Do any of you see any problems? The biggest smell, prompting me to ask in the first place, is the conditionless loop, which would be infinite except that the "happy path" breaks out of the loop manually, while the terminating error will throw out. This has been called out before as a potential bug generator, if it's ever modified in the future (for instance to catch a second type of exception, or to do something else that could throw an exception).

Answer 1

Yeah, I'd probably redo it with an actual condition. That way, intent looks clear and is not dependent on internal code to break, continue, etc.

const int NumberOfRetries = 3;
var retryCount = NumberOfRetries;
var success = false;
while(!success && retryCount > 0)
{
   try
   {
      AttemptToConnect();
      success = true;
   }
   catch(TimeoutException tex)
   {
      retryCount--;

      if (retryCount == 0)
      {
          throw; //or handle error and break/return
      }
   }
}

Answer 2

I actually like the author's code. It's short and clear.

My only (minor) comment would be the if statement...

  if(++retryCount < 3) continue;

  throw; //or handle error and break/return

I would do;

  if(++retryCount > 2)
        throw; //or handle error and break/return

But that's only because I think it reads a little better.

Answer 3

With minor changes I would refactor it as follows:

var retriesLeft = 3;
var connectionEstablished=false;
while(retriesLeft>0)
{
   try
   {
      AttemptToConnect();
      connectionEstablished=true;
      break;
   }
   catch(TimeoutException tex)
   {
      retriesLeft-=1;
   }
}
if (connectionEstablished==false) throw new TimeoutException(); // or whatever
// Do stuff     

Answer 4

I would like to add few changes to the existing. When the attempt to connection fails, it is always better to retry the attempt after some delay.

1. Declare the _retryCounter and _connectionRetryInterval outside scope of the class, and make it configurable. For simplicity I am assigning values directly.

   private int _connectionRetryInterval = 3000;
   private int _maxRetryCount = 3;
   

2. Modified code is as below

   var retryCount = 0;
   var connectionEstablished = false;
   while (retryCount < _maxRetryCount)
   {
       try
       {
           AttemptToConnect();
           connectionEstablished = true;
           break;
       }
       catch (TimeoutException tex)
       {
           // log the exception with retry count.
           System.Threading.Thread.Sleep(_connectionRetryInterval);
           retryCount += 1;
       }
   }
   
   if (connectionEstablished == false)
   {
       throw new TimeoutException(); // or whatever
   }