fix: edge case when `aiohttp` falls back to use `_resolve_charset`

[alessio-locatelli]

Closes https://github.com/requests-cache/aiohttp-client-cache/issues/256

[alessio-locatelli]

I am still thinking about how to extend the tests to cover these edge cases. This may take some time, so I decided to create a new test in a next PR.

[layday]

Since CachedResponse.postprocess is not called if the cache backend is disabled response is not cached, this throws when aiohttp attempts to call get_encoding on a CachedResponse. For example:

[...]

  File "/Users/dimitris/Code/instawow/.nox/dev_env-3-12/lib/python3.12/site-packages/aiohttp/client_reqrep.py", line 1258, in json
    encoding = self.get_encoding()
               │    └ <function CachedResponse.get_encoding at 0x10267f560>
               └ <ClientResponse(https://data.wago.io/api/check/plater) [200 OK]>
                 <CIMultiDictProxy('Date': 'Wed, 02 Oct 2024 20:16:39 GMT', '...
  File "/Users/dimitris/Code/instawow/.nox/dev_env-3-12/lib/python3.12/site-packages/aiohttp_client_cache/response.py", line 94, in get_encoding
    return self._encoding
           └ <ClientResponse(https://data.wago.io/api/check/plater) [200 OK]>
             <CIMultiDictProxy('Date': 'Wed, 02 Oct 2024 20:16:39 GMT', '...

AttributeError: 'CachedResponse' object has no attribute '_encoding'. Did you mean: 'get_encoding'?

[layday]

It is also not safe to call get_encoding mid-request. This will fail if aiohttp cannot read the encoding from Content-Type:

[...]

  File "/Users/dimitris/Code/instawow/.nox/dev_env-3-12/lib/python3.12/site-packages/aiohttp/client.py", line 1355, in __aenter__
    self._resp: _RetType = await self._coro
    │    │                       │    └ <member '_coro' of '_BaseRequestContextManager' objects>
    │    │                       └ <aiohttp.client._BaseRequestContextManager object at 0x105d198d0>
    │    └ <member '_resp' of '_BaseRequestContextManager' objects>
    └ <aiohttp.client._BaseRequestContextManager object at 0x105d198d0>
  File "/Users/dimitris/Code/instawow/.nox/dev_env-3-12/lib/python3.12/site-packages/aiohttp_client_cache/session.py", line 123, in _request
    await self.cache.save_response(new_response, actions.key, actions.expires)
          │    │     │             │             │       │    │       └ <property object at 0x1058250d0>
          │    │     │             │             │       │    └ CacheActions(cache_control=False, expire_after=-1, key='7f8a0ddaf23e8b3e9ba04c886e98e44a919e13179b0f1656a5ea1a90fef0b781', re...
          │    │     │             │             │       └ <member 'key' of 'CacheActions' objects>
          │    │     │             │             └ CacheActions(cache_control=False, expire_after=-1, key='7f8a0ddaf23e8b3e9ba04c886e98e44a919e13179b0f1656a5ea1a90fef0b781', re...
          │    │     │             └ <ClientResponse(https://data.wago.io/api/raw/encoded?id=NjUGOV64I&version=1.0.2) [200 OK]>
          │    │     │               <CIMultiDictProxy('Date': 'Wed, 02...
          │    │     └ <function CacheBackend.save_response at 0x105ac0180>
          │    └ <aiohttp_client_cache.backends.base.CacheBackend object at 0x105acd6d0>
          └ <aiohttp_client_cache.session.CachedSession object at 0x105aced50>
  File "/Users/dimitris/Code/instawow/.nox/dev_env-3-12/lib/python3.12/site-packages/aiohttp_client_cache/backends/base.py", line 196, in save_response
    cached_response = await response.postprocess(expires)
                            │        │           └ None
                            │        └ <function CachedResponse.postprocess at 0x105abf7e0>
                            └ <ClientResponse(https://data.wago.io/api/raw/encoded?id=NjUGOV64I&version=1.0.2) [200 OK]>
                              <CIMultiDictProxy('Date': 'Wed, 02...
  File "/Users/dimitris/Code/instawow/.nox/dev_env-3-12/lib/python3.12/site-packages/aiohttp_client_cache/response.py", line 130, in postprocess
    self._history = (*[await cast(CachedResponse, r).postprocess() for r in self.history],)
    │    │                   │    │                                         │    └ <aiohttp._helpers.reify object at 0x10535fdf0>
    │    │                   │    │                                         └ <ClientResponse(https://data.wago.io/api/raw/encoded?id=NjUGOV64I&version=1.0.2) [200 OK]>
    │    │                   │    │                                           <CIMultiDictProxy('Date': 'Wed, 02...
    │    │                   │    └ <class 'aiohttp_client_cache.response.CachedResponse'>
    │    │                   └ <function cast at 0x102d0ec00>
    │    └ (<ClientResponse(https://data.wago.io/api/raw/encoded?id=NjUGOV64I) [302 Found]>
    │      <CIMultiDictProxy('Date': 'Wed, 02 Oct 2024 ...
    └ <ClientResponse(https://data.wago.io/api/raw/encoded?id=NjUGOV64I&version=1.0.2) [200 OK]>
      <CIMultiDictProxy('Date': 'Wed, 02...
  File "/Users/dimitris/Code/instawow/.nox/dev_env-3-12/lib/python3.12/site-packages/aiohttp_client_cache/response.py", line 135, in postprocess
    self._encoding: str = super().get_encoding()
    └ <ClientResponse(https://data.wago.io/api/raw/encoded?id=NjUGOV64I) [302 Found]>
      <CIMultiDictProxy('Date': 'Wed, 02 Oct 2024 2...
  File "/Users/dimitris/Code/instawow/.nox/dev_env-3-12/lib/python3.12/site-packages/aiohttp/client_reqrep.py", line 1211, in get_encoding
    raise RuntimeError(

RuntimeError: Cannot compute fallback encoding of a not yet read body

[JWCook]

Thanks @layday , that's quite helpful. I'm a bit swamped with work right now, but I will make some time to take care of this tomorrow. Or would you or someone else be interested in submitting a PR for this?

[alessio-locatelli]

@layday Thanks for the good comment. It would be really great if you could also prepare a small reproducible example that we can add to the tests. In the meantime, I will try to write a test to reproduce the problem.

[alessio-locatelli]

To give an update:

The described problem happens when we call get_encoding() on a response that is uncachable, so the following test covers the problem (actually, this is the existing test, except the last line):

    async def test_response__skip_cache_write(self):
        """max-age=0 response header should skip writing to the cache"""
        async with self.init_session(cache_control=True) as session:
            await session.get(httpbin('cache/0'))
            response = cast(CachedResponse, await session.get(httpbin('cache/0')))

            assert response.from_cache is False
            assert await session.cache.responses.size() == 0
            assert response.get_encoding() == 'utf-8'

~I reverted back -> AnyResponse (where we can return ClientResponse or CachedResponse), but I reverted the changes with respect to the latest aiohttp.~

~The "integration" tests are green, but I need to revert/update "unit" tests. Just in case, here is a "WIP" branch that I want to clean up and submit for a review.~

[alessio-locatelli]

@JWCook @layday While inheritance fixed some old problems such as the fact that CachedResponse has outdated attributes that aiohttp.ClientResponse does not have anymore (due to renaming, removing, etc.), but inheritance leaded to some troubles, e.g. I would need to read the body for responses that are "uncacheable".
I ended up with the decision I mentioned in https://github.com/requests-cache/aiohttp-client-cache/issues/251#issuecomment-2393003811