DIP33: A standard exception hierarchy

[Jonathan M Davis]

On Monday, April 01, 2013 13:08:15 Lars T. Kyllingstad wrote:
> It's time to clean up this mess.
> 
> http://wiki.dlang.org/DIP33

The basic idea is good, but of course, some details need to be sorted out. As 
I explained in another response, I really think that we should have derived 
exceptions for many of the "kind"s so that they can be explicitly caught. And 
of course, in some cases, extra information can be put into the subclasses 
(e.g. UTFException contains extra data specific to it), so having derived 
exceptions is very valuable. Having the kinds is great, but I don't think that 
that should preclude having derived classes which can be explicitly caught, 
thereby allowing users to choose whether they want to catch the base type (and 
possibly use the kind field, possibly not) or to catch the derived types when 
they want error handling specific to the errors that those types represent. 
Using only kind fields rather than derived exception types makes it harder to 
use try-catch blocks to separate out error handling code like they were 
designed to do.

Another concern I have is InvalidArgumentError. There _are_ cases where it 
makes sense for invalid arguments to be an error, but there are also plenty of 
cases where it should be an exception (TimeException is frequently used in 
that way), so we may or may not want an InvalidArgumentException, but if you 
do that, you run the risk of making it too easy to confuse the two, thereby 
causing nasty bugs. And most of the cases where InvalidArgumentError makes 
sense could simply be an assertion in an in contract, so I don't know that 
it's really needed or even particularly useful. In general, I think that 
having a variety of Exception types is valuable, because you catch exceptions 
based on their type, but with Errors, you're really not supposed to catch 
them, so having different Error types is of questionable value. That doesn't 
mean that we shouldn't ever do it, but they need a very good reason to exist 
given the relative lack of value that they add.

Also, if you're suggesting that these be _all_ of the exception types in 
Phobos, I don't agree. I think that there's definite value in having specific 
exception types for specific sections of code (e.g. TimeException for time-
related code or CSVException in std.csv). It's just that they should be put in 
a proper place in the hierarchy so that users of the library can choose to 
catch either the base class or the derived class depending on how specific 
their error handling needs to be and on whatever else their code is doing. We 
_do_ want to move away from simply declaring module-specific exception types, 
but sometimes modules _should_ have specific exception types.

The focus needs to be on creating a hierarchy that aids in error handling, so 
what exceptions we have should be based on what types of things it makes sense 
to catch in order to handle those errors specifically rather than them being 
treated as a general error, or even a general error of a specific category. 
Having a solid hierarchy is great and very much needed, but I fear that your 
DIP is too focused on getting rid of exception types rather than shifting them 
into their proper place in the exception hierarchy. In some cases, that _does_ 
mean getting rid of exception types, but I think that on the whole, it's more 
of a case of creating new base classes for existing exceptions so that we have 
key base classes in the hierarchy. The DIP focuses on those base classes but 
seems to want to get rid of the rest, and I think that that's a mistake.

One more thing that I would point out is that your definition of 
DocParseException won't fly. file and line already exist in Exception with 
different meanings, so you'd need different names for them in DocParseException.

- Jonathan M Davis