martinrusev / django-redis-sessions

Session backend for Django that stores sessions in a Redis database
BSD 3-Clause "New" or "Revised" License
494 stars 106 forks source link

Any exception at all when loading results in an empty session #60

Open Drarok opened 5 years ago

Drarok commented 5 years ago

I've spent all morning tracking this problem down, but I think I've finally cracked it!

If there is any exception at all when attempting to load the session, the session key is cleared and an empty dict is returned.

The Django session middleware perform the following check:

empty = request.session.is_empty()
if settings.SESSION_COOKIE_NAME in request.COOKIES and empty:
    …

This means that if you encounter a socket read timeout when attempting to load the session from Redis, the user's sessionid cookie is deleted from their browser, and they are logged out.

This has been hitting me constantly in development lately, which has been hugely frustrating. I would suggest that catching a timeout and retrying, at least optionally, would be a good alternative.

martinrusev commented 5 years ago

@Drarok This behavior is consistent with the session engines present in django itself:

https://github.com/django/django/blob/master/django/contrib/sessions/backends/cache.py#L24 https://github.com/django/django/blob/master/django/contrib/sessions/backends/db.py#L42

You can work around this by tweaking the following 2 settings:

https://github.com/martinrusev/django-redis-sessions/blob/master/redis_sessions/settings.py#L8

SESSION_REDIS_SOCKET_TIMEOUT -> Defaults to 0.1
SESSION_REDIS_RETRY_ON_TIMEOUT -> Defaults to False
normanjaeckel commented 5 years ago

Sorry for the noise in #62. In https://code.djangoproject.com/ticket/29203 @timgraham stated that Django might

discourage use of the "ignore connection timeout exception" option.

So I think django-redis-sessions should raise the exception al least in case of timeout (at least if retry settings is false). What do you think?