Skip to content

Instantly share code, notes, and snippets.

@forstmeier
Last active September 3, 2018 16:55
Show Gist options
  • Save forstmeier/12f6f235fffbe8da4d0313eb2a8ee1f2 to your computer and use it in GitHub Desktop.
Save forstmeier/12f6f235fffbe8da4d0313eb2a8ee1f2 to your computer and use it in GitHub Desktop.
Comments on the proposed Go 2 error inspections

Overall, I'm in support of the suggestions being made in the draft discussions regarding inspection using the proposed Unwrap method. They seem to generally be in line with what I'd expect from Go. I'm in favor of not including the errors.Last for the reasons stated in the Open Questions and I think Unwrap is a fitting name for that method.

However, I'm less comfortable with errors.Is and errors.As. I think they could be useful, particularly in the way they are demonstrated in the draft, but I'm more concerned as to the actual internal implementation. Given errors.Is uses a perpetual for loop to look through the chain and the if err == target { return true } could evaluate to true at any point along the chain, it would be unclear as to where exactly it found a match. The root error (or the first error) is typically the one that I'm most interested in as that was the initial source of the rest of the resulting chain. By adding in the for loop that covers every part of the chain, handling could begin to be bunched up in the root function caller in a more opaque manner.

In my mind, it would make sense to have something more like this which includes a private last function:

func last(err error) bool {
	if wrapper, ok := err.(Wrapper); !ok {
		return false
	}
	if err = wrapper.Unwrap(); err == nil {
		return true
	}
}

func Is(err, target error) bool {
	for {
		if err == target && last(err) {
			return true
		}
		wrapper, ok := err.(Wrapper)
		if !ok {
			return false
		}
		err = wrapper.Unwrap()
		if err == nil {
			return false
		}
	}
}

A similar structure could be used for errors.As; optionally, if there's a way to return an indiciation of the place in the chain at which the error occured, that could be used instead.

For the print, I'd also say everything feels the way I would expect it to feel. Personally, I'd love to see source file lines included in printout via the suggested embedded helper to be used in the fmt.Errorf function rather than default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment