From eceb5e2eea554ea0f9baf7abc1b1972459854cef Mon Sep 17 00:00:00 2001 From: Sulabh Katila Date: Wed, 21 Feb 2024 19:51:58 -0500 Subject: [PATCH] Fixed #34806 -- Made cached_db session backend resilient to cache write errors. Co-authored-by: Natalia <124304+nessita@users.noreply.github.com> --- django/contrib/sessions/backends/cached_db.py | 9 ++++++++- docs/ref/logging.txt | 11 +++++++++++ docs/releases/5.1.txt | 5 ++++- docs/topics/http/sessions.txt | 14 +++++++++++--- tests/cache/failing_cache.py | 7 +++++++ tests/sessions_tests/tests.py | 16 ++++++++++++++++ 6 files changed, 57 insertions(+), 5 deletions(-) create mode 100644 tests/cache/failing_cache.py diff --git a/django/contrib/sessions/backends/cached_db.py b/django/contrib/sessions/backends/cached_db.py index 3125a71cd0ea..a2a8cf47afd4 100644 --- a/django/contrib/sessions/backends/cached_db.py +++ b/django/contrib/sessions/backends/cached_db.py @@ -2,12 +2,16 @@ Cached, database-backed sessions. """ +import logging + from django.conf import settings from django.contrib.sessions.backends.db import SessionStore as DBStore from django.core.cache import caches KEY_PREFIX = "django.contrib.sessions.cached_db" +logger = logging.getLogger("django.contrib.sessions") + class SessionStore(DBStore): """ @@ -52,7 +56,10 @@ def exists(self, session_key): def save(self, must_create=False): super().save(must_create) - self._cache.set(self.cache_key, self._session, self.get_expiry_age()) + try: + self._cache.set(self.cache_key, self._session, self.get_expiry_age()) + except Exception: + logger.exception("Error saving to cache (%s)", self._cache) def delete(self, session_key=None): super().delete(session_key) diff --git a/docs/ref/logging.txt b/docs/ref/logging.txt index 6d8861299fcc..8a7e58997ebd 100644 --- a/docs/ref/logging.txt +++ b/docs/ref/logging.txt @@ -286,6 +286,17 @@ Messages to this logger have ``params`` and ``sql`` in their extra context (but unlike ``django.db.backends``, not duration). The values have the same meaning as explained in :ref:`django-db-logger`. +.. _django-contrib-sessions-logger: + +``django.contrib.sessions`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Log messages related to the :doc:`session framework`. + +* Non-fatal errors occurring when using the + :class:`django.contrib.sessions.backends.cached_db.SessionStore` engine are + logged as ``ERROR`` messages with the corresponding traceback. + Handlers -------- diff --git a/docs/releases/5.1.txt b/docs/releases/5.1.txt index 3fe0e65410ba..a4a7f359c6bc 100644 --- a/docs/releases/5.1.txt +++ b/docs/releases/5.1.txt @@ -115,7 +115,10 @@ Minor features :mod:`django.contrib.sessions` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -* ... +* :class:`django.contrib.sessions.backends.cached_db.SessionStore` now handles + exceptions when storing session information in the cache, logging proper + error messages with their traceback via the newly added + :ref:`sessions logger `. :mod:`django.contrib.sitemaps` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/topics/http/sessions.txt b/docs/topics/http/sessions.txt index 4f635f17041e..d799c245de93 100644 --- a/docs/topics/http/sessions.txt +++ b/docs/topics/http/sessions.txt @@ -76,9 +76,17 @@ Once your cache is configured, you have to choose between a database-backed cache or a non-persistent cache. The cached database backend (``cached_db``) uses a write-through cache -- -session writes are applied to both the cache and the database. Session reads -use the cache, or the database if the data has been evicted from the cache. To -use this backend, set :setting:`SESSION_ENGINE` to +session writes are applied to both the database and cache, in that order. If +writing to the cache fails, the exception is handled and logged via the +:ref:`sessions logger `, to avoid failing an +otherwise successful write operation. + +.. versionchanged:: 5.1 + + Handling and logging of exceptions when writing to the cache was added. + +Session reads use the cache, or the database if the data has been evicted from +the cache. To use this backend, set :setting:`SESSION_ENGINE` to ``"django.contrib.sessions.backends.cached_db"``, and follow the configuration instructions for the `using database-backed sessions`_. diff --git a/tests/cache/failing_cache.py b/tests/cache/failing_cache.py new file mode 100644 index 000000000000..e2f0043bb7b9 --- /dev/null +++ b/tests/cache/failing_cache.py @@ -0,0 +1,7 @@ +from django.core.cache.backends.locmem import LocMemCache + + +class CacheClass(LocMemCache): + + def set(self, *args, **kwargs): + raise Exception("Faked exception saving to cache") diff --git a/tests/sessions_tests/tests.py b/tests/sessions_tests/tests.py index 7e0677d08d98..c8c556b4dccb 100644 --- a/tests/sessions_tests/tests.py +++ b/tests/sessions_tests/tests.py @@ -517,6 +517,22 @@ def test_non_default_cache(self): with self.assertRaises(InvalidCacheBackendError): self.backend() + @override_settings( + CACHES={"default": {"BACKEND": "cache.failing_cache.CacheClass"}} + ) + def test_cache_set_failure_non_fatal(self): + """Failing to write to the cache does not raise errors.""" + session = self.backend() + session["key"] = "val" + + with self.assertLogs("django.contrib.sessions", "ERROR") as cm: + session.save() + + # A proper ERROR log message was recorded. + log = cm.records[-1] + self.assertEqual(log.message, f"Error saving to cache ({session._cache})") + self.assertEqual(str(log.exc_info[1]), "Faked exception saving to cache") + @override_settings(USE_TZ=True) class CacheDBSessionWithTimeZoneTests(CacheDBSessionTests):