Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ServiceError.Field *string #2902

Merged
merged 1 commit into from
Sep 15, 2021

Conversation

lawrencejones
Copy link
Contributor

Some errors generated by Goa are specific to fields- this commit ensures
we populate Field of ServiceError when this is the case.

An example would be when a field has a validation like MinLength(1) or
similar, where the cause of the error is a specific part of the payload.

It's often useful to retrieve this field, so that people can use this
field to attach errors to the right form input, this being just one
important use case.

@lawrencejones
Copy link
Contributor Author

This was related to my discussion: #2901

This PR allows my custom error formatter to build richer errors, transforming the standard goa.ServiceError into this:

{
  "type": "validation_error",
  "status": 422,
  "request_id": "unknown",
  "errors": [
    {
      "code": "invalid_field",
      "message": "length of body.name must be greater or equal than 1 but got value \"\" (len=0)",
      "source": {
        "field": "name",
        "pointer": "/name"
      }
    },
    {
      "code": "invalid_field",
      "message": "length of body.description must be greater or equal than 1 but got value \"\" (len=0)",
      "source": {
        "field": "description",
        "pointer": "/description"
      }
    }
  ]
}

@raphael
Copy link
Member

raphael commented Aug 15, 2021

This looks great and could be really useful when writing custom error formatters. One thing to think about: could this introduce a backwards incompatible change? I think it should be OK given that this is adding fields and not changing or removing existing ones.

@lawrencejones
Copy link
Contributor Author

Hey @raphael, sorry for the delay in getting back to you!

One thing to think about: could this introduce a backwards incompatible change?

I don't think it can, as the original error struct should be unchanged. I've been quite careful to leave the original merging behaviour untouched, and all the new information is stored in the History only.

So I think we should be fine to merge this as a non-breaking change?

@lawrencejones lawrencejones marked this pull request as ready for review August 19, 2021 09:07
@raphael
Copy link
Member

raphael commented Aug 19, 2021

Makes sense. I see that the build is breaking because of a linting error:

pkg/error.go:210:24: methods on the same type should have the same receiver name (seen 1x "e", 2x "s") (ST1016)

Would you mind taking a look? You can run make locally to make sure the build works, thank you!

@lawrencejones
Copy link
Contributor Author

Yep, I'll take a look tomorrow. Thanks!

Some errors generated by Goa are specific to fields- this commit ensures
we populate `Field` of ServiceError when this is the case.

An example would be when a field has a validation like `MinLength(1)` or
similar, where the cause of the error is a specific part of the payload.

It's often useful to retrieve this field, so that people can use this
field to attach errors to the right form input, this being just one
important use case.

As part of this, we need to preserve the history of all virgin
(un-merged) errors whenever we merge them, so we don't get a list of all
individual errors with a gradually concatenated error message in each.
@lawrencejones
Copy link
Contributor Author

Tomorrow was extremely optimistic- I should have fixed that linting error now, though :)

@raphael
Copy link
Member

raphael commented Sep 15, 2021

Looks great, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants