Catching an exception that is nested into another exception

Question

I want to catch an exception, that is nested into another exception. I'm doing it currently this way:

} catch (RemoteAccessException e) {
    if (e != null && e.getCause() != null && e.getCause().getCause() != null) {
        MyException etrp = (MyException) e.getCause().getCause();
        ...
    } else {
        throw new IllegalStateException("Error at calling service 'service'");
    }
}

Is there a way to do this more efficient and elegant?

Handicap

Answer 1

There is no more elegant way of selectively "catching" nested exceptions. I suppose if you did this kind of nested exception catching a lot, you could possibly refactor the code into a common utility method. But it still won't be either elegant or efficient.

The elegant solution is to do away with the exception nesting. Either don't chain the exceptions in the first place, or (selectively) unwrap and rethrow the nested exceptions further up the stack.

Exceptions tend to be nested for 3 reasons:

1. You have decided that the details of the original exception are unlikely to be useful for the application's error recovery ... but you want to preserve them for diagnostic purposes.

2. You are implementing API methods that don't allow a specific checked exception but your code unavoidably throws that exception. A common workaround is to "smuggle" the checked exception inside an unchecked exception.

3. You are being lazy and turning a diverse set of unrelated exceptions into a single exception to avoid having lots of checked exceptions in your method signature¹.

In the first case, if you now need to discriminate on the wrapped exceptions, then your initial assumptions were incorrect. The best solution is change method signatures so that you can get rid of the nesting.

In the second case, you probably should unwrap the exceptions as soon as control has passed the problematic API method.

In the third case, you should rethink your exception handling strategy; i.e. do it properly².

---

^{1 - Indeed, one of the semi-legitimate reasons for doing this has gone away due to the introduction of the multi-exception catch syntax in Java 7.}

^{2 - Don't change your API methods to throws Exception. That only makes things worse. You now have to either "handle" or propagate Exception each time you call the methods. It is a cancer ...}

Funds

The third reason for this can also be solved by throwing a more generic superclass of the types of exceptions that can be thrown (like IOException). This is only the case if the are a lot of exception types that are similar and thus can be grouped. – Nagari

@Nagari - that only "works" in some circumstances. If the common supertype of the exceptions is Exception, then declaring that is a really bad idea. Besides, it is often better from the point of view of code readability and maintainability to declare the individual exceptions. Or at least, up to a point. Either way wrapping is a lazy solution for the case #3 "problem". – Funds

Answer 2

The ExceptionUtils#getRootCause() method can come in very handy in such situations.

Handicap

If you just need the root cause then this is it. But if you are looking for a specific Exception implementation in the causedBy chain, check out my answer below. – Camerlengo

Answer 3

You should add some checks to see if e.getCause().getCause() is really a MyException. Otherwise this code will throw a ClassCastException. I would probably write this like:

} catch(RemoteAccessException e) {
    if(e.getCause() != null && e.getCause().getCause() instanceof MyException) {
        MyException ex = (MyException)e.getCause().getCause();
        // Do further useful stuff
    } else {
        throw new IllegalStateException("...");
    }
}

Nagari

The check for e.getCause().getCause() != null is not needed n this case since instanceof will return false if it is null. – Slow

Answer 4

I just solved a problem like this by writing a simple utility method, which will check the entire caused-by chain.

  /**
   * Recursive method to determine whether an Exception passed is, or has a cause, that is a
   * subclass or implementation of the Throwable provided.
   *
   * @param caught          The Throwable to check
   * @param isOfOrCausedBy  The Throwable Class to look for
   * @return  true if 'caught' is of type 'isOfOrCausedBy' or has a cause that this applies to.
   */
  private boolean isCausedBy(Throwable caught, Class<? extends Throwable> isOfOrCausedBy) {
    if (caught == null) return false;
    else if (isOfOrCausedBy.isAssignableFrom(caught.getClass())) return true;
    else return isCausedBy(caught.getCause(), isOfOrCausedBy);
  }

When you use it, you would just create a list of if's from most specific Exception to least specific, with a fallback else-clause:

try {
  // Code to be executed
} catch (Exception e) {
  if (isCausedBy(e, MyException.class)) {
    // Handle MyException.class
  } else if (isCausedBy(e, AnotherException.class)) {
    // Handle AnotherException.class
  } else {
    throw new IllegalStateException("Error at calling service 'service'");
  }
}

