feat(rest): improve validation errors for invalid parameter value #6105
Conversation
bajtos
added
feature
Validation
REST
| export function invalidRequestBody(): HttpErrors.HttpError { | ||
|
|
||
| export function invalidRequestBody( | ||
| details: ValidationErrorDetails[], |
There was a problem hiding this comment.
would there be backward compatibility issue for adding a parameter?
There was a problem hiding this comment.
Good catch!
In the strict interpretation, this is a breaking change because existing callers of invalidRequestBody must be updated. However, I consider invalidRequestBody as an internal helper that's not a part of our public API, therefore I think it's ok to make this change in a semver-minor release.
|
What about tests? |
See PR description:
Do you have any suggestions on where to put the new tests? |
|
@bajtos A test showing the expected behavior for invalid parameter values. If the code works the same for "valid" and "invalid" values, it means there is no distinction between them for the program. It seems to me, we'll have to dig deeper and make some changes somewhere, and add a test that will fail for invalid values. |
Rework the validation code to use exiting `RestHttpErrors.invalidData` error instead of a custom `BadRequest` error. This way the error object includes the parameter name in the error message & properties and has a machine-readable `code` property. Signed-off-by: Miroslav Bajtoš <[email protected]>
bajtos
force-pushed
the
fix/query-param-validation-errors
branch
from
28d375f to
84f3d7f
Compare
|
@hacksparrow added a new test, see https://github.com/strongloop/loopback-next/pull/6105/files#diff-f1805dfae45a4ba243a038f5794b800cR257 LGTY now? |
Rework the validation code to use exiting
RestHttpErrors.invalidDataerror instead of a customBadRequesterror. This way the error object includes the parameter name in the error message & properties and has a machine-readablecodeproperty.See also #5808 which introduced parameter coercion.
Surprisingly, all existing tests are passing with my change in place, so I didn't make any changes in the test suite. Please let me know if you think I should write a new test 🤷🏻
Checklist
👉 Read and sign the CLA (Contributor License Agreement) 👈
npm testpasses on your machinepackages/cliwere updatedexamples/*were updated👉 Check out how to submit a PR 👈