django-redis-sessions icon indicating copy to clipboard operation
django-redis-sessions copied to clipboard

Improve exception handling when loading a session

Open gtnx opened this issue 6 years ago • 3 comments

We've recently encountered instability issues with a redis instance in our infrastructure. This generated many user unwanted disconnections but no errors were tracked server side. The problem was that we had TimeoutError when querying redis and that those errors were silently ignored by redis_sessions.

There are two methods of SessionStore which ignores the errors:

It looks a bad design to me for two reasons:

  • It's not recommended by pyqca: it's the error E722
  • It's terrible in terms of monitoring

I suggest to remove those try/except as

  • The StrictRedis.get used in SessionStore.load and StrictRedis.delete in SessionStore.delete does not raise exceptions if the key does not exist
  • The decode method already handle exceptions in case of corrupted data

What do you think?

gtnx avatar Dec 18 '18 09:12 gtnx

Any thoughts?

gtnx avatar Jan 21 '19 13:01 gtnx

Maybe this issue is related to https://code.djangoproject.com/ticket/29203

If connection to redis timed out, django thinks the session is deleted and removes the session cookie during response. So the client is logged off although it might reload or reconnect some seconds later.

Here I would expect a HTTP 5xx.

normanjaeckel avatar Apr 01 '19 22:04 normanjaeckel

Any news?

Allan-Nava avatar Sep 16 '21 14:09 Allan-Nava