Add support for Cache-Control: stale-if-error

4 files changed, 89 insertions, 79 deletions

diff --git a/requests_cache/policy/actions.py b/requests_cache/policy/actions.py
index b579943..0d5586b 100644
--- a/requests_cache/policy/actions.py
+++ b/requests_cache/policy/actions.py
@@ -1,6 +1,6 @@
-from datetime import datetime
+from datetime import datetime, timedelta
 from logging import getLogger
-from typing import TYPE_CHECKING, Dict, Optional
+from typing import TYPE_CHECKING, Dict, Optional, Union
 
 from attr import define, field
 from requests import PreparedRequest, Response
@@ -13,6 +13,7 @@ from . import (
     CacheDirectives,
     ExpirationTime,
     get_expiration_datetime,
+    get_expiration_seconds,
     get_url_expiration,
 )
 from .settings import CacheSettings
@@ -54,14 +55,15 @@ class CacheActions:
     skip_read: bool = field(default=False)
     skip_write: bool = field(default=False)
 
-    # Inputs/internal attributes
-    _settings: CacheSettings = field(default=None, repr=False, init=False)
-    _request_directives = field(default=None, repr=False, init=False)
-    _validation_headers: Dict[str, str] = field(factory=dict, repr=False, init=False)
+    # Inputs
+    _directives: CacheDirectives = field(default=None, repr=False)
+    _settings: CacheSettings = field(default=None, repr=False)
 
-    # TODO: set these on either directives or settings?
-    _only_if_cached: bool = field(default=False)
-    _refresh: bool = field(default=False)
+    # Temporary attributes
+    _only_if_cached: bool = field(default=False, repr=False)
+    _refresh: bool = field(default=False, repr=False)
+    _stale_if_error: Union[bool, ExpirationTime] = field(default=None, repr=False)
+    _validation_headers: Dict[str, str] = field(factory=dict, repr=False)
 
     @classmethod
     def from_request(cls, cache_key: str, request: PreparedRequest, settings: CacheSettings = None):
@@ -76,11 +78,10 @@ class CacheActions:
         directives = CacheDirectives.from_headers(request.headers)
         logger.debug(f'Cache directives from request headers: {directives}')
 
-        # Merge relevant headers with session + request settings
-        expire_immediately = directives.max_age == EXPIRE_IMMEDIATELY
+        # Merge values that may come from either settings or headers
         only_if_cached = settings.only_if_cached or directives.only_if_cached
-        refresh = expire_immediately or directives.must_revalidate
-        force_refresh = directives.no_cache
+        refresh = directives.max_age == EXPIRE_IMMEDIATELY or directives.must_revalidate
+        stale_if_error = settings.stale_if_error or directives.stale_if_error
 
         # Check expiration values in order of precedence
         expire_after = coalesce(
@@ -93,8 +94,7 @@ class CacheActions:
         read_criteria = {
             'disabled cache': settings.disabled,
             'disabled method': str(request.method) not in settings.allowable_methods,
-            'disabled by headers': directives.no_store,
-            'disabled by refresh': force_refresh,
+            'disabled by headers or refresh': directives.no_cache or directives.no_store,
             'disabled by expiration': expire_after == DO_NOT_CACHE,
         }
         _log_cache_criteria('read', read_criteria)
@@ -106,9 +106,10 @@ class CacheActions:
             refresh=refresh,
             skip_read=any(read_criteria.values()),
             skip_write=directives.no_store,
+            stale_if_error=stale_if_error,
+            directives=directives,
+            settings=settings,
         )
-        actions._settings = settings
-        actions._request_directives = directives
         return actions
 
     @property
@@ -118,56 +119,50 @@ class CacheActions:
         """
         return get_expiration_datetime(self.expire_after)
 
-    def _is_expired(self, cached_response: 'CachedResponse'):
-        """Determine whether a given cached response is "fresh enough" to satisfy the request"""
-        if not getattr(cached_response, 'expires', None):
+    # TODO: Better name?
+    def is_usable(self, cached_response: 'CachedResponse', error: bool = False):
+        """Determine whether a given cached response is "fresh enough" to satisfy the request,
+        based on min-fresh, max-stale, or stale-if-error (if an error has occured).
+        """
+        if cached_response is None:
             return False
+        elif cached_response.expires is None:
+            return True
+        # Handle additional types supported for stale_if_error
+        elif error and self._stale_if_error is True:
+            return True
+        elif error and self._stale_if_error:
+            offset_seconds = get_expiration_seconds(self._stale_if_error)
+            offset = timedelta(seconds=offset_seconds)
+        # Handle min-fresh and max-stale
+        else:
+            offset = self._directives.get_expire_offset()
 
-        expires_ajusted = cached_response.expires + self._request_directives.expire_offset
-        return datetime.utcnow() >= expires_ajusted
+        return datetime.utcnow() < cached_response.expires + offset
 
     def update_from_cached_response(self, cached_response: 'CachedResponse'):
-        """Check for relevant cache headers from a cached response, and set headers for a
-        conditional request, if possible.
+        """Determine if we can reuse a cached response, or set headers for a conditional request
+        if possible.
 
         Used after fetching a cached response, but before potentially sending a new request.
         """
