Forbid wrong 'expires' values when low-level API is used that silently results in always expired responses

HISTORY.md

@@ -1,5 +1,9 @@
 # History
 
+## (unreleased)
+
+- Fixed a bug that allowed users to use `save_response()` and `from_client_response()` with an incorrect `expires` argument without throwing any warnings or errors.
+
 ## 0.12.3 (2024-10-04)
 
 - Revert some changes from `v0.12.0`, and add alternative fix for compatibility with aiohttp 3.10.6+

aiohttp_client_cache/response.py

@@ -52,6 +52,11 @@
 logger = getLogger(__name__)
 
 
+class UnsupportedExpiresError(Exception):
+    def __init__(self, expires: datetime):
+        super().__init__(f'Expected a naive datetime, but got {expires}.')
+
+
 @attr.s(slots=True)
 class CachedResponse(HeadersMixin):
     """A dataclass containing cached response information, used for serialization.
@@ -81,6 +86,10 @@ async def from_client_response(
         cls, client_response: ClientResponse, expires: datetime | None = None
     ):
         """Convert a ClientResponse into a CachedResponse"""
+        if expires is not None and expires.tzinfo is not None:
+            # An unrecoverable error (wrong library low-level API usage).
+            raise UnsupportedExpiresError(expires)
+
         # Copy most attributes over as is
         copy_attrs = set(attr.fields_dict(cls).keys()) - EXCLUDE_ATTRS
         response = cls(**{k: getattr(client_response, k) for k in copy_attrs})
@@ -91,7 +100,7 @@ async def from_client_response(
         response._body = client_response._body
         client_response.content = CachedStreamReader(client_response._body)
 
-        # Set remaining attributes individually
+        # Set remaining attributes individually.
         response.expires = expires
         response.links = client_response.links
         response.real_url = client_response.request_info.real_url
@@ -158,11 +167,7 @@ def host(self) -> str:
     @property
     def is_expired(self) -> bool:
         """Determine if this cached response is expired"""
-        try:
-            return self.expires is not None and utcnow() > self.expires
-        except (AttributeError, TypeError, ValueError):
-            # Consider it expired and fetch a new response
-            return True
+        return self.expires is not None and utcnow() > self.expires
 
     @property
     def links(self) -> MultiDictProxy:

test/unit/test_response.py

@@ -1,6 +1,6 @@
 from __future__ import annotations
 
-from datetime import timedelta
+from datetime import datetime, timedelta, timezone
 from unittest import mock
 
 import pytest
@@ -9,7 +9,7 @@
 from yarl import URL
 
 from aiohttp_client_cache.cache_control import utcnow
-from aiohttp_client_cache.response import CachedResponse, RequestInfo
+from aiohttp_client_cache.response import CachedResponse, RequestInfo, UnsupportedExpiresError
 
 
 async def get_test_response(client_factory, url='/', **kwargs):
@@ -78,8 +78,10 @@ async def test_is_expired(mock_utcnow, aiohttp_client):
 
 
 async def test_is_expired__invalid(aiohttp_client):
-    response = await get_test_response(aiohttp_client, expires='asdf')
-    assert response.is_expired is True
+    with pytest.raises(AttributeError, match="'str' object has no attribute 'tzinfo'"):
+        await get_test_response(aiohttp_client, expires='asdf')
+    with pytest.raises(UnsupportedExpiresError, match='Expected a naive datetime'):
+        await get_test_response(aiohttp_client, expires=datetime.now(timezone.utc))
 
 
 async def test_content_disposition(aiohttp_client):