Design strategy for wrapping exceptions

Question

I'm implementing a type of Repository for a framework/library that has (roughly) the following:

public interface FooRepository {

    boolean contains(String id);

    Foo fetch(String id);

    void commit(Foo foo):
}

We can implement it several different ways, depending on what storage medium we're using; for example

public class FileFooRepositiory implements FooRepository {

    public boolean contains(String id) {
        return fileExistsInFileSystem(id);
    }

    public Foo fetch(final String id) {
        return parseFooFromFile(id);
    }

    public void commit(final Foo foo) {
        writeFooToFile(foo);
    }
}

But we can also have

public class SQLFooRepositiory implements FooRepository {

    public SQLFooRepository(Connection connection) {
        // ...
    }

    public boolean contains(String id) {
        return fooExistsInDatabase(id);
    }

    public Foo fetch(final String id) {
        return parseFooFromDatabase(id);
    }

    public void commit(final Foo foo) {
        writeFooToDatabase(foo);
    }
}

There can also be implementations that store Foo objects in a Map<String, Foo> or ones that use external storage systems (appfabric, redis, etc).

However, there are things that can prevent each of these implementation from doing what they are supposed to do. They would each throw a different type of Exception. For SQLException, most things that could happen would use this one exception. For the file-based, it would be some sort of IOException, more specifically, FileNotFoundException, AccessDeniedException, etc.

My question is this: How can I change the contract of the FooRepository interface to allow these types of Exceptions being thrown, without using the throws Exception clause. My initial thought was to have a RepositoryException that would extend RuntimeException and wrap the actual exceptions

try {
    // ...
}
catch (SQLException sqle) {
    throw new RepositoryException(sqle);
}

but I'm not sure if this is the proper strategy I should be using.

Answer 1

Your solution is correct: repository should throw a generic RepositoryException-type to signal that something went wrong. You wrap the actual exception in that RepositoryException so your logging can get access to the data it needs and your consumers can trap the generic exception and handle it somewhere down the line. Wrapping the actual exceptions hides the implementation details of the concrete repository from the consumer.

The only reason you would differentiate between the exceptions (eg return specific exceptions from the interface) is when the handling of one exception type should be different from the other. In this case I can't really think of a reason you would handle exceptions from the repository differently, so just use the RepositoryException.

Answer 2

I think your strategy is correct (and I accept opinions may vary as to "best").

The client should not care if the underlying repository is a file or a DB server, they care that the commit failed - they need to know that first and handle it. Following on from that, if they wish (e.g. require user intervention or detailed logging) try deal with the underlying issue. Throwing an exception that represents that failure, the RepositoryException is the right thing to do. If that the one exception is sufficient here then use just that one. If the errors are more complex, then include more exceptions - but be judicious.

I generally advise that the exceptions thrown represent the error condition you wish to relay to the client - a "semantic error" if you will, not just the root cause. Inner exceptions (java getCause) are good for expressing the root cause of the failure that occurred.

Basically, I argue that the exception is "I could not do this, because the repository is not available", the inner exception is "I couldn't find the file because the user hasn't got permission."

Defining exceptions runs a fine line between offering the richness of detail required to describe the error to help client code correct it and not being too verbose that it becomes a burden to catch and handle.

Answer 3

Your approach is correct and if you need that your repository client could differentiate between repository exceptions, maybe for showing different kinds of errors, you could also create a tree of Repository Exceptions (always inheriting RuntimeException) like Spring does.

public class IntegrityConstraintViolationextends RepositoryException {

}

public class Client1 {
  public void consumeRepository(){
    try {
      getRepository.fetch
    } 
    catch (IntegrityConstraintViolatione1){
      // do Something special if there is a problem with integrity violation
    }
    catch (RepositoryException e2){
      // do something in any other case
    }
  }
}

In this case, you need that the underlying repository implementation have the ability of inform integrity constraint violations. For example the JDBC driver throws IntegrityConstraintViolation

try {
    // ...
}
catch (SQLIntegrityConstraintViolationException sqle) {
    throw new IntegrityConstraintViolation(sqle);
}

The advantage of this is that you always work with your own exception hierarchy tree.

Answer 4

Been there.

What I did was as follows:

In the file repository class that implements FooRepository:

    try{            
       // try to store
    } catch (IOException e) {
        throw new CouldntStoreFooException(e.getLocalizedMessage());
    } 

    try{            
       // try to retrieve
    } catch (IOException e) {
        throw new CouldntRetrieveFooException(e.getLocalizedMessage());
    } 

In the database repository class that implements FooRepository:

    try{            
       // try to store
    } catch (SQLException e) {
        throw new CouldntStoreFooException(e.getLocalizedMessage());
    } etc

    try{            
       // try to retrieve
    } catch (SQLException e) {
        throw new CouldntRetrieveFooException(e.getLocalizedMessage());
    }

That way the interface signature only knows about CouldntRetrieveFooException or CouldntRetrieveFooException but when you get the exception, and see the log, you can see the actual problem because you passed the original problem (e.getLocalizedMessage()) as a message in the constructor.

The exceptions doesn't need to wrap anything, just:

public class CouldntStoreFooException extends Exception {

    public CouldntStoreFooException(String reason) {
        super("Problem retrieving foo "+reason);
    }

}

...

public class CouldntRetrieveFooException extends Exception {

    public CouldntRetrieveFooException(String reason) {
        super("Problem retrieving foo "+reason);
    }

}