-        # Determine if we need to send a new request or respond with an error
-        is_expired = self._is_expired(cached_response)
-        invalid_response = cached_response is None or is_expired
-        if invalid_response and self._only_if_cached and not self._settings.stale_if_error:
+        valid_response = self.is_usable(cached_response)
+        valid_if_error = self.is_usable(cached_response, error=True)
+
+        # Can't satisfy the request
+        if not valid_response and self._only_if_cached and not valid_if_error:
             self.error_504 = True
+        # Send the request for the first time
         elif cached_response is None:
             self.send_request = True
-        elif is_expired and not (self._only_if_cached and self._settings.stale_if_error):
+        # Resend the request, unless settings permit a stale response
+        elif not valid_response and not (self._only_if_cached and valid_if_error):
             self.resend_request = True
 
         if cached_response is not None:
             self._update_validation_headers(cached_response)
         logger.debug(f'Post-read cache actions: {self}')
 
-    def _update_validation_headers(self, response: 'CachedResponse'):
-        """If needed, get validation headers based on a cached response. Revalidation may be
-        triggered by a stale response, request headers, or cached response headers.
-        """
-        directives = CacheDirectives.from_headers(response.headers)
-        revalidate = directives.has_validator and (
-            response.is_expired
-            or self._refresh
-            or directives.no_cache
-            or directives.must_revalidate
-            and directives.max_age == 0
-        )
-
-        # Add the appropriate validation headers, if needed
-        if revalidate:
-            if directives.etag:
-                self._validation_headers['If-None-Match'] = directives.etag
-            if directives.last_modified:
-                self._validation_headers['If-Modified-Since'] = directives.last_modified
-            self.send_request = True
-            self.resend_request = False
-
     def update_from_response(self, response: Response):
         """Update expiration + actions based on headers and other details from a new response.
 
@@ -197,10 +192,25 @@ class CacheActions:
         self.skip_write = any(write_criteria.values())
         _log_cache_criteria('write', write_criteria)
 
+    def update_request(self, request: PreparedRequest) -> PreparedRequest:
+        """Apply validation headers (if any) before sending a request"""
+        request.headers.update(self._validation_headers)
+        return request
+
+    def update_revalidated_response(
+        self, response: Response, cached_response: 'CachedResponse'
+    ) -> 'CachedResponse':
+        """After revalidation, update the cached response's expiration and headers"""
+        logger.debug(f'Response for URL {response.request.url} has not been modified')
+        cached_response.expires = self.expires
+        cached_response.headers.update(response.headers)
+        return cached_response
+
     def _update_from_response_headers(self, directives: CacheDirectives):
         """Check response headers for expiration and other cache directives"""
         logger.debug(f'Cache directives from response headers: {directives}')
 
+        self._stale_if_error = self._stale_if_error or directives.stale_if_error
         if directives.immutable:
             self.expire_after = NEVER_EXPIRE
         else:
@@ -211,23 +221,27 @@ class CacheActions:
             )
         self.skip_write = self.skip_write or directives.no_store
 