Alternative/Addition per requests in comments

If you want to use a similar method to get the Exception object of the class you're looking for, you can use something like this:

  private Throwable getCausedByOfType(Throwable caught, Class<? extends Throwable> isOfOrCausedBy) {
    if (caught == null) return null;
    else if (isOfOrCausedBy.isAssignableFrom(caught.getClass())) return caught;
    else return getCausedByOfType(caught.getCause(), isOfOrCausedBy);
  }

This could be used in addition to isCausedBy() this way:

  if (isCausedBy(e, MyException.class)) {
    Throwable causedBy = getCausedByOfType(e, MyException.class);
    System.err.println(causedBy.getMessage());
  }

It can also used directly instead of isCausedBy(), although it's probably a matter of opinion whether this is more readable.

  Throwable causedBy;
  if ((causedBy = getCausedByOfType(e, IllegalAccessException.class)) != null) {
    System.err.println(causedBy.getMessage());
  }

Camerlengo

Great solution, but I guess isCausedBy should be getCausedBy and return searched class not just boolean. Otherwise to handle exception you'll have to search for it again if isCausedBy returns true. – Shadbush

@MaksimMaksimov not a bad point. It would make the handling code a little more cumbersome if you're trying to handle more than one specific implementation. But if you're only looking for one specific one and/or you need the causedBy.getMessage() without the stack trace then your suggested adjustment is needed. – Camerlengo

Hey.. can you guys please put an example of the suggestion given by @MaksimMaksimov Would be a great help.. – Leavy

@Leavy I added an example. Don't forget to upvote if this answer is useful! :) – Camerlengo

@RicardovandenBroek I don't see the getCausedByType method referenced in your example of how to use it with isCausedBy(). Should getCausedBy(e, MyException.class) be getCausedByOfType(e, MyException.class)? – Gable

@MassDotNet Good eye! I've update that code block to correctly reference getCausedByOfType(). The return type on that method was actually also wrong, so I corrected it. Hope this helps! – Camerlengo

Answer 5

I see no reason why you want exception handling to be efficient and elegant, I settle for effective. They're called Exceptions for a reason.

This code will be a maintenance nightmare. Can't you redesign the call stack to throw the Exception you are interested in? If it is important the method signatures should show it and not hide it wrapped in 2 other exceptions.

The first (e != null) is unnecessary.

And you can change the 3rd better to e.getCause().getCause() instanceof MyException)

Scottyscotus

"I see no reason why you want exception handling to be efficient and elegant ..." - really? REALLY? (Sure, it isn't going to happen, but that is no reason not to want it to happen.) – Funds

:-) I know, but I only have a limited number of hours in a day and I rather have the rest efficient and elegant. I mean trying to get Java Exception handling elegant is like trying to teach ballet to a rhinoceros : it'll never be pretty. And throwing exception is slow, so it will never be efficient. So I settle for effective. – Scottyscotus

It would be a maintenance nightmare if it contained bugs (and in this code sample, it does.) The biggest frustration for a support engineer is a bug in exception-handling code - it ought to have captured relevant information about a situation, yet it doesn't (and then died without a trace). – Coarse

I ogree wholeheartedly. The less Exception handling the better. It is a 1000x easier to do it wrong than to do it right. Life would be better if there are less places where exceptions are caught and more exceptions are allowed to fly. And yes, that sometimes you cannot effectively 'hide implementation' etc, but it is a lot better than hiding faults triggered by defects. – Scottyscotus

Answer 6

You can do as below:

catch (RemoteAccessException e) {
    int index = ExceptionUtils.indexOfThrowable(e, MyExcetption.class)
    if (index != -1) {
         //handleMyException
    } else {
    }
}

Debate

Answer 7

I suppose you could also use ExceptionUtils.throwableOfThrowable() as in here

Duggan

Answer 8

If you are investigating that whether an exception is caused by a custom exception (e.g. MyException) you can iterate with a while-loop until you find an instance of MyException.

boolean isCausedByMyException(Throwable exception) {
    do {
        if (exception instanceof MyException) {
            return true;
        }

        exception = exception.getCause();
    } while (exception != null);

    return false;
}

Credendum

Answer 9

I doubt, but you can check with instanceof if the exception is of the correct type.

Edit: There should be a reason that the nested exception is wrapped, so you have to ask yourself what is the purpose of catching the nested one.

Traffic