Cannot access an error message when receive 400

[yeralin]

🐞 Describe the bug
While using aiohttp.client.ClientSession to send requests and receive responses, I found out that I cannot access a bad response's error message while catching ClientResponseError because aiohttp simply does not propagate the response content when raising ClientResponseError here:

aiohttp/aiohttp/client_reqrep.py

Lines 879 to 889 in 63a0d10

	def raise_for_status(self) -> None:
	if 400 <= self.status:
	# reason should always be not None for a started response
	assert self.reason is not None
	self.release()
	raise ClientResponseError(
	self.request_info,
	self.history,
	status=self.status,
	message=self.reason,
	headers=self.headers)

It only passes self.status == 400 and self.reason == "BAD REQUEST".

💡 To Reproduce

from aiohttp import ClientResponseError, ClientSession

session = ClientSession(raise_for_status=True)

try:
    # assuming that the request will result in 400 BAD REQUEST with a specific error message
    async with session.post(**kwargs) as response: 
        # handle response
except ClientResponseError as ex:
    print(ex.__dict__) 
    """
        {'request_info': RequestInfo(...), 
        'status': 400, 
        'message': 'BAD REQUEST', 
        'headers': ..., 
        'history': ()}
    """

💡 Expected behavior
Received ClientResponseError should contain a response message received along with 400 response.

📋 Logs/tracebacks

Not applicable

📋 Your version of the Python

$ python --version
Python 3.7.6

📋 Your version of the aiohttp/yarl/multidict distributions

$ python -m pip show aiohttp
Name: aiohttp
Version: 3.5.4
Summary: Async http client/server framework (asyncio)
Home-page: https://github.com/aio-libs/aiohttp
Author: Nikolay Kim
Author-email: fafhrd91@gmail.com
License: Apache 2
Location: /Applications/miniconda3/envs/sw-asyncher-services/lib/python3.7/site-packages
Requires: yarl, async-timeout, attrs, multidict, chardet
Required-by: aiojobs, aiohttp-apispec

$ python -m pip show multidict
Name: multidict
Version: 4.7.5
Summary: multidict implementation
Home-page: https://github.com/aio-libs/multidict
Author: Andrew Svetlov
Author-email: andrew.svetlov@gmail.com
License: Apache 2
Location: /Applications/miniconda3/envs/sw-asyncher-services/lib/python3.7/site-packages
Requires:
Required-by: yarl, aiohttp

$ python -m pip show yarl
Name: yarl
Version: 1.4.2
Summary: Yet another URL library
Home-page: https://github.com/aio-libs/yarl/
Author: Andrew Svetlov
Author-email: andrew.svetlov@gmail.com
License: Apache 2
Location: /Applications/miniconda3/envs/sw-asyncher-services/lib/python3.7/site-packages
Requires: idna, multidict
Required-by: aiohttp

📋 Additional context
On the server side, when I change the response from 400 to any non-bad response say 200, I can access passed error message.

Current workaround would be to have session = ClientSession(raise_for_status=False), and handle bad responses manually.

🔷 Possible solution
Modify ClientResponseError exception to include an error message, convert raise_for_status method to be async to be able to read the response message, and raise ClientResponseError with the read message.

[borysvorona]

Hi. You can use a simple solution here.

import asyncio
from pprint import pprint

from attr import dataclass

from aiohttp import ClientSession, ClientResponseError, ClientResponse


@dataclass
class ClientResponseMessage:
    reason: str
    body: str


async def raise_for_status(self: ClientResponse) -> None:
    if 400 <= self.status:
        # reason should always be not None for a started response
        assert self.reason is not None
        message = ClientResponseMessage(reason=self.reason, body=await self.text())
        self.release()
        raise ClientResponseError(
            self.request_info,
            self.history,
            status=self.status,
            message=message,
            headers=self.headers)


async def main():
    try:
        async with ClientSession(raise_for_status=raise_for_status) as session:
            async with session.get("http://www.mocky.io/v2/5e5e682d310000cef62c254b") as response:
                pprint(await response.text())
    except ClientResponseError as ex:
        pprint(ex.__dict__)
        """
        {'headers': <CIMultiDictProxy(...)>,
         'history': (),
         'message': ClientResponseMessage(reason='Bad Request', body='Some interesting text. You are welcome!'),
         'request_info': RequestInfo(...),
         'status': 400}
        """

asyncio.run(main())

[yeralin]

Nope, I don't think this solution is correct.

1. raise_for_status expects bool
2. It does not call a custom raise_for_status callable
3. Even if it did, the custom raise_for_status would be never awaited by the internal request handler

My version is aiohttp==3.5.4

Related: #3365 (comment)

UPD:
Actually I am only partially wrong. My statements above are correct, but not for the latest aiohttp codebase:

aiohttp/aiohttp/client.py

Lines 550 to 559 in 63a0d10

	# check response status
	if raise_for_status is None:
	raise_for_status = self._raise_for_status
	if raise_for_status is None:
	pass
	elif callable(raise_for_status):
	await raise_for_status(resp)
	elif raise_for_status:
	resp.raise_for_status()

In the latest code, it does await and can accept Callable

[yeralin]

Fixed by #3892

[yeralin]

I just checked aiohttp==3.6.2 and the change with awaiting custom raise_for_status (#3892) is not in place. However, it was committed on Sep 9th, 2019.

I think it will be part of 4.0.0 release.

[Ma233]

This also broke resp.ok:

    # Will crash while server response 4xx or 5xx
    async with aiohttp.request(method=method, url=url, data=data, headers=headers) as resp:
        is_ok, resp_data = resp.ok, await resp.json()

    # Have to fix in this way
    async with aiohttp.request(method=method, url=url, data=data, headers=headers) as resp:
        resp_data = await resp.json()  # access body before invoke resp.ok
        is_ok = resp.ok

Hope fixes will release soon.

[HansBrende]

FYI @yeralin and everyone, I've implemented the following workaround for this while we are waiting on v4:

class MessagePreservingClientResponse(aiohttp.ClientResponse):
    async def start(self, connection: aiohttp.connector.Connection) -> aiohttp.ClientResponse:
        result = await super().start(connection)
        if self.status >= 400:
            await self.read()  # Avoid a ClientConnectionError later
        return result

    def raise_for_status(self) -> None:
        try:
            super().raise_for_status()
        except aiohttp.ClientResponseError as err:
            try:
                coro = self.text()
                try:
                    coro.send(None)
                except StopIteration as text:
                    if text.value:
                        err.message = text.value
                finally:
                    coro.close()
            finally:
                raise err


def make_session() -> aiohttp.ClientSession:
    #  Workaround for https://github.com/aio-libs/aiohttp/issues/4600
    return aiohttp.ClientSession(raise_for_status=True, response_class=MessagePreservingClientResponse)

Usage:

async with make_session() as session:
    do your normal stuff... Magic!