This week's book giveaway is in the Java 9 forum.
We're giving away four copies of Java 9 Modularity: Patterns and Practices for Developing Maintainable Applications and have Sander Mak & Paul Bakker on-line!
See this thread for details.
Win a copy of Java 9 Modularity: Patterns and Practices for Developing Maintainable Applications this week in the Java 9 forum!
  • Post Reply Bookmark Topic Watch Topic
  • New Topic

Retry after exception - SCA compliant  RSS feed

 
Rajdeep Biswas
Ranch Hand
Posts: 231
1
Eclipse IDE Java Opera
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Here's the logic I used:


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.
How do I write it nicely?

 
Campbell Ritchie
Marshal
Posts: 56187
171
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Rajdeep Biswas wrote:. . . . . .
Did you really use instanceof there? That isn't right, surely? You catch a particular exception type. I am surprised the code verifying tool complained about retries and didn't say anything about instanceof. The bit about retries being 0 is also difficult to read (line 11).
I presume you are sure that particular kind of Exception allows you to retry immediately without altering anything else?
 
Rajdeep Biswas
Ranch Hand
Posts: 231
1
Eclipse IDE Java Opera
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
The instanceof is not there, just I added to make others understand. There is a method call that checks the contents of the exception.
 
Campbell Ritchie
Marshal
Posts: 56187
171
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Please post the actual code you are running. We can hardly help on different code.
 
Rajdeep Biswas
Ranch Hand
Posts: 231
1
Eclipse IDE Java Opera
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
As I said, it calls a boolean returning method passing on the exception object, and in the method we check for exception message contents and other things. But I wonder why should it matter because the assumption is that if the condition is met, i.e. if the method returns true, then we think of retry, anything else we throw the exception. My concerns I have put.

 
Rajdeep Biswas
Ranch Hand
Posts: 231
1
Eclipse IDE Java Opera
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
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.
     
    Dave Tolls
    Ranch Foreman
    Posts: 2996
    37
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    To be honest, I think that code looks fine to me.
    This looks more like Fortify having issues identifying the flow.
    It essentially doesn't understand that retries is for handling the failure flow.
     
    Liutauras Vilda
    Marshal
    Posts: 4783
    330
    BSD
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Dave Tolls wrote:To be honest, I think that code looks fine to me.

    Apart maybe from the line 8, which supposed to prevent code from compiling.
     
    Dave Tolls
    Ranch Foreman
    Posts: 2996
    37
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    This bit?


    It's clunky, but if you've already wrapped the actual exception types that you can recover from, then it's fair enough I suppose.

    Didn't think it was preventing anything from compiling, though.
     
    Campbell Ritchie
    Marshal
    Posts: 56187
    171
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Agree; that is a great improvement on the previous code. It looks strange to have a method for checking an Exception, however. You have some repeated code, logging the exception thrice and rethrowing it twice. You can probably simplify that to LOGGER.info(exceptionMessage); or similar.
    If you are simply testing the type of the exception, what aboutPlease check very carefully because I might have got the flow of control completely wrong. I also suspect I have got one { too many. Why do I have the catch in lines 16‑18? I suspect I can miss that out and simply allow the exception to propagate.
     
    Rajdeep Biswas
    Ranch Hand
    Posts: 231
    1
    Eclipse IDE Java Opera
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    That is not my problem at all. Please read the last statements in original post.
     
    Liutauras Vilda
    Marshal
    Posts: 4783
    330
    BSD
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Dave Tolls wrote:This bit?

    Yes. I was just saying, if OP claims it is a real code he's using currently, then it supposed to fail to compile as there are missing parentheses for "if" statement. But I might misunderstanding something what is discussed here as didn't read thread carefully.
     
    Dave Tolls
    Ranch Foreman
    Posts: 2996
    37
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Liutauras Vilda wrote:
    Yes.


    Ohhhhh!
    I missed that.


    Actually...would you have a different result with a while loop?
    Or is it because it's Friday and I'm not thinking straight?
     
    Dave Tolls
    Ranch Foreman
    Posts: 2996
    37
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    No...I'm right I think.

    At the moment if 'retries' is 2, then it goes around a maximum of 2 times (so it's actually only retrying once).
    If it were a while loop you would get the same result...2 attempts.
     
    Rajdeep Biswas
    Ranch Hand
    Posts: 231
    1
    Eclipse IDE Java Opera
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Ok, I am going to put things once again.

    1. I am reading number of retries from outside.
    2. Now, dont take it literally. If I read a value of 3 in retries, then, sat if we have the known exception in each execution, then total number of times the operation is executed is 3 (including original request, effectively meaning 2 retries instead of 3). Please ignore this thing. I will modify it later.
    3. The way catch is defined and handled also is not in the scope of my discussion.

    Here I am reiterating my issue:

    1. If I do like this:

    then I am getting issue that variable "retries" is not read. So to resolve this, I tried below:
    2.
    this now removes above problem but introduces another problem - redundant initialization of "retries", i.e. without being used, retries is being reassigned another value.
    So I am looking for a clean approach to write it in a better way.
    Here are my 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.
  •  
    Dave Tolls
    Ranch Foreman
    Posts: 2996
    37
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Rajdeep Biswas wrote:Ok, I am going to put things once again.


    I understand what your issue is.
    I think you might be putting too much faith in the code analysis tool...though having said that, changing your loop to a while rather than a do-while will fix all those issues that Fortify is complaining about.  The only situation it will cause a problem is if retires is set to 0 in the config.  Since that may well be an issue (I wouldn't like to trust people not to change things) then either do a check on 'retries' or stick with your current code and ignore Fortify.
     
    Rajdeep Biswas
    Ranch Hand
    Posts: 231
    1
    Eclipse IDE Java Opera
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Alright Dave. Checking and setting retries to 1 for wrong values was in my mind. Just thought if we have a better piece of code in the same manner as do ... (once), and while ...(retries) - try-again. It wont be an issue though, I will go with former only if thats that.
     
    With a little knowledge, a cast iron skillet is non-stick and lasts a lifetime.
    • Post Reply Bookmark Topic Watch Topic
    • New Topic
    Boost this thread!