diff --git a/ibm_cloud_sdk_core/base_service.py b/ibm_cloud_sdk_core/base_service.py index f279a0b..1a9d72b 100644 --- a/ibm_cloud_sdk_core/base_service.py +++ b/ibm_cloud_sdk_core/base_service.py @@ -25,6 +25,7 @@ import requests from requests.structures import CaseInsensitiveDict +from requests.exceptions import JSONDecodeError from ibm_cloud_sdk_core.authenticators import Authenticator from .api_exception import ApiException @@ -32,6 +33,7 @@ from .token_managers.token_manager import TokenManager from .utils import ( has_bad_first_or_last_char, + is_json_mimetype, remove_null_values, cleanup_values, read_external_sources, @@ -310,21 +312,32 @@ def send(self, request: requests.Request, **kwargs) -> DetailedResponse: try: response = self.http_client.request(**request, cookies=self.jar, **kwargs) + # Process a "success" response. if 200 <= response.status_code <= 299: if response.status_code == 204 or request['method'] == 'HEAD': - # There is no body content for a HEAD request or a 204 response + # There is no body content for a HEAD response or a 204 response. result = None elif stream_response: result = response elif not response.text: result = None - else: + elif is_json_mimetype(response.headers.get('Content-Type')): + # If this is a JSON response, then try to unmarshal it. try: result = response.json() - except: - result = response + except JSONDecodeError as err: + raise ApiException( + code=response.status_code, + http_response=response, + message='Error processing the HTTP response', + ) from err + else: + # Non-JSON response, just use response body as-is. + result = response + return DetailedResponse(response=result, headers=response.headers, status_code=response.status_code) + # Received error status code from server, raise an APIException. raise ApiException(response.status_code, http_response=response) except requests.exceptions.SSLError: logger.exception(self.ERROR_MSG_DISABLE_SSL) diff --git a/ibm_cloud_sdk_core/detailed_response.py b/ibm_cloud_sdk_core/detailed_response.py index 1eb94e8..4987247 100644 --- a/ibm_cloud_sdk_core/detailed_response.py +++ b/ibm_cloud_sdk_core/detailed_response.py @@ -26,7 +26,7 @@ class DetailedResponse: Keyword Args: response: The response to the service request, defaults to None. headers: The headers of the response, defaults to None. - status_code: The status code of there response, defaults to None. + status_code: The status code of the response, defaults to None. Attributes: result (dict, requests.Response, None): The response to the service request. diff --git a/ibm_cloud_sdk_core/utils.py b/ibm_cloud_sdk_core/utils.py index 7dcbfeb..61fb89e 100644 --- a/ibm_cloud_sdk_core/utils.py +++ b/ibm_cloud_sdk_core/utils.py @@ -16,6 +16,7 @@ # from ibm_cloud_sdk_core.authenticators import Authenticator import datetime import json as json_import +import re import ssl from os import getenv, environ, getcwd from os.path import isfile, join, expanduser @@ -395,3 +396,19 @@ def __read_from_vcap_services(service_name: str) -> dict: new_vcap_creds['APIKEY'] = vcap_service_credentials.get('apikey') vcap_service_credentials = new_vcap_creds return vcap_service_credentials + + +# A regex that matches an "application/json" mimetype. +json_mimetype_pattern = re.compile('^application/json(\\s*;.*)?$') + + +def is_json_mimetype(mimetype: str) -> bool: + """Returns true if 'mimetype' is a JSON-like mimetype, false otherwise. + + Args: + mimetype: The mimetype to check. + + Returns: + true if mimetype is a JSON-line mimetype, false otherwise. + """ + return mimetype is not None and json_mimetype_pattern.match(mimetype) is not None diff --git a/test/test_base_service.py b/test/test_base_service.py index bf456f0..cffe575 100644 --- a/test/test_base_service.py +++ b/test/test_base_service.py @@ -345,19 +345,20 @@ def test_has_bad_first_or_last_char(): @responses.activate def test_request_server_error(): - responses.add( - responses.GET, - 'https://gateway.watsonplatform.net/test/api', - status=500, - body=json.dumps({'error': 'internal server error'}), - content_type='application/json', - ) - service = AnyServiceV1('2018-11-20', authenticator=NoAuthAuthenticator()) - try: + with pytest.raises(ApiException, match=r'internal server error') as err: + responses.add( + responses.GET, + 'https://gateway.watsonplatform.net/test/api', + status=500, + body=json.dumps({'error': 'internal server error'}), + content_type='application/json', + ) + service = AnyServiceV1('2018-11-20', authenticator=NoAuthAuthenticator()) prepped = service.prepare_request('GET', url='') service.send(prepped) - except ApiException as err: - assert err.message == 'internal server error' + assert err.value.code == 500 + assert err.value.http_response.headers['Content-Type'] == 'application/json' + assert err.value.message == 'internal server error' @responses.activate @@ -382,6 +383,27 @@ def test_request_success_json(): assert detailed_response.get_result() == {'foo': 'bar'} +@responses.activate +def test_request_success_invalid_json(): + # expect an ApiException with JSONDecodeError as the cause when a "success" + # response contains invalid JSON in response body. + with pytest.raises(ApiException, match=r'Error processing the HTTP response') as err: + responses.add( + responses.GET, + 'https://gateway.watsonplatform.net/test/api', + status=200, + body='{ "invalid": "json", "response"', + content_type='application/json; charset=utf8', + ) + service = AnyServiceV1('2018-11-20', authenticator=NoAuthAuthenticator()) + prepped = service.prepare_request('GET', url='') + service.send(prepped) + assert err.value.code == 200 + assert err.value.http_response.headers['Content-Type'] == 'application/json; charset=utf8' + assert isinstance(err.value.__cause__, requests.exceptions.JSONDecodeError) + assert "Expecting ':' delimiter: line 1" in str(err.value.__cause__) + + @responses.activate def test_request_success_response(): responses.add( @@ -398,20 +420,101 @@ def test_request_success_response(): @responses.activate -def test_request_fail_401(): +def test_request_success_nonjson(): responses.add( responses.GET, 'https://gateway.watsonplatform.net/test/api', - status=401, - body=json.dumps({'foo': 'bar'}), - content_type='application/json', + status=200, + body='

Hola, amigo!

', + content_type='text/html', ) service = AnyServiceV1('2018-11-20', authenticator=NoAuthAuthenticator()) - try: + prepped = service.prepare_request('GET', url='') + detailed_response = service.send(prepped) + # It's odd that we have to call ".text" to get the string value + # (see issue 3557) + assert detailed_response.get_result().text == '

Hola, amigo!

' + + +@responses.activate +def test_request_fail_401_nonerror_json(): + # response body not an error object, so we expect the default error message. + error_msg = 'Unauthorized: Access is denied due to invalid credentials' + with pytest.raises(ApiException, match=error_msg) as err: + responses.add( + responses.GET, + 'https://gateway.watsonplatform.net/test/api', + status=401, + body=json.dumps({'foo': 'bar'}), + content_type='application/json', + ) + service = AnyServiceV1('2018-11-20', authenticator=NoAuthAuthenticator()) + prepped = service.prepare_request('GET', url='') + service.send(prepped) + assert err.value.code == 401 + assert err.value.http_response.headers['Content-Type'] == 'application/json' + assert err.value.message == error_msg + + +@responses.activate +def test_request_fail_401_error_json(): + # response body is an error object, so we expect to get the message from there. + error_msg = 'You dont need to know...' + with pytest.raises(ApiException, match=error_msg) as err: + responses.add( + responses.GET, + 'https://gateway.watsonplatform.net/test/api', + status=401, + body=json.dumps({'message': error_msg}), + content_type='application/json', + ) + service = AnyServiceV1('2018-11-20', authenticator=NoAuthAuthenticator()) + prepped = service.prepare_request('GET', url='') + service.send(prepped) + assert err.value.code == 401 + assert err.value.http_response.headers['Content-Type'] == 'application/json' + assert err.value.message == error_msg + + +@responses.activate +def test_request_fail_401_nonjson(): + response_body = 'You dont have a need to know...' + with pytest.raises(ApiException, match=response_body) as err: + responses.add( + responses.GET, + 'https://gateway.watsonplatform.net/test/api', + status=401, + body=response_body, + content_type='text/plain', + ) + service = AnyServiceV1('2018-11-20', authenticator=NoAuthAuthenticator()) + prepped = service.prepare_request('GET', url='') + service.send(prepped) + assert err.value.code == 401 + assert err.value.http_response.headers['Content-Type'] == 'text/plain' + assert err.value.message == response_body + + +@responses.activate +def test_request_fail_401_badjson(): + # if an error response contains invalid JSON, then we should + # end up with 'Unknown error' as the message since we couldn't get + # the actual error message from the response body. + response_body = 'This is not a JSON object' + with pytest.raises(ApiException, match=response_body) as err: + responses.add( + responses.GET, + 'https://gateway.watsonplatform.net/test/api', + status=401, + body=response_body, + content_type='application/json', + ) + service = AnyServiceV1('2018-11-20', authenticator=NoAuthAuthenticator()) prepped = service.prepare_request('GET', url='') service.send(prepped) - except ApiException as err: - assert err.message == 'Unauthorized: Access is denied due to invalid credentials' + assert err.value.code == 401 + assert err.value.http_response.headers['Content-Type'] == 'application/json' + assert err.value.message == response_body def test_misc_methods(): diff --git a/test/test_utils.py b/test/test_utils.py index ddd5d7e..f9c9425 100644 --- a/test/test_utils.py +++ b/test/test_utils.py @@ -28,7 +28,7 @@ from ibm_cloud_sdk_core import get_query_param from ibm_cloud_sdk_core import read_external_sources from ibm_cloud_sdk_core.authenticators import Authenticator, BasicAuthenticator, IAMAuthenticator -from ibm_cloud_sdk_core.utils import strip_extra_slashes +from ibm_cloud_sdk_core.utils import strip_extra_slashes, is_json_mimetype def datetime_test(datestr: str, expected: str): @@ -605,3 +605,15 @@ def test_strip_extra_slashes(): assert strip_extra_slashes('https://host/path//') == 'https://host/path/' assert strip_extra_slashes('https://host//path//') == 'https://host//path/' assert strip_extra_slashes('https://host//path//////////') == 'https://host//path/' + + +def test_is_json_mimetype(): + assert is_json_mimetype(None) is False + assert is_json_mimetype('') is False + assert is_json_mimetype('application/octet-stream') is False + assert is_json_mimetype('ApPlIcAtION/JsoN') is False + assert is_json_mimetype('applicaiton/json; charset=utf8') is False + assert is_json_mimetype('fooapplication/json; charset=utf8; foo=bar') is False + + assert is_json_mimetype('application/json') is True + assert is_json_mimetype('application/json; charset=utf8') is True