Refactoring Undifferentiated Exceptions

Posted on by

Some exception-handling code peels apart exceptions to figure out how to handle them. This is a missed opportunity for more direct code.

Exception handlers can work directly or indirectly; multiple handlers might handle exceptions from the same throwing code

The Simple Case: Type-Tested Exceptions

Sometimes we see handler code like this:

catch (Exception e) {
    // possible code before
    if (e is FileNotFoundException) {
        FNF-handling code
    } else if (e is NullPointerException) {
        NPE-handling code 
    } else if ... {
        etc
    }
    :
    } else {
        handle other Exceptions
    }
    // possible code after the conditional
}

(It could be a method, or part of an exception handler somewhere.)

To refactor this, you can work one exception type at a time:

  • Create a catch clause for each mentioned exception
  • Copy the original catch clause inside it
  • Simplify the conditional by deleting all the clauses that can’t happen
  • Remove the now-useless if statement

For the example above, that results in this:

catch (FileNotFoundException e) {
    // possible code before
    FNF-handling code
    // possible code after the conditional
} catch (NullPointerException e) {
    // possible code before
    NPE-handling code
    // possible code after the conditional
} catch (Exception e) {
    // possible code before
    handle other Exceptions
    // possible code after the conditional
}

One last thing to be careful of: make sure the catch clauses are in the same order as the if-statements. You generally want exceptions in the order of most-specific subtype to least specific. If your if-check wasn’t organized that way, it may have been an error, but changing the order can change the behavior.

The Hard Case: Non-Type-Checked Exceptions

Other handlers aren’t looking at types, but some other condition of the exception, such as:

if (e.innerException is FileNotFoundException) { ... }
  -or-
