Java Exception Error Enumerations Anti-pattern

Question

Over the years I've many times seen folks enumerate error codes for exceptions in Java. I generally have felt this leads to more tedious code and does not provide value.

For example, there will be an enum like this ErrorType, often times with a numeric errorCode, which I find additionally egregious.

package so.example;

public enum ErrorType {
  INVALID_DATA_FORMAT(1000),
  MISSING_REQUIRED_FIELD(1010),
  MODIFY_DISABLED(2020),
  DUPLICATE_MERCHANT(2050),
  UNKNOWN_RESELLER(2051),
  XML_PARSING_FAILURE(2999),
  SYSTEM_ERROR(3000),
  AND_HUNDREDS_MORE_LIKE_THIS(4444);

  private final int errorCode;

  ErrorType(final int errorCode) {
    this.errorCode = errorCode;
  }

  public int getErrorCode() {
    return errorCode;
  }
}

Then there will be an Exception subclass which requires this enum as a construction parameter, like

package so.example;

public class SoExampleException extends GenericExampleException {

  public SoExampleException(ErrorType subtype, String message) {
    super(subtype, message);
  }

  public SoExampleException(ErrorType subtype, Exception causedBy) {
    super(subtype, causedBy);
  }
}

with code calling it like

  private void validate(final Request requestBody) throws SoExampleException {
    if (StringUtils.isBlank(requestBody.getSomeNumber())) {
      throw new SoExampleException(ErrorType.MISSING_REQUIRED_FIELD, "someNumber cannot be empty or null");
    }
  }

To me it feels like a type of anti-pattern because

1. Every time some new exception condition is dreamed up, then the enumeration has to be updated.
2. It implies callers should explicitly deal with each possible type, which makes the client exception handling tedious.
3. To me, an enumeration should really only be used for a small set of well known values, like "North, East, South, West" or "Red, Green, Blue". Having exceptions mapped to that implies that we know exactly, forever, the small set of things that can go wrong, which feels pretentious.

And the errorCode seems even more unnecessary considering this is an enumeration with a built-in ordinal.

Am I off base here? Is there a good reason for this type of pattern?

Answer 1

It implies callers should explicitly deal with each possible type, which makes the client exception handling tedious.

Yes, your calling code has to catch each and every Exception, then work out (from the error code) if it's an Exception that it can do something [useful] about and, if so, do whatever that is.
If it can't do anything, the it has to rethrow that same Exception and that's where this model gets horribly expensive. OK, "Exception shouldn't happen often", but every time they do, it may take numerous "throws" before the Exception finally reaches a catch clause that can do anything [useful] with it.

For that reason alone, I'd avoid doing things this way.

I'd define Custom exception classes for each "handleable" case and throw those. That way, a single throw will get that Exception to the correct catch clause first time.

IIRC, Java's "catch" clause only works at Class level, so you catch the "smallest" exception class first, and then successively "larger" ones, until you reach Exception itself.

catch( SubSubException ex ) { ... }
catch( SubException ex ) { ... } 
catch( Exception ex ) { ... } 

By comparison, .Net allows you to add property "filters" into the mix as well:

catch( SubException ex ) when ( ex.errorCode = ErrorType.INVALID_DATA_FORMAT ) { ... } 
catch( SubException ex ) when ( ex.errorCode = ErrorType.MISSING_REQUIRED_FIELD ) { ... } 

(Any other value of error code would bypass this catch altogether).
I've no idea what this does to the performance of the Exception-throwing process, though.

Answer 2

You haven't talked about proposed alternatives to this approach so I'll answer in generalities that may not be relevant to you in a particular situation. Usually, I would expect something like this to be done when you have a distributed system in which agreeing on the meaning of specific error codes is useful, if not extremely important. HTTP is a decent reference: error codes have specific well-defined meanings. If you return a 429 when you mean 403, that could be a big problem. That's probably the argument for specifying the codes. You don't want a code to change because someone change the order of the enum definition.

In such a system, the problem becomes how to manage those codes, make sure the correct codes are used and prevent redefinition of the same code with a different meaning. That last point is something I have seen go wrong and it was ugly: the code was used to map-in a description of what happened so the description you saw in an error report wasn't always the problem that actually occurred.

There are other approaches to be sure. I once worked on a system where the error codes were maintained in a DB table. That included the error "can't connect to DB" (Oops!) You could also just keep documentation which needs to be maintained like in HTTP.

But if you are going to have to maintain this documentation anyway and all (or many) of the applications in the system are built on Java, why not maintain it as an enum which you can then share across applications.

Now, as often happens, these kinds of things might be done for a good reason in one context and then someone repeats the approach in one where it doesn't make much sense. For example, if you just want to display a message to the user about an error, it might be over-kill to do something like this, unless you have very specific requirements about what error messages must say in a given circumstance.

This does have a lot of the problems you mention so if you have a different approach that meets your needs, I don't see any reason you have to do this.

Answer 3

So much work when it could just be:

