Skip to content
New issue

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

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

Already on GitHub? # to your account

Raise a warning when request validation causes a response validation failure #80

Merged
merged 1 commit into from
Jun 21, 2020

Conversation

MatthewWilkes
Copy link
Contributor

The request validator can raise a 400 error if the incoming request fails
the validation implied by the spec. If the spec does not allow a matching
400 response from this endpoint, that error will be lost and replaced with
a validation error for a bad response code. While we already log the relevant
information, this adds a warning to explicitly alert programmers that they
have erred with their API specification.

This was inspired by #49 and causes the following output from pytest if you write a test that hits this edge case:

pyramid_openapi3/tests/test_validation.py::TestImproperAPISpecValidation::test_request_validation_error_causes_response_validation_error
  /tmp/tmpbbfc55gb:0: ImproperAPISpecificationWarning: Discarding 400 Bad Request validation error with body [{"exception":"MissingRequiredParameter","message":"Missing required parameter: bar","field":"bar"}] as it is not a valid response for GET to /foo (foo)

Other test runners may not show the warning, but I think this is an improvement.

Note: The push pre-commit step segfaulted for me. The individual commit ones worked fine, so I think it's okay, but I wanted to flag this.

@MatthewWilkes MatthewWilkes marked this pull request as draft May 27, 2020 09:15
@MatthewWilkes
Copy link
Contributor Author

... apparently pre-commit did not catch everything! Sigh!

@MatthewWilkes MatthewWilkes force-pushed the warnings-for-discarded-validation branch from 08c617a to c75120b Compare May 27, 2020 09:19
…failure

The request validator can raise a 400 error if the incoming request fails
the validation implied by the spec. If the spec does not allow a matching
400 response from this endpoint, that error will be lost and replaced with
a validation error for a bad response code. While we already log the relevant
information, this adds a warning to explicitly alert programmers that they
have erred with their API specification.
@MatthewWilkes MatthewWilkes force-pushed the warnings-for-discarded-validation branch from c75120b to 498f449 Compare May 27, 2020 09:23
@MatthewWilkes
Copy link
Contributor Author

Okay, running pipenv run git push fixed that problem, I'd not noticed as it didn't admonish me to activate the venv. Unmarking draft.

@MatthewWilkes MatthewWilkes marked this pull request as ready for review May 27, 2020 09:28
@zupo
Copy link
Collaborator

zupo commented May 30, 2020

@MatthewWilkes: I'm on the road till next weekend, not sure when I'll be able to review it. Generally it looks good but I'd like to give it a spin locally first.

@zupo zupo merged commit 661168a into Pylons:master Jun 21, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants