Closed macisamuele closed 5 years ago
@sjaensch I've analysed alternatives to avoid shallowing JSON decoding errors in case a body parameter is optional.
The only solution that I have in mind is adding an extra expected method to the IncomingResponse
that reports if a body is present and modifying the method like
elif location == 'body':
if param.required or request.has_body():
try:
# TODO: verify content-type header
raw_value = request.json()
except ValueError as json_error:
raise SwaggerMappingError(
"Error reading request body JSON: {0}".format(str(json_error)),
)
else:
raw_value = default_value
This change could be made in a backward compatible way if we ensure that request.has_body()
does return True
but it would require changes in bravado
and pyramid_swagger
to actually fix the issue.
Considering this I would still propose to follow with the currently proposed implementation in order to fix the issue (we open the possibility to a new issue, but previously it was broken and now it would be a bit less broken) and provide PRs to define the has_body
method.
As we could do it in a different PR we could also decide to have a major version bump and not defaulting has_body
to True
to preserve compatibility
@sjaensch thanks a lot for the feedback. I was not 100% sure about the next course of action, but at least we can move a tiny bit toward a solution.
I'm planning to release it as version 5.11.0
. Do you have a different advice?
Fixes: #321
The following PR addresses the not needed assumption of body presence in case a body parameter is defined.
In case of an operation defined like
a call like
curl -X POST .../endpoint
would fail with a stacktrace likeNOTE: the current implementation will shallow eventual json validation errors in case the endpoint is called with a body containing invalid JSON
FYI: @erezeshkol