-    def update_request(self, request: PreparedRequest) -> PreparedRequest:
-        """Apply validation headers (if any) before sending a request"""
-        request.headers.update(self._validation_headers)
-        return request
-
-    def update_revalidated_response(
-        self, response: Response, cached_response: 'CachedResponse'
-    ) -> 'CachedResponse':
-        """After revalidation, update the cached response's headers and reset its expiration"""
-        logger.debug(
-            f'Response for URL {response.request.url} has not been modified; '
-            'updating and using cached response'
+    def _update_validation_headers(self, cached_response: 'CachedResponse'):
+        """If needed, get validation headers based on a cached response. Revalidation may be
+        triggered by a stale response, request headers, or cached response headers.
+        """
+        directives = CacheDirectives.from_headers(cached_response.headers)
+        revalidate = directives.has_validator and (
+            cached_response.is_expired
+            or self._refresh
+            or directives.no_cache
+            or directives.must_revalidate
+            and directives.max_age == 0
         )
-        cached_response.expires = self.expires
-        cached_response.headers.update(response.headers)
-        self.update_from_response(cached_response)
-        return cached_response
+
+        # Add the appropriate validation headers, if needed
+        if revalidate:
+            if directives.etag:
+                self._validation_headers['If-None-Match'] = directives.etag
+            if directives.last_modified:
+                self._validation_headers['If-Modified-Since'] = directives.last_modified
+            self.send_request = True
+            self.resend_request = False
 
 
 def _log_cache_criteria(operation: str, criteria: Dict):
diff --git a/requests_cache/policy/directives.py b/requests_cache/policy/directives.py
index 8b8df2b..c89fad7 100644
--- a/requests_cache/policy/directives.py
+++ b/requests_cache/policy/directives.py
@@ -24,13 +24,10 @@ class CacheDirectives:
     no_cache: bool = field(default=False)
     no_store: bool = field(default=False)
     only_if_cached: bool = field(default=False)
+    stale_if_error: int = field(default=None, converter=try_int)
     etag: str = field(default=None)
     last_modified: str = field(default=None)
 
-    # Not yet implemented:
-    # stale_if_error: int = field(default=None, converter=try_int)
-    # stale_while_revalidate: bool = field(default=False)
-
     @classmethod
     def from_headers(cls, headers: HeaderDict):
         """Parse cache directives and other settings from request or response headers"""
@@ -46,8 +43,7 @@ class CacheDirectives:
         kwargs['last_modified'] = headers.get('Last-Modified')
         return cls(**kwargs)
 
-    @property
-    def expire_offset(self) -> timedelta:
+    def get_expire_offset(self) -> timedelta:
         """Return the time offset to use for expiration, if either min-fresh or max-stale is set"""
         offset_seconds = 0
         if self.max_stale:
diff --git a/requests_cache/policy/settings.py b/requests_cache/policy/settings.py
index 6b458a9..3e5d1cf 100644
--- a/requests_cache/policy/settings.py
+++ b/requests_cache/policy/settings.py
@@ -33,7 +33,7 @@ class CacheSettings:
     key_fn: KeyCallback = field(default=None)
     match_headers: Union[Iterable[str], bool] = field(default=False)
     only_if_cached: bool = field(default=False)
-    stale_if_error: bool = field(default=False)
+    stale_if_error: Union[bool, ExpirationTime] = field(default=False)
     urls_expire_after: Dict[str, ExpirationTime] = field(factory=dict)
 
     @classmethod
diff --git a/requests_cache/session.py b/requests_cache/session.py
index 2f35d6e..59a1d4f 100644
--- a/requests_cache/session.py
+++ b/requests_cache/session.py
@@ -55,7 +55,7 @@ class CacheMixin(MIXIN_BASE):
         match_headers: Union[Iterable[str], bool] = False,
         filter_fn: FilterCallback = None,
         key_fn: KeyCallback = None,
-        stale_if_error: bool = False,
+        stale_if_error: Union[bool, int] = False,
         **kwargs,
     ):
         self.cache = init_backend(cache_name, backend, serializer=serializer, **kwargs)
@@ -256,17 +256,16 @@ class CacheMixin(MIXIN_BASE):
 
     def _handle_error(self, cached_response: CachedResponse, actions: CacheActions) -> AnyResponse:
         """Handle a request error based on settings:
-        * Default behavior: delete the stale cache item and re-raise the error
+        * Default behavior: re-raise the error
         * stale-if-error: Ignore the error and and return the stale cache item
         """
-        if self.settings.stale_if_error:
+        if actions.is_usable(cached_response, error=True):
             logger.warning(
                 f'Request for URL {cached_response.request.url} failed; using cached response',
                 exc_info=True,
             )
             return cached_response
         else:
-            self.cache.delete(actions.cache_key)
             raise
 
     @contextmanager
@@ -331,7 +330,8 @@ class CachedSession(CacheMixin, OriginalSession):
             a list of specific headers to match
         ignored_parameters: Request paramters, headers, and/or JSON body params to exclude from both
             request matching and cached request data
-        stale_if_error: Return stale cache data if a new request raises an exception
+        stale_if_error: Return stale cache data if a new request raises an exception. Optionally
+            accepts a time value representing maximum staleness to accept.
         filter_fn: Response filtering function that indicates whether or not a given response should
             be cached. See :ref:`custom-filtering` for details.
         key_fn: Request matching function for generating custom cache keys. See