Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CachedResponse.history is not fully deserialized on python 3.7 and 3.8 #836

Closed
JWCook opened this issue Jun 5, 2023 · 1 comment · Fixed by #835
Closed

CachedResponse.history is not fully deserialized on python 3.7 and 3.8 #836

JWCook opened this issue Jun 5, 2023 · 1 comment · Fixed by #835
Labels
Milestone

Comments

@JWCook
Copy link
Member

JWCook commented Jun 5, 2023

Found during #835; related to python-attrs/cattrs#206

Example error:

requests_cache/session.py:103: in get
    return self.request('GET', url, params=params, **kwargs)
requests_cache/session.py:159: in request
    return super().request(method, url, *args, headers=headers, **kwargs)  # type: ignore
.venv/lib/python3.7/site-packages/requests/sessions.py:589: in request
    resp = self.send(prep, **send_kwargs)
requests_cache/session.py:195: in send
    actions.update_from_cached_response(cached_response, self.cache.create_key, **kwargs)
requests_cache/policy/actions.py:194: in update_from_cached_response
    elif create_key and not self._validate_vary(cached_response, create_key, **key_kwargs):
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = CacheActions(expire_after=-1)
cached_response = CachedResponse(_content=b'mock redirect response with Vary header (2/2)', created_at='2023-06-05 17:56:56.815238', ela.../requests-cache.com/vary-redirect-target'), status_code=200, url='http+mock://requests-cache.com/vary-redirect-target')
create_key = <bound method BaseCache.create_key of <SQLiteCache(name=/tmp/pytest-of-runner/pytest-0/popen-gw1/test_match_headers__vary_with_4/24501901-26eb-4b6b-a6d0-6b12df83c48b.db)>>
key_kwargs = {'allow_redirects': True, 'cert': None, 'match_headers': ['Accept-Language'], 'proxies': OrderedDict(), ...}
vary = 'Accept-Language'

    def _validate_vary(
        self, cached_response: 'CachedResponse', create_key: KeyCallback, **key_kwargs
    ) -> bool:
        """If the cached response contains Vary, check that the specified request headers match"""
        vary = cached_response.headers.get('Vary')
        if not vary:
            return True
        elif vary == '*':
            return False
    
        # Generate a secondary cache key based on Vary for both the cached request and new request.
        # If there are redirects, compare the new request against the last request in the chain.
        key_kwargs['match_headers'] = [k.strip() for k in vary.split(',')]
        vary_request = (
            cached_response.history[-1].request
>           if cached_response.history
            else cached_response.request
        )
E       AttributeError: 'dict' object has no attribute 'request'

requests_cache/policy/actions.py:308: AttributeError

CachedResponse.history should contain CachedResponse when there is redirect history available. Instead, it contains dicts, meaning the structure hook is not working.

@JWCook JWCook added the bug label Jun 5, 2023
@JWCook JWCook added this to the v1.1 milestone Jun 5, 2023
@JWCook
Copy link
Member Author

JWCook commented Jun 5, 2023

Posting this for future reference.

It turns out that CachedResponse.history wasn't being fully deserialized on python<=3.8 due to an unevaluated ForwardRef, causing the cattrs unstructure hook to fail.

When not yet evaluated, ForwardRef.__forward_value__ is None, so this call:

converter.structure(obj, cls.__forward_value__)

no longer works.

I'm unsure why this behaves differently on python 3.9+. Anyway, the workaround is to manually evaluate the ForwardRef:

if not cls.__forward_evaluated__:
    cls._evaluate(globals(), locals())

In python 3.9+, the call would be:

cls._evaluate(globals(), locals(), frozenset())

but that seems to be unnecessary here.

@JWCook JWCook changed the title CachedResponse.history contains dicts instead of CachedResponse objects on python 3.7 and 3.8 CachedResponse.history is not fully deserialized on python 3.7 and 3.8 Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant