Wrap original error by goa.ServiceError

[tchssk]

I tried to implement #3058.

[raphael]

This looks good, a few questions:

• What happens for errors that do not have the err field initialized (e.g. all the validation errors)? Unwrap would return nil so As and Is would always fail? Maybe that's OK.
• Should ServiceError implement As and Is to make it more consistent? (use err if present otherwise compare the error names if the target is also a ServiceError). This is extending a bit the intent of this change but it seems to make sense.
• Should newError set the err field? Maybe not unless it helps with implementing As and Is.

[tchssk]

Thank you for your opinion.

I added As/Is to ServiceError. I think newError doesn't need to set err. It's a shorthand only used by error create functions and the functions have no error as argument.

[raphael]

Makes sense! A couple more questions:

• It seems As doesn't set the target when it's a ServiceError? seems like it should initialize the fields in this case? Also I believe the target might be **ServiceError, from the docs (emphasis mine):

An error matches target if the error's concrete value is assignable to the value pointed to by target, or if the error has a method As(interface{}) bool such that As(target) returns true. In the latter case, the As method is responsible for setting target.

So the usage would be:

res, err := client.SomeMethod(ctx, payload)
se := &ServiceError{name: "fault"}
if errors.As(err, &se) {
    log.Errorf(ctx, "faulted with error %s: %s", se.ID, se.Message)
}

As would have set the ID and Message fields of se (and the other fields of ServiceError as well).

• Should Unwrap be removed now that As and Is are implemented? If not should it check whether err is nil and if so return itself (the ServiceError) instead of nil?

Thank you!

[tchssk]

Sorry for late reply.
I reverted As/Is. Would you merge this once?

[raphael]

The latest change makes sense, I guess we now rely on go-multierror for supporting errors.As and errors.Is. Thank you!