Skip to content

Commit

Permalink
raise ClientResponseError from response.json() #1723
Browse files Browse the repository at this point in the history
  • Loading branch information
fafhrd91 committed Mar 17, 2017
1 parent f7b8ef8 commit 8974844
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 24 deletions.
3 changes: 3 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ CHANGES

- Added session's `raise_for_status` parameter, automatically calls raise_for_status() on any request. #1724

- `response.json()` raises `ClientReponseError` exception if response's
content type does not match #1723

- Cleanup timer and loop handle on any client exception.


Expand Down
24 changes: 14 additions & 10 deletions aiohttp/client_reqrep.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
from multidict import CIMultiDict, CIMultiDictProxy, MultiDict, MultiDictProxy
from yarl import URL

import aiohttp

from . import hdrs, helpers, http, payload
from .client_exceptions import (ClientConnectionError, ClientOSError,
ClientResponseError)
from .formdata import FormData
from .helpers import PY_35, HeadersMixin, SimpleCookie, TimerNoop, noop
from .http import SERVER_SOFTWARE, HttpVersion10, HttpVersion11, PayloadWriter
Expand Down Expand Up @@ -323,7 +323,7 @@ def write_bytes(self, writer, conn):

yield from writer.write_eof()
except OSError as exc:
new_exc = aiohttp.ClientOSError(
new_exc = ClientOSError(
exc.errno,
'Can not write request body for %s' % self.url)
new_exc.__context__ = exc
Expand Down Expand Up @@ -606,7 +606,7 @@ def release(self):

def raise_for_status(self):
if 400 <= self.status:
raise aiohttp.ClientResponseError(
raise ClientResponseError(
code=self.status,
message=self.reason,
headers=self.headers)
Expand All @@ -620,7 +620,7 @@ def _notify_content(self):
content = self.content
if content and content.exception() is None and not content.is_eof():
content.set_exception(
aiohttp.ClientConnectionError('Connection closed'))
ClientConnectionError('Connection closed'))

@asyncio.coroutine
def wait_for_close(self):
Expand Down Expand Up @@ -671,15 +671,19 @@ def text(self, encoding=None, errors='strict'):
return self._content.decode(encoding, errors=errors)

@asyncio.coroutine
def json(self, *, encoding=None, loads=json.loads):
def json(self, *, encoding=None, loads=json.loads,
content_type='application/json'):
"""Read and decodes JSON response."""
if self._content is None:
yield from self.read()

ctype = self.headers.get(hdrs.CONTENT_TYPE, '').lower()
if 'json' not in ctype:
client_logger.warning(
'Attempt to decode JSON with unexpected mimetype: %s', ctype)
if content_type:
ctype = self.headers.get(hdrs.CONTENT_TYPE, '').lower()
if content_type not in ctype:

This comment has been minimized.

Copy link
@socketpair

socketpair Mar 17, 2017

Contributor

why not startswith() ? seems, it should be faster.

This comment has been minimized.

Copy link
@fafhrd91

fafhrd91 Mar 17, 2017

Author Member

ok.
I am not sure yet about usage of this api. may it should accept list of content types

This comment has been minimized.

Copy link
@fafhrd91

fafhrd91 Mar 17, 2017

Author Member

I leave it for now, will change in next version.

This comment has been minimized.

Copy link
@socketpair

socketpair Mar 17, 2017

Contributor

I think, list of content-types will be superfluous.

https://www.ietf.org/rfc/rfc4627.txt

This comment has been minimized.

Copy link
@fafhrd91

fafhrd91 Mar 17, 2017

Author Member

I would agree for server code, but client needs to work with many different web services.

This comment has been minimized.

Copy link
@socketpair

socketpair Mar 17, 2017

Contributor

Well, I will try to find any, which set wrong content-type or not set at all. I think there are no such services that are still alive.

This comment has been minimized.

Copy link
@fafhrd91

fafhrd91 Mar 17, 2017

Author Member

Ok, if nobody complain next couple weeks I will change this to startwith

raise ClientResponseError(
message=('Attempt to decode JSON with '
'unexpected mimetype: %s' % ctype),
headers=self.headers)

stripped = self._content.strip()
if not stripped:
Expand Down
17 changes: 11 additions & 6 deletions docs/client_reference.rst
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ The client session supports the context manager protocol for self closing.

.. versionchanged:: 1.0

``.cookies`` attribute was dropped. Use :attr:`cookie_jar`
``.cookies`` attribute was dropped. Use :attr:`cookie_jar`
instead.

.. attribute:: closed
Expand Down Expand Up @@ -1024,7 +1024,7 @@ Response object

:return str: decoded *BODY*

.. comethod:: json(encoding=None, loads=json.loads)
.. comethod:: json(encoding=None, loads=json.loads, content_type='application/json')

Read response's body as *JSON*, return :class:`dict` using
specified *encoding* and *loader*.
Expand All @@ -1033,8 +1033,9 @@ Response object
using :term:`cchardet` or :term:`chardet` as fallback if
*cchardet* is not available.

Close underlying connection if data reading gets an error,
release connection otherwise.
if response's `content-type` does not match `content_type` parameter
:exc:`aiohttp.ClientResponseError` get raised. To disable content type
check pass ``None`` value.

:param str encoding: text encoding used for *BODY* decoding, or
``None`` for encoding autodetection
Expand All @@ -1043,9 +1044,13 @@ Response object
:param callable loads: :func:`callable` used for loading *JSON*
data, :func:`json.loads` by default.

:param str content_type: specify response's content-type, if content type
does not match raise :exc:`aiohttp.ClientResponseError`.
To disable `content-type` check, pass ``None`` as value.
(default: `application/json`).

:return: *BODY* as *JSON* data parsed by *loads* parameter or
``None`` if *BODY* is empty or contains white-spaces
only.
``None`` if *BODY* is empty or contains white-spaces only.


ClientWebSocketResponse
Expand Down
6 changes: 5 additions & 1 deletion docs/migration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,11 @@ Various

5. `ClientResponse.raise_for_status()` raises :exc:`aiohttp.ClientResponseError` exception

6. `ClientSession.close()` is a regular function returning None, not a coroutine.
6. `ClientResponse.json()` is strict about response's content type, is content type

This comment has been minimized.

Copy link
@socketpair

socketpair Mar 17, 2017

Contributor

is content -> if content

This comment has been minimized.

Copy link
@fafhrd91

fafhrd91 Mar 17, 2017

Author Member

thanks

does not match it raises :exc:`aiohttp.ClientResponseError` exception.
To disable content type check you can pass ``None`` as `content_type` parameter.

7. `ClientSession.close()` is a regular function returning None, not a coroutine.



Expand Down
4 changes: 2 additions & 2 deletions tests/test_client_functional.py
Original file line number Diff line number Diff line change
Expand Up @@ -646,14 +646,14 @@ def handler(request):
resp = web.StreamResponse(headers={'content-length': '100'})
yield from resp.prepare(request)
yield from resp.drain()
yield from asyncio.sleep(0.1, loop=loop)
yield from asyncio.sleep(0.2, loop=loop)
return resp

app = web.Application(loop=loop)
app.router.add_route('GET', '/', handler)
client = yield from test_client(app)

resp = yield from client.get('/', timeout=0.01)
resp = yield from client.get('/', timeout=0.05)

with pytest.raises(asyncio.TimeoutError):
yield from resp.read()
Expand Down
14 changes: 9 additions & 5 deletions tests/test_client_response.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,12 @@ def test_repr_non_ascii_reason():
in repr(response)


def test_url_obj_deprecated():
response = ClientResponse('get', URL('http://fake-host.org/'))
with pytest.warns(DeprecationWarning):
response.url_obj


@asyncio.coroutine
def test_read_and_release_connection(loop):
response = ClientResponse('get', URL('http://def-cl-resp.org'))
Expand Down Expand Up @@ -314,13 +320,11 @@ def test_json_no_content(loop):
'Content-Type': 'data/octet-stream'}
response._content = b''

with mock.patch('aiohttp.client_reqrep.client_logger') as m_log:
res = yield from response.json()
with pytest.raises(aiohttp.ClientResponseError):
yield from response.json()

res = yield from response.json(content_type=None)
assert res is None
m_log.warning.assert_called_with(
'Attempt to decode JSON with unexpected mimetype: %s',
'data/octet-stream')


@asyncio.coroutine
Expand Down

0 comments on commit 8974844

Please sign in to comment.