[RFC] change AssertError in exception hierarchy to allow simpler exception handling

[timotheecour]

In Nim: Exception is the root of all exceptions, whether they are catchable or not.
but according to @dom96 :

Technically, assertion failures are not catchable.

(which I understand as: the compiler will allow it but it's not recommended)

This prevents code like this that catches all "ethically" catchable exceptions:

try:
  foo
except Exception: # not good semantically because it would also catch `AssertionError`
  bar

I prefer D's design for its exception hierarchy:
EDIT [edited to add the missing Error as root of all un-ethical exceptions]
Throwable is the root of all exceptions
Exception and Error are children of Throwable
all unethically catchable exceptions are children of Error (eg AssertionError, QuitError; see https://dlang.org/library/object/error.html)
all ethically catchable exceptions are children of Exception (eg IOError etc)

This allows code to distinguish whether to catch all (compiler allowed) catchable exceptions, or just the "ethically" catchable ones, or some other subset.

I propose following D's approach here, it hopefully won't cause breakage:

except Exception: # ok, only catches "ethically" catchable exceptions
except: # implies "Exception"
except Throwable: # should be rarely used, to catch all compiler allowed exceptions
except AssertionError: # should be rarely used, to catch AssertionError

Furthermore, after that change is in place, throwing "Exception" (instead of one of the many subclasses) now becomes a sensible default option (which it currently isn't so sensible because catch Exception implies catching AssertionError in particular, which is not recommended), so, as in D, we can have an enforce proc that by defaults throws Exception. Makes it simpler than having to keep wondering each time what type of exception we should throw. D's design specifically has a rather flat exception hierarchy thanks to that aspect (not too many Exception subclasses).

reference for enforce: https://dlang.org/phobos/std_exception.html#enforce

Nim's suggested enforce: something like following:

template enforce(bool condition, message: string, exceptn: typedesc = Exception) = ...

which replaces code like:

if not conditionOK():
  raise newException(FooError, "bad")

with:

enforce(conditionOK() , FooError, "bad")
# or even in most cases, the simpler:
enforce conditionOK() # exceptn would be Exception, and `msg` can be auto-populated with macros from file:line and code, etc

EDIT: s/ref Exception/Exception/

[zah]

I think this is a good idea. Our own team had to decide on a common error handling approach recently and after much research on subject it is clear for me that there is a lot of value in discriminating between recoverable error conditions and programming errors (bugs). I've explained these terms and the reasoning behind this here:

https://gist.github.com/zah/d2d729b39d95a1dfedf8183ca35043b3

There is always a little bit of gray area regarding what really resides in the "non-recoverable" category and my own proposal suggests some ways to define this at the level of individual modules and scopes, but having such default distinction in the standard library is certainly step in the right direction.

[dom96]

FYI except ref Exception can just be except Exception, a related aspect that we need to change is that all exception types should be made into refs to allow the following:

raise Exception(msg: "Some error")

Right now we are forced to use this awkward template:

raise newException(Exception, "Some error")

[cooldome]

-1 from me. Assert is already flexible in Nim such you can have custom action in case of failure, raise/quit whatever you want. If you still not happy with system.assert you can implement your on assert, it takes less than 5 lines of code in Nim. There is little sense in changing all of the Exception hierarchy just to make 5 line template happy.

[timotheecour]

@cooldome That's not what this proposal is about, please re-read (and apologies if some parts were not explained properly). Currently there's no way to catch all "ethically" catchable exceptions (which I define as all exceptions except AssertionError), since there's no common root for all "ethical" exceptions. catching AssertionError is authorized by the compiler (and there are valid use cases eg debugging, and in unittest frameworks), however we shouldn't catch AssertionError in production code as it indicates a usually unrecoverable error. So that makes it impossible to catch all "ethical" exceptions:

try:
  foo
except Exception: # not good semantically because it would also catch `AssertionError`
  bar

It also forces one to choose which exception to raise in each piece of code that throws a non-AssertionError, which is often quite arbitrary.
Instead, D solved this problem by allowing to throw Exception (which, in D, is "ethically" catchable and rooted under Throwable), as well as catch-all-ethical clause shown above.

[andreaferretti]

You can just

try:
  foo
except Exception e:
  if e is AssertionError:
    raise e
  bar

and even hide this pattern inside a template. There is no need to force a particular definition of what is ethically catchable.

[Araq]

I think we need to rethink the exception hierarchy indeed, but AssertionError is just one example of this.

[timotheecour]

I think we need to rethink the exception hierarchy indeed, but AssertionError is just one example of this.

actually I had forgotten to introduce Error as children of Throwable and root of all un-unethical exceptions (eg AssertionError or QuitError ); fixed it in top-post.

[Araq]

This has been done/implemented.

[arnetheduck]

As a side note - I've seen plenty of cases where a "non-recoverable" error should have been "recoverable" but cannot be changed because this breaks api/abi - the other way around, not so often. Sounds like something to document at least..

[timotheecour]

can u provide some concrete examples please?

[Araq]

I've seen plenty of cases where a "non-recoverable" error should have been "recoverable" but cannot be changed because this breaks api/abi - the other way around, not so often. Sounds like something to document at least..

So ... better assume everything can throw to begin with and start loving exceptions again? ;-)

[arnetheduck]

can u provide some concrete examples please?

SIGPIPE on unixes must be the canonical example, with practically every sane app out there having to install a sig handler for it as a crutch because someone thought it a good idea that a closed file should cause an abort..

Another good example is rust - part of their 1.0 road was to clean up their std library so that it wouldn't panic, willy-nilly, to avoid situations like rust-lang/rust#30138.

The more a library uses "non-recoverable" errors, the less useful it is - it means that there are land-mines in the API that you can't tell from looking at it, and it's hard to make stability guarantees about code.. of course, it's convenient for the API author because they can simply shove these things under the carpet and pass the buck to everyone that uses the lib instead.

[Araq]

The more a library uses "non-recoverable" errors, the less useful it is - it means that there are land-mines in the API that you can't tell from looking at it, and it's hard to make stability guarantees about code..

So tell me, are clear violations of a function contract that happened to be not-encodable in the type system and that are mapped to a trap/quit/exit "hidden land-mines"? You seem to argue for turning clear programming bugs into recoverable exceptions or to encode it in the return type.

[arnetheduck]

You seem to argue for turning clear programming bugs into recoverable exceptions or to encode it in the return type.

no, I argue for the developer of the API to make an effort to avoid such situations completely, using the other tools at hand - types etc - and for there being language in the description of this feature pointing these things out as well as the std lib setting a good example - basically, an API that can't be used the wrong way ("against the contract"), as enforced by the compiler, won't be..

ie typical example: parsing a regex. I've come across a few libraries that consider this a panic situation - of course it's not. neither is having a file disappear in a multiprocess OS, or having a remote client close a socket.

other common examples include "you have to call this special init function only once before calling function yyy" - these tend to needlessly create an opportunity for breaking the contract, which later has to handled / checked for.

[Araq]

ie typical example: parsing a regex. I've come across a few libraries that consider this a panic situation - of course it's not. neither is having a file disappear in a multiprocess OS, or having a remote client close a socket.

Fair enough, but for me an AssertionError is exactly the same as an "index out of bounds" error which is bad to encode in the return type (or raise as a checked exception...).

[arnetheduck]

yeah, so the index-out-of-bounds is an excellent example of this principle - let's say you design a seq from scratch and the only way to walk the seq is to use the index operator [] and len, ie write a for loop with an integer index.. doing it this way, you're inviting off-by-one errors etc etc - is it 0-based? 1-based? all kinds of things that the user of the API has to think about - you're inviting them to break the contract so that you can raise your assert.

if instead you also have access through an iterator-like construct, the caller of that construct is guaranteed to not hit any contractual issues, simply because the API does not allow it.

so how will this feature get used? I'd say that two things will set the example - docmentation, and the standard library.. in introducing a feature like this, both of these need to catch up and be revised, such that an healthy eco-system can grow up around it

[Araq]

Ok, but "better API design" only gets you so far. And assert or "index out of bounds" exist almost universally out there and should not be catchable, ymmv.

[timotheecour]

the difference between defect and recoverable error is:

• defect is for program logic bugs (and some of these should be disabled when passing --assertions:off, ideally all of these in hypothetical scenario where application is fully debugged)
• recoverable error is for input/environmental error (and shouldn't be disabled by --assertions:off)
  that still leaves room for debate as to what's considered "input" vs "part of application" (boundaries can depend on context), but it's useful as a general guideline.
  If you're in a situation where user input has already been validated (eg throwing recoverable errors), then remaining errors should be defect, not recoverable error.

for example: index out of bounds is definitely on the category of defect; but consider this example: say you're writing an interpreter / REPL for a simple language in nim, then bad user inputs (eg: 1+2 {) or let a=[1,2]; echo a[3] should be recoverable by the application that handles user input, not crash the REPL.

[arnetheduck]

Thus the logic goes like this: All errors are unrecoverable until you can think of a single case where you should be able to recover ;)

[timotheecour]

that's not a correct conclusion. Here's an example:

proc foo(a: string)=
  if not validateInput(a): raise recoverableError("invalid input:" & a);
  let age = a.split("-")[2].parse(int) # this could throw a defect, but not a recoverable error: if it throws, there's a bug in `foo` or `validateInput`, not in user input

[arnetheduck]

But you just said you don't want your repl to crash.. for example because of a bug in validateInput that you're developing right there with the repl?

Also, in this particular example, if you structure your code more cleverly (by not separating validation from parsing), the need for the defect goes away.. of course, this can be hard if you're dealing with 3rd parties or existing code, but the frequency of such code will be directly related to the ease with which you can create it - ie if there's plenty of copy-paste material that looks like this in the std lib, that's what you'll get in end-user code as well..

[timotheecour]

But you just said you don't want your repl to crash.. for example because of a bug in validateInput that you're developing right there with the repl?

it's ok for repl to crash if there's a bug in repl code (eg validateInput) ; it's not ok for repl to crash if there's a bug in user input to the repl

Also, in this particular example, if you structure your code more cleverly (by not separating validation from parsing), the need for the defect goes away

granted; that was just a simplified example; proper parsing code would not separate validation from parsing and would not crash on bad user input but instead report catchable error.
It's analog to how, when compiling a nim program, "internal compiler error" are bugs in nim compiler code, whereas regular "error" are errors in user code

[dom96]

This has been done/implemented.

Can you elaborate about/provide links to what was actually implemented?