/apr 13, 2015

Avoiding Nested Try-Catch in Java

By Asankhaya Sharma

Error handling mechanisms like exceptions cause deviations in the usual control flow of the program. These deviations make it harder to reason about the program and are often the source of bugs in the program. Java provides exception handling via the Try-Catch-Finally statement. A www.cs.virginia.edu/ ~weimer/p/weimer-toplas2008.pdf 2008 study of open-source Java programs by Wiemer and Necula found that exception handling contributes to up to 5% of the source code and up to 45% of program text was transitively reachable from catch and finally blocks. They also found that sometimes for correct handling of exceptions you need to nest Try-Catch-Finally blocks, however developers avoid doing so as it affects readability of the code. In this article, we look at various examples of nested Try-Catch-Finally statements in Java and how and when to avoid them.

Nested Try-Finally Example

Let us start with the following example from the paper by Wiemer and Necula:

Connection cn;
PreparedStatement ps;
ResultSet rs;
cn = ConnectionFactory.getConnection(/* ... */);
try {
    StringBuffer qry = ...; // do some work
    ps = cn.prepareStatement(qry.toString());
    try {
        rs = ps.executeQuery();
        try {
            ... // do I/O-related work with rs
        } finally {
            rs.close();
        }
    } finally {
        ps.close();
    }
} finally {
    cn.close();
}

In this example, while using several resources (cn, ps and rs) simultaneously, it becomes necessary to nest Try-Finally blocks 3 levels deep to release them properly with the close() method. The resulting code is not very readable and may be even confusing to some developers. An alternative is to do something like the following:

Connection cn = null;
PreparedStatement ps = null;
ResultSet rs = null;
try {
    cn = ConnectionFactory.getConnection(/* ... */);
    StringBuffer qry = ...; // do some work
    ps = cn.prepareStatement(qry.toString());
    rs = ps.executeQuery();
    ... // do I/O-related work with rs
} finally {
    if (rs != null) try {rs.close()} catch (Exception e) {}
    if (ps != null) try {ps.close()} catch (Exception e) {}
    if (cn != null) try {cn.close()} catch (Exception e) {}
}

This solution uses null as a sentinel value. Although it is correct, it is more prone to error. e.g. it is easy to forget checking for a particular resource. In general, correctly dealing with N resources like this would require N nested Try-Finally blocks or a number of runtime checks.

Nested Try-Catch Example

Similar to the situation with a Try-Finally block, we may sometimes need to nest Try-Catch blocks. As an example, take a look at the code below (taken from this link) :

try {
    transaction.commit();
} catch {
    logerror();
    try {
        transaction.rollback();
    } catch {
        logerror();
    }
}

While committing a transaction, if it fails we log the error and then try to recover by rolling back. However this rollback can fail itself which requires another nested Try-Catch block to handle correctly. In fact a common use case that leads to a nested Try-Catch happens when error recovery from an exception itself may raise an another exception.

Recently, I ran into a similar issue while trying to use the JRuby-Parser library to parse some Ruby code. The parser provided by the library has three compatibility versions for parsing different versions of Ruby (2.0, 1.9 and 1.8). Hence, to parse arbitrary Ruby code we needed to try them each one by one if the first one fails. The parser throws can exception if it fails to parse the code, so I ended up writing something with two nested Try-Catch blocks as shown below:

try {
    Node root = rubyParser.parse(f, new StringReader(fileContent), new ParserConfiguration(0, CompatVersion.RUBY2_0));
      root.accept(new RubyVisitor(calls));
} catch (Exception e) {
    try {
        Node root = rubyParser.parse(f, new StringReader(fileContent), new ParserConfiguration(0, CompatVersion.RUBY1_9));
        root.accept(new RubyVisitor(calls));
    }
    catch (Exception ex) {
        Node root = rubyParser.parse(f, new StringReader(fileContent), new ParserConfiguration(0, CompatVersion.RUBY1_8));
        root.accept(new RubyVisitor(calls));
    }
}

How to Avoid the Nesting?

The nested Try-Catch block above is actually totally avoidable. One way to safely rewrite it so that it doesn't use a nested Try-Catch is as follows:

for(CompatVersion cv : cvarr) {
  try {
    root = rubyParser.parse(f, new FileReader(f), new ParserConfiguration(0, cv));
    break;
  }
  catch (Throwable e){
    Logger.getLogger(SourceAnalysis.class.getName()).log(Level.WARNING, "Parse error in file " +
          f + " and parser version " + cv, e);
  }
}

I refactored the code to move the Try-Catch inside a loop which tries different compatible versions of the parser. So one way to avoid the nesting is to change the control flow around the block. Another good way (described here) to avoid a nested Try-Catch-Finally block is to see if we can combine them. This is more useful if the nesting is only used to catch different kinds of exceptions and can be combined as follows:

try {
    //code
} catch (FirstKindOfException e) {
    //do something
} catch (SecondKindOfException e) {
    //do something else
}

However, in some cases is may not be possible to avoid the nesting (e.g. when error recovery itself can throw exceptions). Even then we can improve the readability of the code by using early exists and moving the nested part into a new method. Thus, for the transaction error recovery example earlier we can write it without nesting in the following way:

try {
    transaction.commit();
} catch {
    callerrorrecovery();
}
private void callerrorrecovery() {
    try {
        transaction.rollback();
    } catch {
        logerror();
    }
}

Extracting the nested part as a new method will always work for any arbitrarily nested Try-Catch-Finally block. So this is one trick that you can always use to improve the code. Do you consider nested Try-Catch-Finally blocks to be a bad practice? Or, if you know some other ways to avoid them do let us know in comments.

Related Posts

By Asankhaya Sharma

Dr. Asankhaya Sharma is the Director of Software Engineering at Veracode. Asankhaya is a cyber security expert and technology leader with over a decade of experience in creating security products for industry, academia and open-source community. He is passionate about building high performing teams and taking innovative products to market. He is also an Adjunct Professor at the Singapore Institute of Technology.