Writing (and Testing) Good Error Messages

Maybe I do spend more time on error messages than most people. I usually write some sort of unit test, which at least quadruples the time it takes to write "throw new RuntimeException()

"OH GOD NO! Unit testing exception messages, what is he thinking?"

Hear me out become calling me a pedant.

Contract-Driven Development is a technique more widely accepted than it is debated, especially for Java development. While Java does not support contracts as part of the language, the conventional wisdom I'm given is to think about what constitutes a method's contract and invariant state and then fail fast when this contract is violated.

Consider the following Java method:

public void process(Message msg) {
 Content content = msg.getContent();
 //...continue processing mesage content
}

At the most trivial level, thinking about contracts means thinking about the range of inputs. Null is certainly not a valid input to this method. One might be tempted to explicitly check for a null parameter like this:

public void process(Message msg) {
 if (msg == null) throw new IllegalArgumentException("Null: msg")
 Content content = msg.getContent();
 //...continue processing mesage content
}

This seems like a silly exception; you would have gotten a NullPointerException instead of an IllegalArgumentException, big deal. But what if the NullPointerException happened in the middle of a long process task instead of the first line of the method. Could a transaction be left half executed? What state is the system in when an exception is thrown halfway through a method? It's quite possible that the system would be left in an unstable state: a state that violates your class's invariant. So yes, I do believe we should be asserting the proper range of parameters on method input. In fact, this type of checking saved me a surprising amount of time on Friday. When we changed our web service schema, JAX-B beans started coming through our sevice with nulls in bad places. Our Flex developer was able to say, "Why am I getting a null: site exception" in our standup instead of asking, "why am I getting a null pointer at line 23 of class xxx". Good error messages are easier to diagnose.

Not only do I like to write argument checking exceptions but I also like to test them:

public void testProcess_Contract() {
 shouldFail(IllegalArgumentException) {
   service.process(null)
 }
}

This is not an incredibly helpful test and looks like the litter avid TDD practitioners like to scatter around a test tree. I write it mainly to get my testing juices flowing and to take the overhead of creating the test class. Plus it's part of my TestCase template file so it's probably just habit.

So what is this about testing error messages? As a system wide invariant, this is what I consider to be my contract with my users: Error messages should contain the cause of the problem and a solution to the problem. There is no error message on Java's NullPointerException, but "Null: message" satisfies this contract because it tells you what the cause of the error is (message) and what a solution is (don't pass null). By the way, including the failure information in the exception message is Effective Java Item 63: Include failure-capture information in detail messages. A better example can be seen in the Groovy codebase, where a method called visit requires some very specific objects to be in an array:

public void visit(ASTNode[] nodes, SourceUnit source) {
 if (!(nodes[0] instanceof AnnotationNode) || !(nodes[1] instanceof AnnotatedNode)) {
   throw new RuntimeException(
     String.format(
       "Internal error: wrong types: %s / %s. Expected: AnnotationNode / AnnotatedNode",
       nodes[0].getClass(),
       nodes[1].getClass())
   );
 }
 //... continue processing

This is quite a bit of code for error handling, largely because of my insistence on using String.format whenever I can. Anyway, if this error occurs you are told what happened (wrong types), you're told what was expected (AnnotationNode and AnnotatedNode), and you're told what was actually passed (nodes[0].getClass() and nodes[1].getClass()).

So should you test the exception message? Yes you test the contract: Error messages should contain the cause of the problem and a solution to the problem.

public void testVisit_Contract() {
 def message = shouldFail(RuntimeException) {
   def badInput = [new ConstantExpression("sample"), new EmptyExpression()] as ASTNode[]
   new SingletonASTTransformation().visit(badInput, null);
 }

 assertTrue(ex.getMessage().contains("wrong types"));
 assertTrue(ex.getMessage().contains("ConstantExpression"));
 assertTrue(ex.getMessage().contains("EmptyExpression"));
 assertTrue(ex.getMessage().contains("Expected: AnnotationNode / AnnotatedNode"));
}

Testing exceptions is fragile, I'll admit. But using assertTrue and String.contains minimizes that fragility. Here we're testing that the error is stated (wrong types), that the values causing the error are reported ("ConstantExpression" and "EmptyExpression"), and that the expected values are reported ("Expected: AnnotationNode / AnnotatedNode"). I find this testing approach to be a good compromise between not testing the contract and writing a fragile test, and the style still allows those annoying TDD zealots to write tests in your codebase while playing nicely with everyone else.

Lastly, I believe writing assertions on exception messages might fail under certain character set/localization schemes. I seem to recall this was a Java Puzzler at some point but don't have the book handy. Will these assertions fail under different character sets? I've never seen it happen myself.