Add "attempt" keyword for Go-like error handling

[d-torrance]

This is an alternative to #4047 that may address @mahrud's concerns. Rather than touching try, we add a new keyword attempt that returns a pair (val, err) (much like Go), where val is the value of the code and err is null if no error occurred, and otherwise val is null and err is an instance of a new Error type with information about the error that occurred .

i1 : attempt 5

o1 = (5, )

o1 : Sequence

i2 : attempt 1/0

o2 = (, Error{"message" => division by zero})
              "position" => stdio:2:8-2:11

o2 : Sequence

[mahrud]

I really like this solution!

[seangrate]

This seems like a good approach. Hopefully at some point, we can also build in some sort of error types so that we can check for particular errors by their types as opposed to their messages.

[d-torrance]

That sounds like a good idea!

[pzinn]

what I don't understand is, the output of attempt is a pair, but either the first or the second element of the pair is null? what's the point? one might as well test if output is an instance of Error

[pzinn]

the location of the error is different than in the usual error message. is that intentional?

i1 : 1/0
stdio:1:1:(3): error: division by zero

i2 : attempt(1/0)

o2 = (, Error{"message" => division by zero})
              "position" => stdio:2:8-2:11

o2 : Sequence

(it may not be obvious, but in the example the first error point to / whereas the second points to 1/0)

[pzinn]

This location doesn't seem right either:

i1 : attempt(1/a)

o1 = (, Error{"message" => no method for binary operator / applied to objects:})
                                       1 (of class ZZ)
                                 /     a (of class Symbol)
              "position" => ../../Macaulay2/m2/robust.m2:101:25-101:66

o1 : Sequence

[d-torrance]

what I don't understand is, the output of attempt is a pair, but either the first or the second element of the pair is null? what's the point? one might as well test if output is an instance of Error

That's a good point. I was basically just copying the Go syntax that Mahrud pointed out in the other PR. Also, in an earlier draft, the second element was a string, not an Error object, so it was necessary then.

[d-torrance]

As for the file positions, as I recall, errors originally all start out with dummyPosition and then get updated in various places, right? I think attempt is grabbing it a little later than it would have gotten printed if printing wasn't suppressed by tryEval, so maybe the position was updated a bit more in the meantime?

[mahrud]

what I don't understand is, the output of attempt is a pair, but either the first or the second element of the pair is null? what's the point? one might as well test if output is an instance of Error

Strictly speaking, in Go the return value is always of the expected type, so attempt 1/0 should return (0, Error{...}) or something like that, but currently there's just no output. But even for us, because of type consistency this is more readable:

