From 746dfb84784e31bb45fb3721cbe9a001c4e3f14d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antti=20Nyk=C3=A4nen?= Date: Sun, 16 Feb 2020 17:08:56 +0200 Subject: [PATCH 1/2] #24: Fix error handling of server response --- src/jsonapi_client/common.py | 4 +- src/jsonapi_client/session.py | 24 +++---- tests/test_client.py | 115 ++++++++++++++++++++++++++++++++++ 3 files changed, 130 insertions(+), 13 deletions(-) diff --git a/src/jsonapi_client/common.py b/src/jsonapi_client/common.py index f62b849..c97c318 100644 --- a/src/jsonapi_client/common.py +++ b/src/jsonapi_client/common.py @@ -97,9 +97,9 @@ def mark_invalid(self): self._invalid = True -def error_from_response(response): +def error_from_response(response_content): try: - error_str = response.json()['errors'][0]['title'] + error_str = response_content['errors'][0]['title'] except Exception: error_str = '?' return error_str diff --git a/src/jsonapi_client/session.py b/src/jsonapi_client/session.py index 8ebf125..4732c95 100644 --- a/src/jsonapi_client/session.py +++ b/src/jsonapi_client/session.py @@ -486,12 +486,13 @@ def _fetch_json(self, url: str) -> dict: parsed_url = urlparse(url) logger.info('Fetching document from url %s', parsed_url) response = requests.get(parsed_url.geturl(), **self._request_kwargs) + response_content = response.json() if response.status_code == HttpStatus.OK_200: - return response.json() + return response_content else: raise DocumentError(f'Error {response.status_code}: ' - f'{error_from_response(response)}', + f'{error_from_response(response_content)}', errors={'status_code': response.status_code}, response=response) @@ -506,12 +507,13 @@ async def _fetch_json_async(self, url: str) -> dict: logger.info('Fetching document from url %s', parsed_url) async with self._aiohttp_session.get(parsed_url.geturl(), **self._request_kwargs) as response: + response_content = await response.json(content_type='application/vnd.api+json') if response.status == HttpStatus.OK_200: - return await response.json(content_type='application/vnd.api+json') + return response_content else: - raise DocumentError(f'Error {response.status_code}: ' - f'{error_from_response(response)}', - errors={'status_code': response.status_code}, + raise DocumentError(f'Error {response.status}: ' + f'{error_from_response(response_content)}', + errors={'status_code': response.status}, response=response) def http_request(self, http_method: str, url: str, send_json: dict, @@ -533,15 +535,16 @@ def http_request(self, http_method: str, url: str, send_json: dict, headers=headers, **kwargs) + response_json = response.json() if response.status_code not in expected_statuses: raise DocumentError(f'Could not {http_method.upper()} ' f'({response.status_code}): ' - f'{error_from_response(response)}', + f'{error_from_response(response_json)}', errors={'status_code': response.status_code}, response=response, json_data=send_json) - return response.status_code, response.json() \ + return response.status_code, response_json \ if response.content \ else {}, response.headers.get('Location') @@ -570,16 +573,15 @@ async def http_request_async( headers=headers, **kwargs) as response: + response_json = await response.json(content_type=content_type) if response.status not in expected_statuses: raise DocumentError(f'Could not {http_method.upper()} ' f'({response.status}): ' - f'{error_from_response(response)}', + f'{error_from_response(response_json)}', errors={'status_code': response.status}, response=response, json_data=send_json) - response_json = await response.json(content_type=content_type) - return response.status, response_json or {}, response.headers.get('Location') @property diff --git a/tests/test_client.py b/tests/test_client.py index 825d311..ba08d07 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1,8 +1,12 @@ from unittest.mock import Mock from urllib.parse import urlparse +from yarl import URL +from aiohttp import ClientResponse +from aiohttp.helpers import TimerNoop import jsonschema import pytest +from requests import Response import json import os from jsonschema import ValidationError @@ -214,6 +218,11 @@ def mock_update_resource(mocker): return m +@pytest.fixture +def session(): + return mock.Mock() + + def test_initialization(mocked_fetch, article_schema): s = Session('http://localhost:8080', schema=article_schema) article = s.get('articles') @@ -740,6 +749,7 @@ async def test_more_relationships_async_fetch(mocked_fetch, api_schema): # ^ now parent lease is fetched, but attribute access goes through Relationship await s.close() + class SuccessfullResponse: status_code = 200 headers = {} @@ -1647,3 +1657,108 @@ async def test_posting_async_with_custom_header(): assert args[1]['headers']['X-Test'] == 'test' assert args[1]['something'] == 'else' patcher.stop() + + +def test_error_handling_get(): + response = Response() + response.url = URL('http://localhost:8080/invalid') + response.request = mock.Mock() + response.headers = {'Content-Type': 'application/vnd.api+json'} + response._content = json.dumps({'errors': [{'title': 'Resource not found'}]}).encode('UTF-8') + response.status_code = 404 + + patcher = mock.patch('requests.get') + client_mock = patcher.start() + s = Session('http://localhost', schema=leases) + client_mock.return_value = response + with pytest.raises(DocumentError) as exp: + s.get('invalid') + + assert str(exp.value) == 'Error 404: Resource not found' + patcher.stop() + + +def test_error_handling_post(): + response = Response() + response.url = URL('http://localhost:8080/invalid') + response.request = mock.Mock() + response.headers = {'Content-Type': 'application/vnd.api+json'} + response._content = json.dumps({'errors': [{'title': 'Internal server error'}]}).encode('UTF-8') + response.status_code = 500 + + patcher = mock.patch('requests.request') + client_mock = patcher.start() + s = Session('http://localhost', schema=leases) + client_mock.return_value = response + a = s.create('leases') + assert a.is_dirty + a.lease_id = '1' + a.active_status = 'pending' + a.reference_number = 'test' + with pytest.raises(DocumentError) as exp: + a.commit() + + assert str(exp.value) == 'Could not POST (500): Internal server error' + patcher.stop() + + +@pytest.mark.asyncio +async def test_error_handling_async_get(loop, session): + response = ClientResponse('get', URL('http://localhost:8080/invalid'), + request_info=mock.Mock(), + writer=mock.Mock(), + continue100=None, + timer=TimerNoop(), + traces=[], + loop=loop, + session=session, + ) + response._headers = {'Content-Type': 'application/vnd.api+json'} + response._body = json.dumps({'errors': [{'title': 'Resource not found'}]}).encode('UTF-8') + response.status = 404 + + patcher = mock.patch('aiohttp.ClientSession') + client_mock = patcher.start() + s = Session('http://localhost', schema=leases, enable_async=True) + client_mock().get.return_value = response + with pytest.raises(DocumentError) as exp: + await s.get('invalid') + + assert str(exp.value) == 'Error 404: Resource not found' + patcher.stop() + + +@pytest.mark.asyncio +async def test_error_handling_posting_async(loop, session): + response = ClientResponse('post', URL('http://localhost:8080/leases'), + request_info=mock.Mock(), + writer=mock.Mock(), + continue100=None, + timer=TimerNoop(), + traces=[], + loop=loop, + session=session, + ) + response._headers = {'Content-Type': 'application/vnd.api+json'} + response._body = json.dumps({'errors': [{'title': 'Internal server error'}]}).encode('UTF-8') + response.status = 500 + + patcher = mock.patch('aiohttp.ClientSession.request') + request_mock = patcher.start() + s = Session( + 'http://localhost:8080', + schema=api_schema_all, + enable_async=True + ) + request_mock.return_value = response + + a = s.create('leases') + assert a.is_dirty + a.lease_id = '1' + a.active_status = 'pending' + a.reference_number = 'test' + with pytest.raises(DocumentError) as exp: + await a.commit() + + assert str(exp.value) == 'Could not POST (500): Internal server error' + patcher.stop() From c5205a3fdd1e3e47d88d1a52be427b90162a18d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antti=20Nyk=C3=A4nen?= Date: Sun, 16 Feb 2020 17:26:41 +0200 Subject: [PATCH 2/2] Add pytest-aiohttp to requirements.txt --- requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/requirements.txt b/requirements.txt index 3886023..ce222c1 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,6 +2,7 @@ requests pytest-asyncio pytest-mock pytest +pytest-aiohttp asynctest jsonschema aiohttp