Error unwrapping implemented incorrectly in 0.4.0

[jbowes]

A recent change Updated how validation errors implement the error Unwrap interface. The change calls errors.Unwrap() instead of returning the err field.

This is incorrect (the previous behaviour was correct) because errors.Unwrap returns either the error that the passed-in error wraps, or nil if the passed-in error doesn't implement Unwrap.

Effectively, the immediate error contained in the validation error will never be returned -- it will always be skipped over, having its wrapped error returned instead.

This matters for errors.Is and errors.As. If an error in the chain is skipped over, then the outermost error will not respond correctly to those two funcs.

[a8m]

Hey @jbowes,
The error inside the ValidationError is a fmt.wrapError that prexies the error defined in the schema with the following string: <pkg>: validator failed for field "<field>": , therefore, I found it more intuitive to return the actual error (schema error) when calling errors.Unwrap, instead of asking users to call errors.Unwrap(errors.Unwrap(err)).

What's the issue you have/found?

[jbowes]

That's fair, but I think it's tricky for two reasons:

• (what we hit) ValidationError is in a generated package end users own and control, and so we've extended it, and have access to the err field. You could easily argue this isn't supported.
• Not all code in the ent code base populates the ValdationError's err with a fmt.wrapError. The create template generates uses of ValidationError with errors.New, which will end up returning nil :(

[a8m]

Fair points. You are welcome to send a patch or I can address this later.

Thanks 🙂

[jbowes]

Thank you! We'll see who gets to it first :)