CODEWITHSUNDEEP
javarecursion
gene b.
gene b.

Reputation: 11994

Recursive call in try-catch block to retry N number of times

I have a regular, non-static sendMail method which may occasionally fail. I need to catch any errors and retry the method N number of times. I'm not sure I'm doing the right thing, and there's also a compilation error:

public void sendMail(List<String> params) {

   try {
       //...

       static int retrycount = 0; // static not allowed here (but need to keep static var for recursion)
       int maxretries = 3;
   }
   catch (Exception e) {
       log.info(e);
       // Recursion to retry
       sendMail(params);
       retrycount++;
   }
}

First of all, is recursion from a try/catch block correct? Also, is there a better way to do this?

I can't make the sendMail method static, there are too many references to it in the existing code.

Upvotes: 4

Views: 2603

Answers (3)

Tom Hawtin - tackline
Tom Hawtin - tackline

Reputation: 147164

The standard recursive solution would be to add retryCount as a paremeter.

public void sendMail(List<String> params) {
    sendMail(params, 0);
}
private void sendMail(List<String> params, int retryCount) {

   try {
       //...

       int maxRetries = 3;
   } catch (Exception e) {
       log.info(e);
       // Recursion to retry
       sendMail(params, retryCount+1);
   }
}

A loop would be the more idiomatic way of writing this.

public void sendMail(List<String> params) {
    int maxTries = 4;
    for (int tryCount=0; tryCount<maxTries; ++tryCount) {
        try {
            //...
            break;
        } catch (Exception e) {
            log.info(e);
            // continue to retry
        }
    }
}

In the spirit of the original question, the retryCount can be kept as a field within an introduced object. It's easiest (if slightly obscure) to do this with an anonymous inner class.

public void sendMail(List<String> params) {
    int maxTries = 4;
    new Object() {
        int tryCount = 0;
        public void sendMail() {
            try {
                //...
            } catch (Exception e) {
                log.info(e);
                // Recursion to retry
                if (tryCount < maxTries) {
                    ++tryCount;
                    sendMail();
                }
            }
        }
    }.sendMail();
}

Upvotes: 5

Adam
Adam

Reputation: 1754

Your retry will never work in the first place because inside every try block you are setting retrycount to 0.

You'd probably be better off throwing the exception instead of catching it. Then using some kind of a while loop till it completes, maybe with a configurable delay between retries. Or if you're using Spring there is the Retryable annotation.

void someMethod(){
  int attempts = 0;
  while(attemps <= 3){
    try {
      sendMail(...);
      break;
    } catch (Exception e){
      attempts++;
      // Log failed to send mail or something meaningful, maybe add a delay here?
    } 
  }
}

This solution is much cleaner than using recursion as if you wanted to retry many times, eventually you'd get a stack overflow error. It also keeps the responsbility of the sendMail function simple, and avoids adding complicated retry logic to an otherwise simple method.

Also, if you end up having to make other methods retryable in the same fashion then it would be much easier to abstract away the retry logic into some kind of executor service that handles it all.

Upvotes: 6

eattrig
eattrig

Reputation: 326

What if you just wrapped the code in a retry loop:

public void sendMail(List<String> params) {
  for (int attempt = 0; attempt < 3; attempt++)
   try {
       //...

       if (<some success condition>)
          return;
   }
   catch (Exception e) {
       log.info(e);
   }
}

Upvotes: 4

Related Questions