fix(API): validate JSON input for APIError.__init__() #597
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
directly passing `r.json()` into APIError.__init__() is incorrect, as it expects a `Dict[str, str]`. instead, it should first validate that the json object is in fact the correct schema, by using a pydantic model
silentworks
changed the title
Pull Request Test Coverage Report for Build 15079111328Details
💛 - Coveralls |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What kind of change does this PR introduce?
Introduce validation that the error received on a request has the the correct JSON schema, as reported by #595.
What is the current behavior?
Currently, if a request is not successful, it will try to convert its response to JSON by using
r.json(), correctly ensuring that theJSONDecodeErroris dealt with in the try block. However, given thatr.json()returnsAny, it is not guaranteed to be aDict[str, str]as theAPIError.__init__()method expects, and thus if an error response returns a valid non-dict JSON object, such as text surrounded by quotes (a valid JSON string) the initializer will throw anAttributeErrorwhen trying to access one of the JSON object's methods.What is the new behavior?
A simple
APIErrorFromJSONpydantic model is introduced, to throw aValidationErrorin the case of the response content not being JSON, or if it is valid JSON but not a valid expected schema. This simplifies the code a little bit, asJSONDecodeErrordoes not need to be handled anymore, because it was thrown by the.json()method on theResponse. I've also added a new test to ensure that this behavior is taken into account from now on.Additional context
This could be handled by adding some check akin to
isinstance(r.json(), dict), but this only handles the first layer of the validation, and it would need to validate that the keys are also of the expected type. This is exactly whatpydanticdoes, so I believe it to be the better solution overall.