Catching base exception to throw a more specific exception?

Question

I have an Accessor and Repository interface. Accessor abstracts over where to store JSON documents (local file system, NoSQL database, etc). Repository abstracts over the representation of my domain objects and does my object-relational mapping. Accessor is injected into a Repository.

I have a UserRepository that is injected with any Accessor. For example, I could inject a FileSystemAccessor into this UserRepository, and it would pull JSON documents from my file system, mapped to User objects.

Accessor provides a method get_for_unique_value that finds a JSON document by a unique value for a specific key, given the name of a "collection" containing all these JSON documents. If multiple documents exist with the same value, a NonUniqueValueError is thrown.

UserRepository provides a method get_user_by_email that finds a user by their email. Multiple users are not supposed to exist with the same email, so I want to throw an exception to indicate this. The underlying injected Accessor provides get_for_unique_value, so get_user_by_email uses this internally. Any top-level controller would not be able to derive context from NonUniqueValueError bubbling up, so I want to catch the underlying NonUniqueValueErrorand throw a NonUniqueUserError instead.

NonUniqueUserError can be declared in three ways:

1. NonUniqueUserError takes the thrown NonUniqueValueError, and extends Exception.
2. NonUniqueUserError takes the thrown NonUniqueValueError, and extends NonUniqueValueError.
3. NonUniqueUserError extends NonUniqueValueError, without taking the caught NonUniqueValueError.

(1) preserves the underlying stack trace of the thrown NonUniqueValueError, but is not caught by a broader NonUniqueValueError clause higher up the call stack. (3) would be caught by such a clause, but loses the stack trace. (2) seems to combine the best of both worlds.

Am I approaching this problem right by catching a NonUniqueValueError and throwing NonUniqueUserError instead? If so, is there any reason I shouldn't go with (2)? I've read Is it really that bad to catch a general exception?, but want to confirm that I am still properly addressing the NonUniqueValueError exception here (as it is more general than NonUniqueUserError).

Catching base Exception to preserve data integrity shows a situation where the same exact exception is caught and thrown, but I'm trying to add context by throwing a new more specific exception.

Answer 1

First, let me answer your immediate questions:

• Adding more context to an otherwise unspecific exception by catching and rethrowing it is a standard technique, there is nothing wrong with it (See here or here.)

• When rethrowing, preserving the underlying stack trace is definitely a good idea, especially in case the stack trace from the unspecific exception may point to some code which is under your own control. If not, the stack trace may be useful for the vendor of the framework/library which throwed the exception.

• Making NonUniqueUserError a specialization of NonUniqueValueError sounds reasonable to me, but that is more a less a matter of taste.

However, I think the elephant in the room is the question whether it is a good idea - or not - to provide a method get_user_by_email which can only return exactly one user, when the system actually allows to store several users with the same email address. Even if that "should normally not be the case", it seems to be possible in the described system. When it occurs, it is not a program failure in the query code, but a data inconsistency. This means debugging the query code with a stack trace is most probably leading to nowhere.

To allow a caller to handle the "exceptional" situation where multiple of such users exist in a more flexible way, a method get_users_by_email (which returns an array or sequence of multiple users) appears to me the most sensible option. In case that method ever returns more than one user record, callers can still decide whether they prefer

1. to handle this case by displaying or logging the different multiple users, or

2. to select one of the results by additional means, like other attributes than the email address, or maybe an interactive selection, or

3. a different action like stopping the program, do nothing, or whatever is appropriate.

Just throwing an exception would prevent the caller from actions like #1 and #2. So instead of putting a lot of thought into the design of the forementioned exception classes, better check if an API design with no such specific exception at all might fit better to the given requirements.