Skip to content
This repository has been archived by the owner on Jun 28, 2024. It is now read-only.

Improve error messages #168

Closed
displague opened this issue Oct 26, 2023 · 3 comments · Fixed by #169
Closed

Improve error messages #168

displague opened this issue Oct 26, 2023 · 3 comments · Fixed by #169
Labels
enhancement New feature or request

Comments

@displague
Copy link
Member

displague commented Oct 26, 2023

Today the error messages emitted only report the HTTP Status message.

422 Unrecognized Entities
https://github.com/equinix-labs/metal-go/blob/main/metal/v1/api_projects.go#L175

// format error message using title and detail when model implements rfc7807
func formatErrorMessage(status string, v interface{}) string {
str := ""
metaValue := reflect.ValueOf(v).Elem()
if metaValue.Kind() == reflect.Struct {
field := metaValue.FieldByName("Title")
if field != (reflect.Value{}) {
str = fmt.Sprintf("%s", field.Interface())
}
field = metaValue.FieldByName("Detail")
if field != (reflect.Value{}) {
str = fmt.Sprintf("%s (%s)", str, field.Interface())
}
}
return strings.TrimSpace(fmt.Sprintf("%s %s", status, str))
}

https://www.rfc-editor.org/rfc/rfc7807#section-3

In packngo the possible error bodies were converted into the error message returned, along with the HTTP status message.

422 Unrecognized Entities: foo is not valid with bar, baz is required
https://github.com/packethost/packngo/blob/master/packngo.go#L71-L81

metal-go (arguably all of the generated SDKs) should offer some examples and improved experience around accessing the reported error message(s).

@displague
Copy link
Member Author

displague commented Oct 26, 2023

Metal Errors are returned as such:

HTTP 422 Unrecognized something
Content-type: application/json

{"errors":["the explanation"]}

and sometimes: {"error": "single error"}

In the OAS3 spec, we have the Error type defined: https://github.com/equinix-labs/metal-go/blob/b1e03ba5195bd47eed1dd3db3ce38d2b494b349f/spec/oas3.fetched/components/schemas/Error.yaml

This renders to model_error.go

That type does not implement Error() string though (which would make it a go error type). The generator has no idea how to parse the errors nor how that would be presented as a Go error.

If we created a separate errors.go file in v1/ (through either patching or additional templates or a cp)

// copyrights
package v1

func (o *Error) Error() string {
  errs := []string{}
  errs = append(errs, r.Errors...)
  if r.Error != "" {
      errs = append(errs, r.Error)
  }
  return strings.Join(errs, ", ")
}

Inside of client.mustache, we would modify the error handling of formatErrorMessage so that where it currently checks for Title and Details we would preset str to Error(). Something like:

if err, ok := v.(error); ok {
  str = err.Error()
}

(We could consider removing the RFC-7807 handling portion since that was generic and not implemented in this API)

@displague displague added the enhancement New feature or request label Oct 26, 2023
@ctreatma
Copy link
Contributor

ctreatma commented Oct 26, 2023

I'm not sure we get much out of patching the Error() method into model_error.go, since we still have to wire it in somewhere else anyway and I don't think we'd expect users to be interacting directly with an error model rather than with the GenericOpenAPIError type that wraps it. IMO we could simply update formatErrorMessage to cover both of the code snippets from your suggestion, which is more straightforward to do with custom templates.

ctreatma added a commit that referenced this issue Nov 2, 2023
This updates the `formatErrorMessage` helper so that, if the API
responded with an Error model, the messages from that Error model are
appended to the status code & status description. This makes it easier
for users to get visibility into why an API call failed.

I chose to leave the existing code to support rfc7807 messages in place,
just in case we ever migrate to that standard.

Fixes #168
Copy link
Contributor

github-actions bot commented Nov 2, 2023

This issue has been resolved in version 0.26.0 🎉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants