Support for Go 1.13 Error Wrapping

[krak3n]

Hi

I was wondering if you had any interest in support for go1.13 error wrapping? For example:

const (
	ErrIncorrectLength Error = iota + 1
	// Other sentinel error types here
)

type Error int

func (e Error) Error() string {
	switch e {
	case ErrIncorrectLength:
		return "incorrect length"
	// Other error strings here
	default:
		return "unknown error"
	}
}

func (u *UUID) UnmarshalText(text []byte) error {
	switch len(text) {
	// Previous switch conditions
	default:
		return fmt.Errorf("uuid: %w %d in string %q", ErrIncorrectLength, len(text), text)
	}
}

This would allow for the usage of errors.Is() and errors.As(), useful for testing or performing other conditional logic based on the type of error, for example:

id, err := uuid.FromString("foo")
if err != nil {
	if errors.Is(err, uuid.ErrIncorrectLength) {
		// some bespoke logic
	}
}

If you approve I'd be more than happy to open a PR with the implementation detail.If backwards compatibility is a concern we can probably get around it with build tags.

Demo over on the playground: https://play.golang.org/p/H5teBSdvfct

[krak3n]

https://github.com/gofrs/uuid/blob/master/codec.go#L117 does not use error wrapping as my example above.

[niaow]

Wait sorry i mixed up what repo i was looking at oops.

[krak3n]

So what do you think @jaddr2line is it worth me working in a PR for it?

[theckman]

@krak3n I think this could be an interesting idea, but before expanding the API I'd love to understand how these errors could be handled. At least for the one provided, if the input is invalid there really isn't a way to handle that beyond logging or presenting it to the end user.

[krak3n]

@theckman indeed and ultimately if the end-user is just printing err.Error() then the underlying behaviour is no different, there error string presented to the user in my example above is the same as the one that exists currently. However, now that Go offers a way to wrap errors in error stings and return a wrapped sentinel error (as shown above) or a concrete struct type (like os.PathError) we can offer the end-user some more flexibility in how to handle specific types of errors.

You can see this happening more in the standard library as support for errors.Is/As is rolled out via the Unwrap interface, for example here https://golang.org/pkg/os/#PathError.Unwrap and https://golang.org/pkg/strconv/#NumError.Unwrap. Whilst it's rare that you'll use these the option is now there for end-user to gather more information about an error and/or perform any conditional logic they may want to.

Let's say for example I've written a HTTP API that takes some JSON as it's input, one of the attributes on the JSON is a UUID but the user fills in a bad UUID so json.Unmarshal will return an error, we can't know what that error is so we are forced to return a 500 or similar back to the user, but with error wrapping and something like I have suggested we can now know what went wrong and return something a little bit better to the user, maybe a 400 or a 422:

type Request struct {
	ID uuid.UUID `json:"id"`
}

func Handler(w http.ResponseWriter, r *http.Request) {
	b, _ := ioutil.ReadAll(r.Body) // omitting error handling

	var req Request

	if err := json.Unmarshal(b, &req); err != nil {
		switch {
		case errors.Is(err, uuid.ErrIncorrectLength):
			w.WriteHeader(http.StatusUnprocessableEntity)
			w.Write([]byte(fmt.Sprintf("invalid uuid: %s". err)))
		case errors.Is(err, ErrSomeOther):
			w.WriteHeader(http.StatusBadRequest)
		default:
			w.WriteHeader(http.StatusInternalServerError)
		}
	}

	// Continue as normal
}

This pattern also allows for better error testing, instead of comparing error strings or asserting that an error is returned we can check the error is what we expect:

id,err := FromString("foo")
if !errors.Is(err, ErrIncorrectLength) {
	t.Errorf("want %v got %v", ErrIncorrectLength, err)
}

This also allows for self-documenting errors, users can now see what errors this package can return and choose how to handle them, or not.

[PaluMacil]

I use error wrapping all the time to recognize specific errors. What if I am working on a website and this error is returned from your library? Right now I need to search the error text for specific strings to decide if it's this error since I can't match a single static string, and it could get worse as the error is wrapped with other text at other levels. However, if I can check if it's this specific error, I know it's a 400, not a 500.

The expansion of the API is minor. You'd be returning something implementing the same error interface as before, and there would be no breaking changes to any code.

[cameracker]

I would be in favor of this change!

[krak3n]

Sill willing to work on this if everyone is up for it 👍

[cameracker]

This is closed with #131 😃

Thanks for the suggestion!