search

IBM / python-sdk-core

The python-sdk-core repository contains core functionality required by Python code generated by the IBM OpenAPI SDK Generator.
Apache License 2.0
20 stars 27 forks source link

fix: detect JSON unmarshal errors in responses #157

Closed padamstx closed 1 year ago

padamstx commented 1 year ago

This commit fixes BaseService.send() so that it will now detect a JSON parsing error when processing a success (2xx) response. If, in fact, a JSON parsing error occurs while trying to process a JSON response body, then a requests.exceptions.JSONDecodeError is raised and propagated back to the caller.

padamstx commented 1 year ago

I pushed a commit that implements @ricellis's suggestion to throw an ApiException with the JSONDecodeError as the "caused by".

padamstx commented 1 year ago

Note: I'm going to hold off merging this until @ricellis's team has a chance to test with it.

emlaver commented 1 year ago

@padamstx For error responses, ApiException tries to parse the response as JSON and if there's an exception during parsing it'll return response.text: https://github.com/IBM/python-sdk-core/blob/main/ibm_cloud_sdk_core/api_exception.py#L75 I'm wondering if it's worth updating this logic to be similar to what you've done for handling malformed JSON in success responses.

padamstx commented 1 year ago

@padamstx For error responses, ApiException tries to parse the response as JSON and if there's an exception during parsing it'll return response.text: https://github.com/IBM/python-sdk-core/blob/main/ibm_cloud_sdk_core/api_exception.py#L75 I'm wondering if it's worth updating this logic to be similar to what you've done for handling malformed JSON in success responses.

Yes, I considered this but decided against it because we're already in the process of building an ApiException to reflect an actual error that was returned by the server (e.g. 400 Bad Request), so I didn't think the resulting JSONDecodeError should take precedence over that.

ibm-devx-sdk commented 1 year ago

:tada: This PR is included in version 3.16.3 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: