Catching base exception to throw a more specific exception?
https://softwareengineering.stackexchange.com/questions/415821
-
15-03-2021 - |
Вопрос
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 NonUniqueValueError
and throw a NonUniqueUserError
instead.
NonUniqueUserError
can be declared in three ways:
NonUniqueUserError
takes the thrownNonUniqueValueError
, and extendsException
.NonUniqueUserError
takes the thrownNonUniqueValueError
, and extendsNonUniqueValueError
.NonUniqueUserError
extendsNonUniqueValueError
, without taking the caughtNonUniqueValueError
.
(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.
Решение
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 ofNonUniqueValueError
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
to handle this case by displaying or logging the different multiple users, or
to select one of the results by additional means, like other attributes than the email address, or maybe an interactive selection, or
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.