(inv, err) := attempt inverse n
if err =!= null then error("oops: ", err#"message")

than a situation where the return value has different types in each branch:

ret := attempt inverse n
if not instance(ret, Error) then inv := ret else error("oops: ", ret#"message")

Also, what if you're writing an error-handling related function that needs to return an error? You need to separate the legitimate output Error object from the error that may have occurred in the process :)

[pzinn]

I stand by my original point.

[antonleykin]

The two ideas are good.
The second (proposed by @seangrate) to have an Error type.
The original idea to "capture message of error" (due to @d-torrance) via attempt (returning it, not storing, proposed by @mahrud).

Returning one thing---either "result" or Error---would be more natural, since our language is not statically typed (e.g., the type of the "result" is unknown anyway).

[antonleykin]

This shouldn't happen, unless the code fabricates it's own Error. I don't think we should let attempt output user-fabricated stuff.

[d-torrance]

Makes sense. I've replaced this with an error.

[antonleykin]

Emphasize that this is a keywork in the language, not a method. One could mentally parse (attempt 1)/0.
Perhaps give more examples: what would attempt 1/2; 1/0 give?

[d-torrance]

Documentation expanded

[d-torrance]

@antonleykin - I think I've addressed your comments. attempt now returns a single element, either the value of the evaluated code or an Error object.

[mahrud]

I would encourage you to reevaluate this decision. Here are a few dangers of this paradigm:

1. If a helper function constructMyError legitimately returns an Error object (we have LOTS of these helpers in Core), it's impossible to detect from err := attempt constructMyError(...) if err is a legitimate output or if constructMyError itself failed. We need Core to be robust, so I wouldn't use attempt there with this implementation.
2. Since error checking is less explicit, it's easy for users to forget to check for errors and only later end up with confusing type errors like dim myFunc() giving error: no method found for applying dim to: argument : Error {...} (of class Error)
3. Even worse, since Error inherits from hash tables, a caller can accidentally treat the output as input to a function that accepts a hash table and consequently mask the error. Here is an explicit example of this:

f := M -> attempt apply(codim M, i -> i^2);
# f(ZZ^100)      -- gives #{}         which is 0
# f((ZZ[x])^100) -- gives #Error{...} which is 2

If you're going through the trouble of adding a new keyword, I'd like it to be robust and future proof, especially for MPI or RPC type applications.

[pzinn]

I would encourage you to reevaluate this decision. Here are a few dangers of this paradigm:

1. If a helper function constructMyError legitimately returns an Error object (we have LOTS of these helpers in Core), it's impossible to detect from err := attempt constructMyError(...) if err is a legitimate output or if constructMyError itself failed. We need Core to be robust, so I wouldn't use attempt there with this implementation.

this I don't understand at all. There seems to be a basic confusion here. These Error types are produced by the interpreter itself or at the level M2 level by calling error, not by creating an error manually. If we were to do what you suggest that would be a totally different usage which would require a separate discussion -- and a complete rewrite of this PR anyway.

3. Even worse, since Error inherits from hash tables, a caller can accidentally treat the output as input to a function that accepts a hash table and consequently mask the error. Here is an explicit example of this:

f := M -> attempt apply(codim M, i -> i^2);
# f(ZZ^100)      -- gives #{}         which is 0
# f((ZZ[x])^100) -- gives #Error{...} which is 2

well, if you're going to write attempt, that means you're expecting an error, so you should test for it, otherwise you probably shouldn't use attempt in the first place.
That being said, I agree that Error probably shouldn't inherit from HashTable; instead, just like Dictionary, it should be its own type.

[mahrud]

There seems to be a basic confusion here. These Error types are produced by the interpreter itself or at the level M2 level by calling error, not by creating an error manually. If we were to do what you suggest that would be a totally different usage which would require a separate discussion

Why shouldn't error objects be produced at the top-level? Is something preventing it in this proposal? There is a huge upside, as @seangrate suggested above, in allowing custom subtypes inheriting from Error so that error checking looks more like if instance(err, RingError) ... rather than if err#"message" == "incorrect ring" .... Otherwise why bother with a new type at all?

We currently don't have an Error type, but once it exists I guarantee this PR isn't going to be the end of it. Soon we'll have extensions like Python error types and Flint error types and use it with hooks and threads and loads more that we can't imagine now. (Side note: @d-torrance if err is an instance of Error then probably error err should produce an error with that location and message)

well, if you're going to write attempt, that means you're expecting an error, so you should test for it, otherwise you probably shouldn't use attempt in the first place.

Mistakes like this will sneak in, either by people treating attempt like try or just mistakes in the programming where there are multiple branches and you forget to check if the output is an error in one branch.

More generally, I'm just surprised that we're going with a syntax like this:

c := attempt codim M;
if instance(c, Error) then
if c#"message" == "codim: expected an affine ring (consider Generic=>true to work over QQ"
then c = planZZ(M)
else error "unexpected module"

[d-torrance]

I'm convinced by @mahrud's arguments and prefer the Go-style two-element output.

[pzinn]

Why shouldn't error objects be produced at the top-level? Is something preventing it in this proposal? There is a huge upside, as @seangrate suggested above, in allowing custom subtypes inheriting from Error so that error checking looks more like if instance(err, RingError) ... rather than if err#"message" == "incorrect ring" .... Otherwise why bother with a new type at all?

because that simply wouldn't work in the current setting. what would happen if you called your function that outputs an error your way without attempt? in fact, what's the role of attempt then? As I said, if you want to go this route, then we should scrap this PR and start from scratch with a completely different design.

There absolutely should be subtypes, but this is irrelevant to the point I'm making.
I'm simply suggesting Error be its own "root" type.

[d-torrance]

I went back to the Go-style output and also added some basic support for subclassing of Error:

i1 : MyError = new SelfInitializingType of Error;

i2 : MyError "foo"

o2 = MyError{"message" => foo}

o2 : MyError

i3 : error o2
stdio:3:5:(3): error: foo

i4 : attempt error o2

o4 = (, MyError{"message" => foo                            })
                "position" => ../m2/debugging.m2:21:22-21:75

o4 : Sequence

[mahrud]

Looks like the error

"position" => ../m2/debugging.m2:21:22-21:75

is pointing to this line:

error Error := err -> olderror new class err from processArgs err#"message"

No obligation to fix this in this PR, but obviously the location should point somewhere more appropriate. There are probably multiple ways to fix it (e.g. maybe moving error(Error) to the interpreter?), but one of my hopes from the attempt keyword is to fix these spurious error layers: #2568.

[pzinn]

I understand less and less the logic. Now one can create Error objects at the M2 level, which are then fed to error, which at the d level proceeds to discard that Error object, only keeping track of the message, and creates a new Error object out of it... ??

[mahrud]

The point of object oriented error handling is that instead of interrupting the computation, an error object is created to help the subsequent code resolve it and proceed without interruption. But there may be errors objects which can't be handled, so there should be a way to force an interrupt so that the user can intervene. This is where error(Error) comes in, to simplify this:

(c, err) := attempt codim M;
if instance(err, Error) then
if instance(err, RingError)
then c = planZZ M
else error("unexpected error: " | err#"message" | " at " | err#"position")

into this:

(c, err) := attempt codim M;
if instance(err, Error) then
if instance(err, RingError)
then c = planZZ M
else error(err) -- the original message and position are returned

[d-torrance]

I made some changes that may address some of @pzinn's concerns.

Error is now a basic type rather than a type of hash table. Under the hood, a top-level Error object is a SpecialExpr that contains the lower-level Error object. So error(Error) just extracts that object rather than creating a new one.

Rather than getting the message and location by looking up keys in a hash table, we can use toString and locate, resp.

[antonleykin]

I think we are getting caught in the weeds and should tread carefully as the discussion diverged

1. from a simple (but elegant) solution to having a construct that

• handles an exception/interrupt like try (not terminating the program)
• but instead of else returning an Error object

2. to an extensive rethinking of how errors are done in Macaulay2

I would like the language modifications to be minimal unless we are constructing a new language (Macaulay3 or GoMacaulay). I believe we should encourage users to, as someone wise said,
"prefer mathematical methods to check the input first and from there run completely error free" or write a simple statement error "something is wrong" when something unexpected happen. (In particular, we'd want people to not use try, catch, etc.)

Let's try to think about minimal changes of language could be done to accomplish 1.
(The following is not a proposition to act upon. It has downsides.)
Could catch be modified so that

i2 : catch apply({-1,0,3}, i->1/i)                                                                             
stdio:2:26:(3):[1]: error: division by zero 

returns either a string that is printed or an Error object?

[d-torrance]

I think it would be straightforward to use catch instead of attempt for this. It would be a breaking change, though, as right now if you use catch on some code that raises an error, that error is raised. So I'd prefer to stick with attempt (or some other new keyword).

I don't think we should return just a string, as otherwise attempt "foo" and attempt error "foo" would be indistinguishable. And returning just the Error object is a problem too, as attempt error "foo" and attempt new Error from "foo" would also be indistinguishable.

@antonleykin - Is the "extensive rethinking of how errors are done in Macaulay2" the idea of subclassing Error? That's something we get for essentially free as soon as we expose that class at top level.

[antonleykin]

I think it would be straightforward to use catch instead of attempt for this. It would be a breaking change, though, as right now if you use catch on some code that raises an error, that error is raised.

@mikestillman and I discussed this earlier today. In .m2 files, the keywords catch and throw appear to be used... 0 times. Perhaps we have a chance to repurpose (dead wood)?

I don't think we should return just a string, as otherwise attempt "foo" and attempt error "foo" would be indistinguishable. And returning just the Error object is a problem too, as attempt error "foo" and attempt new Error from "foo" would also be indistinguishable.

This is a deficiency, but also seem to be a hypothetical edge case.
Suppose we are redesigning catch: one can make

• throw return an error object of one type (inheriting from say, AbstractError)
• any error resulting from error return an error object of a distinct type
• force the code that attempts to return an error object to fail (interrupt the program)

This would make returning a pair unnecessary. Returning a pair is, of course, not a big deal, but I think we'd be just paying tribute to the edge case (e.g., hypothetical users who would use nested catches or micromanaging error objects at the top level).

@antonleykin - Is the "extensive rethinking of how errors are done in Macaulay2" the idea of subclassing Error? That's something we get for essentially free as soon as we expose that class at top level.

My point: we can export Error to top level and subclass it anyway we want, but I would stay away from advertising this to users, and also using it ourselves (at top level) unless absolutely necessary.