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

Exception while opening session causes auto_pop not to be executed on the context #2517

Closed
ottonello opened this issue Nov 10, 2017 · 2 comments
Milestone

Comments

@ottonello
Copy link

ottonello commented Nov 10, 2017

Expected Behavior

auto_pop of the request context should be executed regardless of whether an exception happened after pushing the context(for example, when opening a session).

We're using Redis sessions from flask_session:

def _setup_session(app):
    from util.cache.region import cache_region
    from dogpile.cache.backends.null import NullBackend
    from flask_session import RedisSessionInterface

    if not isinstance(cache_region.backend, NullBackend):
        redis = cache_region.backend.client
        session_interface = RedisSessionInterface(redis, 'test.session.', use_signer=False, permanent=False)
        app.session_interface = session_interface

Actual Behavior

The request context is not popped whenever there is a timeout getting the session from Redis.
We can see in following requests the 'g' object still holds information from the failing request.

In ctx.py the context is first pushed and then the session is opened:

        _request_ctx_stack.push(self)

        # Open the session at the moment that the request context is
        # available. This allows a custom open_session method to use the
        # request context (e.g. code that access database information
        # stored on `g` instead of the appcontext).
        self.session = self.app.open_session(self.request)

But if open_session() fails, then ctx.auto_pop() is not executed. Perhaps ctx.push() should also be under the try...finally block in app.py?

This is the exception that triggers the context not to be popped:

redis.exceptions:TimeoutError: Timeout reading from socket
Traceback (most recent call last):
File "/var/www/censored_production/env/lib/python2.7/site-packages/newrelic-2.74.0.54/newrelic/api/web_transaction.py", line 738, in __iter__
File "/var/www/censored_production/env/lib/python2.7/site-packages/newrelic-2.74.0.54/newrelic/api/web_transaction.py", line 1114, in __call__
File "/var/www/censored_production/env/lib/python2.7/site-packages/werkzeug/contrib/fixers.py", line 152, in __call__
File "/var/www/censored_production/www/censored/util/logging.py", line 136, in __call__
File "/var/www/censored_production/env/lib/python2.7/site-packages/flask/app.py", line 1813, in wsgi_app
File "/var/www/censored_production/env/lib/python2.7/site-packages/flask/ctx.py", line 321, in push
File "/var/www/censored_production/env/lib/python2.7/site-packages/flask/app.py", line 825, in open_session
File "/var/www/censored_production/env/lib/python2.7/site-packages/flask_session/sessions.py", line 132, in open_session
File "/var/www/censored_production/env/lib/python2.7/site-packages/newrelic-2.74.0.54/newrelic/hooks/datastore_redis.py", line 67, in _nr_wrapper_Redis_method_
File "/var/www/censored_production/env/lib/python2.7/site-packages/redis/client.py", line 880, in get
File "/var/www/censored_production/env/lib/python2.7/site-packages/redis/client.py", line 573, in execute_command
File "/var/www/censored_production/env/lib/python2.7/site-packages/redis/client.py", line 585, in parse_response
File "/var/www/censored_production/env/lib/python2.7/site-packages/redis/connection.py", line 577, in read_response
File "/var/www/censored_production/env/lib/python2.7/site-packages/redis/connection.py", line 238, in read_response
File "/var/www/censored_production/env/lib/python2.7/site-packages/redis/connection.py", line 168, in readline
File "/var/www/censored_production/env/lib/python2.7/site-packages/redis/connection.py", line 139, in _read_from_socket

Environment

  • Python version: 2.7.6
  • Flask version: 0.10.1
  • Werkzeug version: 0.11.11
@ottonello
Copy link
Author

Does anybody agree? Notice this specific issue can also be fixed by catching the timeout in flask_session but I still think Flask should handle this gracefully and clean up the context.

@davidism
Copy link
Member

davidism commented Jan 4, 2018

Duplicate of #1528, fixed in #2254. This will be in the next release.

@davidism davidism closed this as completed Jan 4, 2018
@davidism davidism added this to the 1.0 milestone Jan 4, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants