Cached response doesn't set session cookies

rkfcccccc commented 3 years ago

The problem

Cached response doesn't keep the cookies.

Expected behavior

It should

Steps to reproduce the behavior

import asyncio
from aiohttp_client_cache import CachedSession, SQLiteBackend

async def main():
    cache = SQLiteBackend(
        cache_name='.cache/test.db',
        expire_after=-1,
        allowed_methods=('GET', 'POST'),
        allowed_codes=(200, 418)
    )

    async with CachedSession(cache=cache) as session:
        async with session.get('http://httpbin.org/cookies/set?test=123') as response:
            cookies = session.cookie_jar.filter_cookies('http://httpbin.org')
            print(cookies) # prints "Set-Cookie: test=123"

        # Clear the cookies from session
        session.cookie_jar.clear()

        # this will be taken from the cache
        async with session.get('http://httpbin.org/cookies/set?test=123') as response:
            cookies = session.cookie_jar.filter_cookies('http://httpbin.org')
            print(cookies) # prints nothing instead of "set-cookie..."

asyncio.get_event_loop().run_until_complete(main())

Environment

aiohttp-client-cache version: 0.5.1
Python version: 3.9.6
Platform: MacOS

JWCook commented 3 years ago

Thanks for reporting this, I'll get that fixed soon.

JWCook commented 3 years ago

I looked into this a bit more. It looks like the cached response does keep cookies, but CachedSession._request() is missing the extra step to set the session cookies after retrieving the cached response. That part is an easy fix.

However, that particular httpbin endpoint returns a redirect (which aiohttp automatically follows), so cookies aren't actually set on response.cookies, but rather in redirect history (response.history[0].cookies). This makes things a bit complicated, since aiohttp-client-cache optimizes redirects by pointing directly to the final redirect response instead of following the full redirect chain like aiohttp does. This is also fixable, but a little bit more work.

JWCook commented 2 years ago

@rkfcccccc Sorry for the delay, but it looks like I actually fixed this with #101 and didn't realize it. I thought I had only partially fixed the issue, and I was totally stumped as to why it still wasn't working with redirect cookies. I just revisited this to test it again, and... it's working. 🤷

So I have no idea what was going on earlier, but I added tests to cover this, and the fix is now in v0.6. Give that a try and let me know if you have any more problems.

requests-cache / aiohttp-client-cache