Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve exception handling when loading a session #62

Open
gtnx opened this issue Dec 18, 2018 · 3 comments
Open

Improve exception handling when loading a session #62

gtnx opened this issue Dec 18, 2018 · 3 comments

Comments

@gtnx
Copy link

gtnx commented Dec 18, 2018

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?

@martinrusev martinrusev pinned this issue Dec 18, 2018
@gtnx
Copy link
Author

gtnx commented Jan 21, 2019

Any thoughts?

@normanjaeckel
Copy link

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.

@Allan-Nava
Copy link

Any news?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants