Retry after catch

Question

Here's the logic I used:

int retries = config.get("retries");
Response resp = null
do {
    try {
        resp = operation.execute();
        retries = 0;
    } catch (Exception ex) { //Note. Please excuse this catch pattern. Its not a problem now.
        if isAParticularException(ex) { //call a method to check the wrapped exception and other details
            retries--;
            LOGGER.info("Integrity exception, can be retried");
            if (retries == 0) {
                LOGGER.info("Retry exhausted");
                throw ex;
            }
            LOGGER.info("Retrying operation, retry count " + ledgerRetry);
        } else {
            throw ex;
        }
    }
} while (retries > 0);
return resp;

Number of retries is considering original operation as well. But the problem is that

if I return from try block directly without assigning anything, then SCA (Fortify for me) reports that the variable retries is not read (in success flow), and
if I assign and do as above, then SCA shouts about the immediate reassignment of value to the retries variable without even reading it.

Considerations:

The first call should be independent of whatever value we read for 'retries'
Duplicate code should be avoided, and avoiding recursion will be nice too.

May be a simple thing, but I am not catching it probably. Please suggest.

Rather than catching *every* `Exception` and then testing if it's a specific one, catch the specific one directly. `catch(SpecificException ex)` — QBrute, Jul 14 '17 at 05:43
It checks for a particular exception wrapped inside a generic exception. And since that is used by many other methods, its written in a method. Thats not the main concern for Foritfy issue resolution by the way now. — xploreraj, Jul 14 '17 at 05:55

score 0 · Answer 1 · answered Jul 14 '17 at 05:57

Why do you not use break instead of set retries to 0? I guess you sets retries after operation execute, because you want to break executing loop:

int retries = config.get("retries");
Response resp = null

do {
    try {
        resp = operation.execute();
        break;
    } catch (Exception ex) {
        if isAParticularException(ex) { //call a method to check the wrapped exception and other details
            retries--;
            LOGGER.info("Integrity exception, can be retried");
            if (retries == 0) {
                LOGGER.info("Retry exhausted");
                throw ex;
            }
            LOGGER.info("Retrying operation, retry count " + ledgerRetry);
        } else {
            throw ex;
        }
    }
} while (retries > 0);
return resp;

Or if you want you can return resp in try catch, and return null if did not execute anything:

int retries = config.get("retries");
Response resp = null

do {
    try {
        resp = operation.execute();
        return resp;
    } catch (Exception ex) {
        if isAParticularException(ex) { //call a method to check the wrapped exception and other details
            retries--;
            LOGGER.info("Integrity exception, can be retried");
            if (retries == 0) {
                LOGGER.info("Retry exhausted");
                throw ex;
            }
            LOGGER.info("Retrying operation, retry count " + ledgerRetry);
        } else {
            throw ex;
        }
    }
} while (retries > 0);
return null;

If I were you, I would consider throw exception instead of returning null.

In both of the snippets, SCA will complain about the variable `retries` not being used. — xploreraj, Jul 14 '17 at 05:59
I'm not sure if it's a good practice but you can set retries to 0 if it is less than 0 and use while loop instead of do while, and check if retries is >= 0 — m0ncld, Jul 14 '17 at 06:08
Not for sure :-) Its boggling me because the retry value is not hardcoded. I dont think its a good practice though. Because for wrong values, it will not run. THis means I have two checks for same thing. — xploreraj, Jul 14 '17 at 06:16
What if someone set retries to Integer.MIN_VALUE? if you execute retries-- you will have Integer.MAX_VALUE, and while (retries > 0) will be true — m0ncld, Jul 14 '17 at 06:26

Retry after catch

1 Answers1