Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

better handler API error parsing. #1510

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

lavigne958
Copy link
Collaborator

Possibly we encountered an error while parsing the API error JSON, like connection closes early etc and the received JSON is invalid.

Better handle such scenario and build a valid error Dict to keep the exception stack flow running and raise this message to the application.

extract the returned text so if we any bit of valid information at least we can pass that to the application.

This could be helpfull to if we add calls to the API made for web browser that returns HTML error message, it will fail to decode the recieved error and raise a proper error.

closes #1504

Possibly we encountered an error while parsing the API error JSON, like
connection closes early etc and the received JSON is invalid.

Better handle such scenario and build a valid error Dict to keep the
exception stack flow running and raise this message to the application.

extract the returned text so if we any bit of valid information at least
we can pass that to the application.

This could be helpfull to if we add calls to the API made for web
browser that returns HTML error message, it will fail to decode the
recieved error and raise a proper error.

closes #1504

Signed-off-by: Alexandre Lavigne <[email protected]>
@lavigne958 lavigne958 self-assigned this Sep 8, 2024
Copy link
Collaborator

@alifeee alifeee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me.

is there a way to test this?

@lavigne958
Copy link
Collaborator Author

I could possibly manually test it, but that's quiet hard to do. otherwise, using automatic test no 😞
that would require us to close the connection perfectly in the middle of the transfer of the response from the server, way too hard to be automatic 😆

we do not regress in anything so far.

@lavigne958 lavigne958 merged commit 6973eef into master Sep 13, 2024
10 checks passed
@lavigne958 lavigne958 deleted the bugfix/handler_errors_in_api_error_parsing branch September 13, 2024 17:14
@alifeee
Copy link
Collaborator

alifeee commented Sep 14, 2024

we could make a test that uses a HTTP client and mock the response?

@lavigne958
Copy link
Collaborator Author

lavigne958 commented Sep 14, 2024

we could make a test that uses a HTTP client and mock the response?

Possibly yes 🤔
We could use a custom http client for a single request that returns an invalid JSON.

I'm worried how this is gonna play with the cassette recorder too 🤔 it's worth the try.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exception requests.exceptions.JSONDecodeError is raised in APIError constructor
2 participants