public class InvalidDataFormatException extends GenericExampleException {}
public class MissingRequiredFieldException extends GenericExampleException {}
public class ModifyDisabledException extends GenericExampleException {}
public class DuplicateMerchantException extends GenericExampleException {}
public class UnknownResellerException extends GenericExampleException {}
public class XmlParsingFailureException extends GenericExampleException {}
public class SystemErrorException extends GenericExampleException {}
public class AndHundredsMoreLikeThisException extends GenericExampleException {}

You can put each in a separate file. This distributes the exceptions so you're not constantly updating one centralized file that forces the team to mob the ball. All it does is add a unique descriptive name to a perfectly useful exception. Lets you give each it's own catch handler. Happens to be my favorite use of inheritance.

As for the error codes I particularly hate code bases that mix exceptions and error codes. These are different paradigms. They don't mix well. So if you use exceptions don't use error codes.

If you're stuck with error codes because of some third party but refuse to use them as error codes properly because you're in love with exceptions then you leave me no choice:

public abstract class AbstractChildException extends GenericExampleException implements ErrorCode {}

public class InvalidDataFormatException extends AbstractChildException { @Override public int getErrorCode() { return 1000; } }
public class MissingRequiredFieldException extends AbstractChildException { @Override public int getErrorCode() { return 1010; } }
public class ModifyDisabledException extends AbstractChildException { @Override public int getErrorCode() { return 2020; } }
public class DuplicateMerchantException extends AbstractChildException { @Override public int getErrorCode() { return 2050; } }
public class UnknownResellerException extends AbstractChildException { @Override public int getErrorCode() { return 2051; } }
public class XmlParsingFailureException extends AbstractChildException { @Override public int getErrorCode() { return 2999; } }
public class SystemErrorException extends AbstractChildException { @Override public int getErrorCode() { return 3000; } }
public class AndHundredsMoreLikeThisException extends AbstractChildException { @Override public int getErrorCode() { return 4444; } }

And this is still distributable. You don't have to stick them all in one file. The only feature missing from this scheme that you had before is the ability to convert from a dynamic error code to a matching exception. Which I don't see the need for in any of your examples.

The AbstractChildException (yes it's a terrible name) has no body but it merges the concrete interface of SoExampleException with the ErrorCode interface letting you talk to any of these exceptions without knowing exactly which one you're talking to. Such as:

catch (AbstractChildException ace) {
    System.out.println("Error code: " + ace.getErrorCode());    
}

Is there a good reason for this type of pattern?

Well if you had a need to turn a dynamic error code into the matching exception sure. That feature is missing from this exception extending scheme. Oh there is probably some reflection magic thingy someone could do to enable it for this exception extending scheme if it's really important. But be sure you really need it before you worry about that.

Answer 4

Every time some new exception conditional is dreamed up, then the enumeration has to be updated.

Well, this question is slightly orthogonal to exceptions. If you have a (large) set of error codes, do you put them into an enum or into an integer. There are arguments for both, but since your title is about exceptions ...

It implies callers should explicitly deal with each possible type, which makes the client exception handling tedious.

Well, it implies quite the contrary to me. Having a SoExampleException that internally uses an enum just means that a client may inspect the embedded enum, or it may just decide that the details are not important.

This pattern is useful if you mostly don't care what specific error is thrown and you just want additional info that isn't a string.

Or, you have an existing system (like HTTP mentioned in the other answer, or OS error codes) that already defines (a large list of) error codes. Mapping each of those (100s) to a separate exception subclass doesn't seem all that useful to me.

To me, an enumeration should really only be used for a small set of well known values, like "North, East, South, West" or "Red, Green, Blue".

Coming back to that: Yes. Mostly an immutable set. That's why I personally would model a system with 100s of codes not with an enum but with a simple int32 to hold the error code. That also strongly signals that the error codes are basically unbounded (which a list of 100s is anyway).

Answer 5

Usually an enum should have few values, but in the end as many as needed. For example an enum for specifying system colours could have a large number of values, but that isn’t a problem.

One enum catching every error code is not good, because my code writing to a file for example only expects a tiny subset of all errors. Maybe sill a lot but not hundreds.

You could have an error category and a code within the category, and one error class per category. So your download code checks for download errors and doesn’t catch others.

Answer 6

Is there a good reason for this type of pattern?

It is an implementation for the requirement to add meaning for the details of the exceptions (e.g. it is easier to translate 1000 to INVALID_DATA_FORMAT than to understand IndexOutOfBoundException).

The details to choose between using class hierarchy and use a sole class is unrelated to whether to use error codes or not. A class hierarchy for exceptions it is suited when different exceptions require different ways of recovering from the unsupported running context/configuration/... and it is intended to have error recovery solutions implemented. When all it is done with the exceptions it is to write a message in the log file all the exceptions are the same the error handling code is the same either using an exception hierarchy or one exception, the difference laying in the try/catch statement...

try {
   . . .
} catch(GenericExampleException e) {
  // log the exception
}

...or try/catch each type of the exceptions and repeat the logging statement...

try {
   . . .
} catch(SomeExampleException e) {
  // log the exception
}

try {
   . . .
} catch(SomeOtherExampleException e) {
  // log the exception
}

try {
   . . .
} catch(AnotherExampleException e) {
  // log the exception
}