Feedback for Go2 error handling design

errorfeedback.md

Check as an operator

I don't see why check should work like a unary operator and have its own special syntax rule instead of acting like any function call.

Go doesn't have anything like the proposed check, it has unary operators but they are all symbolic (for example !, unary * and &). It would be the first thing in the go language to look like C's sizeof.

Adding this new syntax makes parsing old code either impossible or context sensitive, for example:

check +a 

Does this parse as check(+(a)) or as (check)+(a)? In Go1 it's the latter in Go2 it would have to be the former, unless the parser first checks whether there is a check identifier defined.

On the other hand there is plenty of precedent for special builtins in Go: make and new both look like ordinary function calls even though they could never be defined as such.

I argue that check should be like make or new, not like sizeof.

Handler chains

I don't see any justification for why handler chains are necessary. IMHO requiring every handle block to terminate with a return is completely fine.

I can see how a handler chain would be useful in some circumstance but those are definitely a small minority and even in those cases I think the handler chain will make writing the code marginally easier at the expense of making reading the code harder.

Go has always been about sacrificing syntactic sugar in favor of readability. I would prefer to see no handler chains in Go2.

Going through the examples in the draft document:

func process(user string, files chan string) (n int, err error) {
    handle err { return 0, fmt.Errorf("process: %v", err)  }      // handler A
    for i := 0; i < 3; i++ {
        handle err { err = fmt.Errorf("attempt %d: %v", i, err) } // handler B
        handle err { err = moreWrapping(err) }                    // handler C

        check do(something())  // check 1: handler chain C, B, A
    }
    check do(somethingElse())  // check 2: handler chain A
}

Turns into this without handler chains, which is clearer:

func process(user string, files chan string) (n int, err error) {
    handle err { return 0, fmt.Errorf("process: %v", err)  }      // handler A
    for i := 0; i < 3; i++ {
        handle err { return 0, fmt.Errorf("process: attempt %d: %v", i, moreWrapping(err)) } // handler B

        check do(something())  // check 1: handler B
    }
    check do(somethingElse())  // check 2: handler A
}

The SortContent example already uses the single handler style and would be unchanged.

The ProcessFiles example, turns into this:

type Error struct {
	Func string
	User string
	Path string
	Err  error
}

func (e *Error) Error() string

func ProcessFiles(user string, files chan string) error {
	e := Error{ Func: "ProcessFile", User: user}
	handle err { e.Err = err; return &e } // handler A
	u := check OpenUserInfo(user)         // check 1
	defer u.Close()
	for file := range files {
		handle err { 
			e.Err = err
			e.Path = file
			return &e
		}
		check process(check os.Open(file)) // check 2
	}
	...
}

Which is slightly more verbose but I think clearer.

In cases where code sharing between handlers in a chain is substantial I'd rather see it factored out into a helper function.

In ProcessFile example,

 		handle err { 
-			e.Err
+			e.Err = err
 			e.Path = file
 			return &e
 		}

I agree. I was worried about the "magic fall-through" possibilities. One of the problems I have with try-catch is that an error that I've failed to catch explicitly can be caught by something way up in the call stack, possibly in a separate module. It doesn't have any context, doesn't know what to do with the error, and will probably lead me on a wild goose chase looking for what went wrong near the catch clause rather than where the error actually occurred.

I like Go v1 error handling because it's obvious. It's repetitive, sure, but obvious where an error is going to be handled. We need to keep that. So yeah, handler chains not good. Or at least keep them limited in scope. Go made the right choice with switch case fall-through, let's keep that decision.

I also agree with the operator syntax thing. I'd prefer thing := check(someFunc(params []Stuff)) to thing := check someFunc(params []Stuff) because the check in the second example seems loose, wobbly and disconnected to what it's doing.

@mewmew thanks

I think check and handle both become new keywords and will break any existing Go code that uses either of those to name a variable, constant, function, method, or type. They both are very generic keywords.

I agree with @mfhholmes that this has the same issues as try and catch. My first experience and annoyance with try and catch was within a single function in Python. You could not tell which of the 15 function calls actually caused the error. This will lead to lazy programming. The proposal seems to say the FAQ about exceptions got it wrong.

Agreed that check is an awkward operator, but check() hardly improves it. Wrapping nested function calls in check() is quickly unreadable. I did away with check in my alternative handler concept.

@pborman a way to select among multiple handlers (see above link) might alleviate your concern, but already we see lazy programming with ubiquitous if err != nil { return err }.

Sorry, I am using machine translation. Maybe I am wrong, but my only question is why do we need this check keyword. Why can't we just omit the last return value like a map? The compiler can know that we omitted the error. Remind us that we need handle to handle this error, so we don't need the check keyword, right?

I disagre for

It would be the first thing in the go language to look like C's sizeof.

Because check keyword have power to interupt flow control and throwing it to handle. Like defer keyword, it has power to do something after exiting func. sizeof just generate size, not have power to change the flow program. go have built in func len() for that which is not a keyword. I think it's different.

I agree with @networkimprov. I believe that check() can be seen as a traditional function call, which is not in fact. This could be confusing. I would rather prefer to keep it as an explicit operator, so that it is immediately clear that you are doing an error check.

@Duanqy, I think what you are suggesting goes against the principle of having error checking explicit. For the sake of debugging, it is much better to know where the error can come from.