Improve Problem JSON Error Format for Validation #370
Conversation
packages/http/src/types.ts
Outdated
| @@ -90,16 +102,37 @@ export class ProblemJsonError extends Error { | |||
| return error; | |||
| } | |||
|
|
|||
| public static asValidationIssue(error: Error & { detail: IPrismDiagnostic[]; status?: number }): any { | |||
There was a problem hiding this comment.
The spec for the problem/json says that effectively additional field apart from the standard one are accepted; I'd suggest, instead of creating a new function to handle the specific case, we could extend the fromTemplate function to add an additional: unknown parameter that we can add to the payload, in this way the final code should be kind of simplified.
There was a problem hiding this comment.
hm.. @XVincentX, did you mean fromTemplate or fromPlainError?
..._quantity__3__shipDate___2002-10-02T10_00_00-05_00___status___placed___complete__true__.json
Show resolved
Hide resolved
XVincentX
force-pushed
the
feat/so-197/reword-422-validation-errors
branch
from
ef2aa7b
to
11a39c3
Compare
|
@philsturgeon I took over and brought this over the finish line. I've extended the Also — for the future, the spec mandates that P.S: I'm not too happy with the way I wrote the ProblemJson implementation, but unfortunately it's required because of the way Fastify handles the the |
XVincentX
force-pushed
the
feat/so-197/reword-422-validation-errors
branch
from
11a39c3
to
f0e2209
Compare
| @@ -237,7 +231,9 @@ const createSpec = (specPath) => { | |||
|
|
|||
| expect(reqRes).toMatchObject(masterFile); | |||
|
|
|||
| expect(payload.detail).toContain("should have required property 'name'"); | |||
| expect(payload.detail).toContain( | |||
There was a problem hiding this comment.
This line is less interesting than the previous assertion: checking it is moaning about required proper name. Can we assert that this new validation property has the validation we are interested in instead?
There was a problem hiding this comment.
I'll fix it, however all this is going to be removed soon/needs to be converted to the new harness thing — is it worth to spend time on this?
There was a problem hiding this comment.
Ahh yeah sorry i didnt notice it was in there. Ignore.
|
I'll give @chris-miaskowski some time to jump into this as well |
Checklist
What kind of change does this PR introduce?
Feature.
What is the current behavior? What is the new behavior?
The current behavior gives validation error details as a JSON with a string (namely:
Your request body is not valid) concatenated together.Now, the details (422 validation errors) are just that JSON but under
validationfield.detailfield is now just a string with a description of the problem. Also, in the JSON,pathfield was renamed tolocationandseveritys field value is now represented with a string (likeError), not a number.Does this PR introduce a breaking change?
This change might involve some changes in handling Prism-generated 422 responses.