Infinite loop and memory leak caused by the `omit_exceptions` decorator
HosseyNJF opened this issue · 2 comments
Describe the bug
When the Redis server is down, a ConnectionError
is raised. The omit_exceptions
decorator will check the ignore_exceptions settings and if it is set to False
, it will re-raise the __cause__
of the raised exception, which is a ConnectionInterruped
with a cause of redis.ConnectionError
. But somehow the __context__
of the redis error is equal to the ConnectionInterruped
exception itself. This causes an infinite loop in the django.views.debug.technical_500_response
method that tries to print the traceback but finds itself in a never-ending chain of causes and contexts.
To Reproduce
Steps to reproduce the behavior:
- Stop the Redis server.
- Set
DEBUG = True
andDJANGO_REDIS_IGNORE_EXCEPTIONS = False
. - Try to access the cache in any view.
- Watch as the process hangs and eventually gets OOM killed.
Expected behavior
I expected to simply see a ConnectionInterrupted
exception in my console and in my response because the connection failed.
Environment (please complete the following information):
- Python version: 3.9
- Django Redis Version: 5.2.0
- Django Version: 2.2
- redis Version: irrelevant
- redis-py Version: 4.5.1
Additional context
The problem only happens when Django's DEBUG = True
is set, as it will make Django try to print the traceback. Also, the DJANGO_REDIS_IGNORE_EXCEPTIONS
setting must be set to False
to force the omit_exceptions
decorator to re-raise the exception's __cause__
. The problem doesn't seem to happen when python itself is trying to print the traceback, and I don't know why. Maybe they expected someone to raise a circular exception and had measures for that?
Maybe try this. I did not test this.
Modify the omit_exceptions decorator to handle the ConnectionError exception more appropriately. The current behavior of re-raising the exception cause with a ConnectionInterrupted exception and setting the context to the ConnectionInterrupted exception itself is what causes the infinite loop when Django's DEBUG is set to True.
Instead, we can modify the omit_exceptions decorator to directly re-raise the original ConnectionError exception and skip the ConnectionInterrupted exception altogether, since it's an unnecessary step. Here's the updated code for the omit_exceptions decorator:
from functools import wraps
from django.conf import settings
from redis import ConnectionError
def omit_exceptions(fn):
"""
Decorator that catches ConnectionError exceptions
if DJANGO_REDIS_IGNORE_EXCEPTIONS is set, logs an error,
and returns None.
"""
@wraps(fn)
def wrapper(args, **kwargs):
try:
return fn(args, **kwargs)
except ConnectionError as e:
if getattr(settings, 'DJANGO_REDIS_IGNORE_EXCEPTIONS', False):
msg = 'Ignoring redis cache exception: {}'.format(e)
if hasattr(e, 'args') and e.args:
msg+= ". {}.".format(e.args[0])
logger.error(msg)
return None
else:
raise # Re-raise the original ConnectionError exception
return wrapper
With this modification to the code, the omit_exceptions decorator will now directly re-raise the original ConnectionError exception when Django's DEBUG is set to True, preventing the infinite loop bug when using redis cache with Django.