history tuple on ClientResponseError empty with raise_for_status() or ClientSession(raise_for_status=True)

[brettdh]

Long story short

When using a ClientSession created with raise_for_status=True, the resulting ClientResponseError does not have its history field populated, making it difficult to get the response data.

Update: this is also true when calling .raise_for_status() explicitly.

At a glance, it appears that .raise_for_status() is called before ._history is set on the response object:

aiohttp/aiohttp/client.py

Lines 471 to 493 in 03d83b0

	# check response status
	if raise_for_status is None:
	raise_for_status = self._raise_for_status
	if raise_for_status:
	resp.raise_for_status()
	# register connection
	if handle is not None:
	if resp.connection is not None:
	resp.connection.add_callback(handle.cancel)
	else:
	handle.cancel()
	resp._history = tuple(history)
	for trace in traces:
	await trace.send_request_end(
	method,
	url,
	headers,
	resp
	)
	return resp

Expected behaviour

The history field should be a tuple of ClientResponse objects.

Actual behaviour

The history field is an empty tuple.

Steps to reproduce

import asyncio

import aiohttp
import pytest
from aiohttp.client_exceptions import ClientResponseError


@pytest.fixture(scope='session')
def event_loop():
    loop = asyncio.new_event_loop()
    asyncio.set_event_loop(loop)
    return loop


def test_missing_history(event_loop):
    async def inner():
        session = aiohttp.ClientSession()

        try:
            async with session.get('http://httpbin.org/status/404') as r:
                r.raise_for_status()
                assert False
        except ClientResponseError as e:
            assert e.history

    event_loop.run_until_complete(inner())


def test_missing_history_with_session_var(event_loop):
    async def inner():
        session = aiohttp.ClientSession(raise_for_status=True)

        try:
            async with session.get('http://httpbin.org/status/404') as r:
                assert False
        except ClientResponseError as e:
            assert e.history

    event_loop.run_until_complete(inner())

Your environment

• Python: 3.6.1
• aiohttp: 3.3.2
• Usage: client

[brettdh]

Or perhaps I am misunderstanding what history is. I am looking for some way to get the ClientResponse from the ClientResponseError object, and I thought history filled that purpose. Am I mistaken?

[asvetlov]

Nice catch!
raise_for_status should be processed after all code, just before return resp.

Would you make a Pull Request for the fix?

[brettdh]

Problem: this doesn't appear isolated to the ClientSession(raise_for_status=True) case; see the updated test above. Both versions fail.

[brettdh]

Perhaps this simply means that raise_for_status should add self to the history tuple of the ClientResponseError it raises?

[asvetlov]

Got you.
ClientResponseError.request_info has an info about failed request.
What additional data do you need?

[brettdh]

I'm looking for the ClientResponse object, from which I can get e.g. the response payload via .text() or .json().

[brettdh]

Basically the equivalent of this in requests:

import requests

try:
    r = requests.get('http://httpbin.org/status/404')
    r.raise_for_status()
except requests.exceptions.RequestException as e:
    print(e.response.status_code)
    print(e.response.text)

which prints

404

(the response body is empty in this case, but imagine it wasn't)

[asvetlov]

Consider response as closed after raising an exception.
Technically it can contain a partial body but there is no any guarantee.
There is no reason to read it, the body could be very huge, 1GiB is not a limit.
If you need a response content for non-200 -- read it explicitly.

[brettdh]

I understand the design rationale in general; I'm mainly observing that it makes most raise_for_status usage kinda clunky. In fact, as far as I can tell, it makes it impossible to use ClientSession(raise_for_status=True) if you ever want to read the response body of a 4xx or 5xx response.

I wound up reading the response with .json(), catching the ClientResponseError, attaching my own .response_json field to it, and re-raising it. As you can imagine, I would prefer that aiohttp support this common use case a little more cleanly.

If it were always possible to retrieve the ClientResponse object from a ClientResponseError (say, e.response), I could at least explicitly await r.json() before calling r.raise_for_status(), and then (presumably) a later call of e.g. await e.response.json() would return the already-read json. This would be an improvement - possibly improvable further by a kwarg to make this automatic: r.raise_for_status(read_response=True).

Just throwing out ideas. 🙂

[asvetlov]

raise_for_status is checked after headers receiving. Say again there is no guarantee for HTTP body presence at the moment. It can be inside an internal buffer but could be read only partially.

That's why you cannot rely on resp.json() here.

I understand your wish but sorry.
By design, if you need a response body -- please don't use raise_for_status facility.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a [new issue] for related bugs.
If you feel like there's important points made in this discussion, please include those exceprts into that [new issue].
[new issue]: https://github.com/aio-libs/aiohttp/issues/new