go: proposal: Improve error handing using `guard` and `must` keywords
I’ll start with an example. Here’s a version of the CopyFile example function form the proposal for improved error handling in Go 2.0, rewritting using must and guard:
func CopyFile(src, dst string) (err error) {
defer func() {
if err != nil {
err = fmt.Errorf("copy %s to %s: %v", src, dst, err)
}
}()
r := guard os.Open(src)
defer must r.Close()
w := guard os.Create(dst)
defer must w.Close()
err = io.Copy(w, r)
// here we need to do extra stuff when an Copy error happens: now we must use the 'normal' error handling method, and cannot use guard or must
if err != nil {
_ := os.Remove(dst) // fail silently if errors happen during error handling
}
return
}
The must keyword is syntactic sugar to panic on any error returned by a function call:
w := must os.Open("foo.txt")
is conceptually equivalent to
w, err := os.Open("foot.text")
if err != nil {
panic(err)
}
The must keyword can only be used on function calls that return an error as last return argument. In functions that do not return an error as last return value, or where return values are unnamed, must is exactly equivalent to the code above. In case a function does return a named error value, when a error is returned from the call, must assigns this error to function’s named return error value, then executes all deferred functions just like a normal panic, and then uses the return error value when propagating the panic upwards. As a result, a defer statement can therefore also be used to augment errors raised in a panic caused by the must keyword.
The guard keyword is syntactic sugar to return from a function when a function call returns in an error (as last argument):
func Foo() (int, string, error) {
w := guard os.Open("foo.txt")
// ....
}
is equivalent to:
func Foo() (int, string, error) {
w, err := os.Open("foot.text")
if err != nil {
return 0, "", err // guard returns zero values for all but the last return arguments
}
// ....
}
The guard keyword can be used only on function calls that return an error as last return value, in functions that also return an error as last value.
The must and guard keywords cannot be used in conjunction with the go keyword.
Benefits
The if err := ...; err != nil {} pattern has several drawbacks:
- It is so omni-present, it quickly become
noisein the code - Code like
doFileStuff(os.Open("foobar.txt"))does not work, becauseOpenalso returns an (optional) error
Both of these drawbacks would disappear (in second case either guard or must can be used). Of course there is the very valid argument that errors should be augmented with more relevant error information that only the caller can provide. In general there are three situations:
- No extra error info is needed / available
- The extra info is general for all errors that appear in the function body
- Extra info is added for a specific error
In the first case we can just use guard (or must) and we’re done. In the second case, the defer technique can be used to add function specific information (mostly the call argument values) for all errors that are returned (including panics via must).
Finally there is the case where we want to add information to a specific error only. In that case, as well in the case more has to be done than just augmenting the error, the current if err != nil {} pattern should be used: especially if more error handling code is needed there is no real reason to move this important code elsewhere as it is specific to the code directly above it.
–
As an extra benefit, with the must keyword all functions of the MustXXX() pattern are no longer needed.
Benefits over the 2.0 proposal
My main concern with the handle keyword is that it adds another code path: the error handling path. While it would be fine for error augmentation (added relative information to the error), you need to ‘read down, then read up’ checking all handle clauses to see what happens on an error. This not ideal. If any additional stuff must happen that should really be done under that function call, using the if err != nil {}: it is there where the error happens and any error handling code should be below it, not somewhere above in any of multiple of levels.
The advange of must and guard, in my opinion, over handle and check are that specific error handling code stay where it is done now, but all other cases, where we need to add general function wide info, or no info at all, can be handle much more easily and cleaner.
About this issue
- Original URL
- State: open
- Created 5 years ago
- Reactions: 56
- Comments: 21 (10 by maintainers)
I think that any error handling proposal should make it just as easy to annotate errors with additional information as it is to not annotate errors. That is true today, for example, where it is very easy to write
return fmt.Errorf(..., err). It seems less true with this proposal.@carlmjohnson It seems to me that your proposed
guardis a complicatedifcombined withtry. Considering your examples, if I understood correctly:can be written today as
And if you don’t want the
okin scope at the end, that can be finessed, too.Your ternary initializer
becomes
And finally
becomes
It is hard to see why this should be any better than writing an
ifstatement. Using anifseems clearer and - if you just count characters incl. newlines - shorter, too (becauseifis shorter thanguard). It’s clearer because the condition doesn’t need to be inverted when looking at the block. It’s also simpler because there’s no “combination” of condition orerr != nilcheck to keep in mind. Better to make the latter a separate operation usingtry. The last example reads just as well if not better withtrythan withguardand is the same size.In short, I see no benefit in making
guarda new statement. This original proposal suggestsguardas a unary operator, which I think is a much cleaner, simpler, and thus better idea. Its semantics are basically the same as the proposedtrybuilt-in.I ran some quick numbers on the kubernetes code base (per April 15 2019) to get some idea on actual error handling usage in production quality code.
This last number is interesting: there are almost 3300 errors ignored in Kubernetes (including all dependencies). These are potential bugs. Note that it only counts unchecked Close() functions and assumes all the Close methods return an error. These lines would really benefit from a ‘must’ or ‘guard’ (or check) keyword to make actually handling and not ignoring theses errors more easy. If think this is something that deserves more attention.
Notes:
If a deferred function fails, you almost certainly need local code to handle that failure, e.g. retry or log something and abort the current context. Unless the function can’t return an error any other way,
defer guard f()is probably a bug; I imagine it would be disallowed.However, a Go 2 error handling scheme is likely to provide error handlers, so could potentially offer deferred handlers: