Merge pull request #247 from JWCook/raise_for_status

Handle response error codes with old_data_on_error

5 files changed, 55 insertions, 7 deletions

diff --git a/HISTORY.md b/HISTORY.md
index af88987..0335c5e 100644
--- a/HISTORY.md
+++ b/HISTORY.md
@@ -1,9 +1,10 @@
 # History
 
-## 0.7.0 (2021-TBD) 
-* Add a filesystem backend
+## 0.7.0 (2021-06-TBD)
+* Add a filesystem backend that stores responses as local files
 * Add option to manually cache response objects
 * Add `use_temp` option to both SQLite and filesystem backends to store files in a temp directory
+* Update `old_data_on_error` option to also handle error response codes
 * Use thread-local connections for SQLite backend
 * Fix `DynamoDbDict.__iter__` to return keys instead of values
 
diff --git a/docs/user_guide.rst b/docs/user_guide.rst
index 1c6619f..12517fb 100644
--- a/docs/user_guide.rst
+++ b/docs/user_guide.rst
@@ -227,7 +227,7 @@ is used:
 
 To set expiration for a single request:
 
-    >>> session.get('http://httpbin.org/get', expire_after=360)
+    >>> session.get('https://httpbin.org/get', expire_after=360)
 
 URL Patterns
 ~~~~~~~~~~~~
@@ -270,6 +270,30 @@ revalidate the cache with the new expiration time:
 
     >>> session.remove_expired_responses(expire_after=timedelta(days=30))
 
+
+Error Handling
+--------------
+In some cases, you might cache a response, have it expire, but then encounter an error when
+retrieving a new response. If you would like to use expired response data in these cases, use the
+``old_data_on_error`` option:
+
+    >>> # Cache a test response that will expire immediately
+    >>> session = CachedSession(old_data_on_error=True)
+    >>> session.get('https://httpbin.org/get', expire_after=0.001)
+    >>> time.sleep(0.001)
+
+Afterward, let's say the page has moved and you get a 404, or the site is experiencing downtime and
+you get a 500. You will then get the expired cache data instead:
+
+    >>> response = session.get('https://httpbin.org/get')
+    >>> print(response.from_cache, response.is_expired)
+    True, True
+
+In addition to error codes, ``old_data_on_error`` also applies to exceptions (typically a
+:py:exc:`~requests.RequestException`). See requests documentation on
+`Errors and Exceptions <https://2.python-requests.org/en/master/user/quickstart/#errors-and-exceptions>`_
+for more details on request errors in general.
+
 Potential Issues
 ----------------
 * Version updates of ``requests``, ``urllib3`` or ``requests-cache`` itself may not be compatible with
diff --git a/requests_cache/session.py b/requests_cache/session.py
index 65484ca..d34a114 100644
--- a/requests_cache/session.py
+++ b/requests_cache/session.py
@@ -144,7 +144,10 @@ class CacheMixin:
         # Attempt to send the request and cache the new response
         logger.debug('Expired response; attempting to re-send request')
         try:
-            return self._send_and_cache(request, cache_key, **kwargs)
+            new_response = self._send_and_cache(request, cache_key, **kwargs)
+            if self.old_data_on_error:
+                new_response.raise_for_status()
+            return new_response
         # Return the expired/invalid response on error, if specified; otherwise reraise
         except Exception as e:
             logger.exception(e)
@@ -244,7 +247,7 @@ class CachedSession(CacheMixin, OriginalSession):
         filter_fn: function that takes a :py:class:`aiohttp.ClientResponse` object and
             returns a boolean indicating whether or not that response should be cached. Will be
             applied to both new and previously cached responses.
-        old_data_on_error: Return expired cached responses if new request fails
+        old_data_on_error: Return stale cache data if a new request raises an exception
         secret_key: Optional secret key used to sign cache items for added security
 
     """
diff --git a/tests/conftest.py b/tests/conftest.py
index c01e066..7f155d4 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -51,6 +51,7 @@ MOCKED_URL_HTTPS = 'https+mock://requests-cache.com/text'
 MOCKED_URL_JSON = 'http+mock://requests-cache.com/json'
 MOCKED_URL_REDIRECT = 'http+mock://requests-cache.com/redirect'
 MOCKED_URL_REDIRECT_TARGET = 'http+mock://requests-cache.com/redirect_target'
+MOCKED_URL_404 = 'http+mock://requests-cache.com/nonexistent'
 MOCK_PROTOCOLS = ['mock://', 'http+mock://', 'https+mock://']
 
 AWS_OPTIONS = {
@@ -191,6 +192,11 @@ def get_mock_adapter() -> Adapter:
         text='mock redirected response',
         status_code=200,
     )
+    adapter.register_uri(
+        ANY_METHOD,
+        MOCKED_URL_404,
+        status_code=404,
+    )
     return adapter
 
 
diff --git a/tests/unit/test_cache.py b/tests/unit/test_cache.py
index d27efa4..0044893 100644
--- a/tests/unit/test_cache.py
+++ b/tests/unit/test_cache.py
@@ -20,6 +20,7 @@ from requests_cache.backends import BACKEND_CLASSES, BaseCache, get_placeholder_
 from requests_cache.backends.sqlite import DbDict, DbPickleDict
 from tests.conftest import (
     MOCKED_URL,
+    MOCKED_URL_404,
     MOCKED_URL_HTTPS,
     MOCKED_URL_JSON,
     MOCKED_URL_REDIRECT,
@@ -341,8 +342,8 @@ def test_expired_request_error(mock_session):
     assert len(mock_session.cache.responses) == 0
 
 
-def test_old_data_on_error(mock_session):
-    """With old_data_on_error, expect to get old cache data if there is an error during a request"""
+def test_old_data_on_error__exception(mock_session):
+    """With old_data_on_error, expect to get old cache data if there is an exception during a request"""
     mock_session.old_data_on_error = True
     mock_session.expire_after = 0.2
 
@@ -354,6 +355,19 @@ def test_old_data_on_error(mock_session):
         assert response.from_cache is True and response.is_expired is True
 
 
+def test_old_data_on_error__error_code(mock_session):
+    """With old_data_on_error, expect to get old cache data if a response has an error status code"""
+    mock_session.old_data_on_error = True
+    mock_session.expire_after = 0.2
+    mock_session.allowable_codes = (200, 404)
+
+    assert mock_session.get(MOCKED_URL_404).from_cache is False
+    assert mock_session.get(MOCKED_URL_404).from_cache is True
+    time.sleep(0.2)
+    response = mock_session.get(MOCKED_URL_404)
+    assert response.from_cache is True and response.is_expired is True
+
+
 def test_cache_disabled(mock_session):
     mock_session.get(MOCKED_URL)
     with mock_session.cache_disabled():