if (e.message.contains("some message fragment") { ... }

As for most code smells, this isn’t always wrong, but it’s a warning sign: we’re trying to reconstruct information rather than convey it directly.

Why is this the hard case? Because to do this refactoring with full safety, we have to examine many places that can throw and or catch, often based on possible call chains.

This is a big, sprawling refactoring. Be careful to keep the system running: you probably can’t do this in a sitting. Remember that methods throwing exceptions can be called from multiple paths, and exception handlers may catch exceptions from different methods.

Remedy

The handlers reveal the complexity, but the root problem is that these exceptions don’t differentiate themselves enough.

Instead of decoding inner exceptions or messages, let’s refactor to distinguish exceptions by type. Why type? Because that’s how the try-catch statement organizes what it catches.

A couple times, I’ve come across code that checks inner exceptions three deep. Simplifying this was a lot of work, but helped us rationalize all the error handling.

As you do these steps, you may be able to bounce between fixing handlers and fixing exceptions.

Step 1. Start from a Non-Exception-Typed Handler

Find a handler that uses inner exceptions, messages, etc. to decide what to do.

Step 2. Find All “Odd” Exceptions That Will Be Caught by This Handler

Exceptions of this structure may be thrown from several different places. You will have to work through the possible run-time paths of your code.

Step 3. New Exception Type

Create a new exception type, a subtype of the original exception. Choose a name that reflects the real condition being captured.

Step 4. For All “Odd” Exceptions, One at a Time

(You usually find these from a handler that digs inside an exception.)

Step 4.1 Make the Exception Use the New Type

Replace each “odd” thrown exception with an instance of the new type. However, you must make sure that the message, inner exception, etc. are structured exactly the same, as we’re still using the original handlers.

Step 4.2. Check that the Exception is Handled Properly

Test (automatically if you can, manually and/or with a debugger if you must) to make sure that the exception is still properly handled.

Step 5. Find Relevant Handlers

Locate any exception handler that might catch the original exception.

At first glance, this seems to require considering every handler. However, if the call chain to each relevant throwing site is not complicated, we may be able to exclude many handlers.

We can ignore handlers unrelated to the original type.

We now have handlers of the type of the thrown exception, or one of its parents. If the handler doesn’t examine the data of the exception (messages, inner exceptions, their messages, etc.), we can ignore it too.

We’re left with handlers that depend on the contents or structure of the exception.

Step 6. Add Type-Based Handlers

For each try-catch that handles this exception, add a type-specific handler for the new exception type, before the original catch clause.

Before:
 :
catch (UserException e) {
  if e.innerException.message.startsWith("File not found") {
    // handle "odd" exception
  } else {
    // handle other exceptions
  }
}

Add the new handler, but don’t yet delete the old one:

After:
 :
catch (NewException e) {
    // handle "odd" exception
} catch (UserException e) {
  if e.innerException.message.startsWith("File not found") {
    // handle "odd" exception
  } else {
    // handle other exceptions
  }
}

Note that you can bounce between throwing more-specific exceptions and updating the handlers.

Once you’ve made all “odd” exceptions throw suitable custom exceptions, and pulled out the corresponding handlers to a new catch clause, you can move on to the next step.

Step 7. Trap Special Clauses in the Handlers

Make a safety test. You have code as below, but the if clause should be dead code by now. (That is, no code can reach “handle odd exception” inside it.)

catch (NewException e) {
    // handle "odd" exception
} catch (UserException e) {
  if e.innerException.message.startsWith("File not found") {
    // handle "odd" exception
  } else {
    // handle other exceptions
  }
}

Modify the if clause to trap any usages:

catch (NewException e) {
    // handle "odd" exception
} catch (UserException e) {
  if e.innerException.message.startsWith("File not found") {
    // Set a breakpoint, crash instantly, print or log, etc.
    // Make sure you'd find out if any exception got here.
  } else {
    // handle other exceptions
  }
}

Run the tests again, making sure you don’t trap.

Step 8. Remove “Odd” Handlers

You’ve established that conditional isn’t used, so eliminate it. 

catch (NewException e) {
    // handle "odd" exception
} catch (UserException e) {
    // handle other exceptions
}

Run tests again.

Step 9. Simplify the New Exceptions

You now have no more “odd” exception handlers, but your new exceptions are potentially more complicated than they need to be, since they replicate the original message and inner exception structure.

Check each handler (not just NewException, but all handlers for all ancestor types including default handlers). If none of these look for a particular message or inner exception, you can remove that information from the new exception. You may need to do things like pull something from the inner exception and make it part of the new exception. That’s still better than digging through the structure.

If these handlers do use the structure of the exception:

  • Restructure the exception to directly have the information you want, but keep the old structure for now
  • Fix the handlers to use the direct information
  • Adjust the exception to no longer maintain the old structure

But note: the “ancestor” handlers might be handling other exceptions too; you’ve got to do a coordinated change, restructuring all exceptions that could be handled before updating the handlers.

Step 10. That’s It!

Easy, huh?

Trickiness: System Exceptions and Errors

That’s a mess of a refactoring. If you’re dealing with system exceptions (e.g., FileNotFoundException), you’ve got to be careful – those are not coming from your code. You’ve got to be aware of other code that might throw it.

Even if your language specifies thrown exceptions (as Java does), that specification might be for an ancestor type. Beware, take care.

If things can be bad, they can often be worse, and Error(s) apply here. In Java, you don’t have to declare throws clauses for Errors (whether system ones such as OutOfMemoryError or your own). That makes it even harder to find all the places that can throw, so you may be looking at a lot more handlers.

Deep-Structured Exceptions: Don’t Do It

“If it hurts, don’t do it” wouldn’t be a joke if it weren’t true. If you don’t create exceptions that rely on structure, you won’t ever have to handle them or try to refactor them away!

Conclusion

We’ve looked at a couple ways exception handlers can be improved: an easy way when we are doing manual type tests, and a much harder way when we’re looking inside an exception.

References

“Errors and Exceptions in Java”, https://www.baeldung.com/java-errors-vs-exceptions. Retrieved 2022-04-05.

“Testing Exceptions – Harder Than It Looks“, by Bill Wake. Retrieved 2022-02-03.