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

Cannot inject into teardown_request handler while using request scope #42

Closed
ollien opened this issue Jan 4, 2020 · 10 comments · Fixed by #45
Closed

Cannot inject into teardown_request handler while using request scope #42

ollien opened this issue Jan 4, 2020 · 10 comments · Fixed by #45

Comments

@ollien
Copy link

ollien commented Jan 4, 2020

If I have a request scoped variable, I cannot inject it into a teardown_request handler. If I do, I get the following exception.

Error on request:
Traceback (most recent call last):
  File "/home/nick/.local/share/virtualenvs/flask_injector_poc-sJYA9nHW/lib/python3.7/site-packages/werkzeug/local.py", line 72, in __getattr__
    return self.__storage__[self.__ident_func__()][name]
KeyError: 139781434668800

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/nick/.local/share/virtualenvs/flask_injector_poc-sJYA9nHW/lib/python3.7/site-packages/werkzeug/serving.py", line 304, in run_wsgi
    execute(self.server.app)
  File "/home/nick/.local/share/virtualenvs/flask_injector_poc-sJYA9nHW/lib/python3.7/site-packages/werkzeug/serving.py", line 292, in execute
    application_iter = app(environ, start_response)
  File "/home/nick/.local/share/virtualenvs/flask_injector_poc-sJYA9nHW/lib/python3.7/site-packages/flask/app.py", line 2463, in __call__
    return self.wsgi_app(environ, start_response)
  File "/home/nick/.local/share/virtualenvs/flask_injector_poc-sJYA9nHW/lib/python3.7/site-packages/flask/app.py", line 2457, in wsgi_app
    ctx.auto_pop(error)
  File "/home/nick/.local/share/virtualenvs/flask_injector_poc-sJYA9nHW/lib/python3.7/site-packages/flask/ctx.py", line 452, in auto_pop
    self.pop(exc)
  File "/home/nick/.local/share/virtualenvs/flask_injector_poc-sJYA9nHW/lib/python3.7/site-packages/flask/ctx.py", line 415, in pop
    self.app.do_teardown_request(exc)
  File "/home/nick/.local/share/virtualenvs/flask_injector_poc-sJYA9nHW/lib/python3.7/site-packages/flask/app.py", line 2299, in do_teardown_request
    func(exc)
  File "/home/nick/.local/share/virtualenvs/flask_injector_poc-sJYA9nHW/lib/python3.7/site-packages/flask_injector.py", line 89, in wrapper
    return injector.call_with_injection(callable=fun, args=args, kwargs=kwargs)
  File "/home/nick/.local/share/virtualenvs/flask_injector_poc-sJYA9nHW/lib/python3.7/site-packages/injector/__init__.py", line 972, in call_with_injection
    owner_key=self_.__class__ if self_ is not None else callable.__module__,
  File "/home/nick/.local/share/virtualenvs/flask_injector_poc-sJYA9nHW/lib/python3.7/site-packages/injector/__init__.py", line 96, in wrapper
    return function(*args, **kwargs)
  File "/home/nick/.local/share/virtualenvs/flask_injector_poc-sJYA9nHW/lib/python3.7/site-packages/injector/__init__.py", line 1017, in args_to_inject
    instance = self.get(interface)  # type: Any
  File "/home/nick/.local/share/virtualenvs/flask_injector_poc-sJYA9nHW/lib/python3.7/site-packages/injector/__init__.py", line 915, in get
    result = scope_instance.get(interface, binding.provider).get(self)
  File "/home/nick/.local/share/virtualenvs/flask_injector_poc-sJYA9nHW/lib/python3.7/site-packages/flask_injector.py", line 265, in get
    return self._locals.scope[key]
  File "/home/nick/.local/share/virtualenvs/flask_injector_poc-sJYA9nHW/lib/python3.7/site-packages/werkzeug/local.py", line 74, in __getattr__
    raise AttributeError(name)
AttributeError: scope

After some digging, it seems that this is because Flask executes teardown handlers in the reverse order that they are inserted (see relevant source). Because flask_injector inserts the request scope clean up as a teardown handler, after all others are inserted, flask_injector's handler is run before mine.

I was able to hack around this by, after setting up my FlaskInjector running app.teardown_request_funcs[None].reverse(). This is not a great solution, IMO. I'm hesitant to submit a PR that just inserts this teardown handler as the first one, as the Flask docs don't seem to guarantee that the handlers are called in reverse order; it seems like an implementation detail.

What follows is an (admittedly silly) proof of concept that produces the above exception on request.

import datetime
import flask
import flask_injector
import injector

app = flask.Flask(__name__)


@app.route('/')
def index(time: datetime.datetime):
    return f'The time is {time.isoformat()}'


@app.teardown_request
def teardown_handler(err: Exception, time: datetime.datetime):
    print(f'Tearing down the request from {time.isoformat()}')


flask_injector.FlaskInjector(app=app, modules=[
    lambda binder: binder.bind(datetime.datetime, to=datetime.datetime.now(), scope=flask_injector.request)
])

if __name__ == '__main__':
    # HACK: works around the bug.
    # app.teardown_request_funcs[None].reverse()
    app.run()
@ollien ollien changed the title Using teardown_request with request scope Cannot inject into teardown_request handler while using request scope Jan 4, 2020
@jstasiak
Copy link
Collaborator

jstasiak commented Jan 7, 2020

Thanks for the report, this is something I think should be supported. I understand your hesitation, but I don't see any way to work around this other than to insert the teardown handler as the last one – with a proper test to make sure things keep working. I'll do that and make a release soon. The worst case scenario is this is changed in the future but I guess we'll have to cross that bridge once we reach it.

Permalink to the referenced Flask source code piece for historical visibility.

@ollien
Copy link
Author

ollien commented Jan 7, 2020

Well, inserted as the first one, but yes, same concept. :)

If I find the time in the next day or so, I can open a PR with these changes. Like you said, we can cross the bridge when we come to it.

@jstasiak
Copy link
Collaborator

jstasiak commented Jan 7, 2020

Sorry, yes, the first one. I'll patch this today, but thanks anyway for wanting to help with it.

@jstasiak
Copy link
Collaborator

jstasiak commented Jan 7, 2020

The fix has just been released in version 0.12.1.

@Chratho
Copy link

Chratho commented Feb 21, 2020

Using version 0.12.2 of the plugin I still see flask-injector's teardown handler executing before my apps teardown handler, making injections into these handlers fail. Using blueprints, fwiw.

@jstasiak
Copy link
Collaborator

What versions of Python and Flask are you using? Can you provide a piece of code to reproduce the incorrect behavior? I'm afraid the issue will be difficult to narrow down otherwise.

@Chratho
Copy link

Chratho commented Feb 22, 2020

Python is 3.7.4, Flask is 1.1.0. The exception prints exactly as the one posted by @ollien, but I can try to provide some simplified sample in the evening. Could also upgrade Python and Flask to the latest versions if you wish.

@jstasiak
Copy link
Collaborator

I think this reproduces the issue so no need for further investigation on your part:

import random
from flask import blueprint, flask
from flask_injector import flaskinjector, request

blueprint = blueprint('blueprint', __name__)


class x:
    def __init__(self, value):
        self.value = value


@blueprint.route('/')
def index(x: x):
    print(f'index, x.value: {x.value}')
    return 'ok'


@blueprint.teardown_request
def teardown_handler(err: exception, x: x):
    print(f'teardown_handler, x.value: {x.value}')


def configure(binder):
    binder.bind(x, to=lambda: x(random.randint(0, 1_000_000)), scope=request)


def create_app():
    app = flask(__name__)
    app.register_blueprint(blueprint)
    flaskinjector(app, modules=[configure])
    return app


if __name__ == '__main__':
    app = create_app()
    app.run()

@jstasiak
Copy link
Collaborator

Flask-Injector 0.12.3 I just released should fix this for you.

@Chratho
Copy link

Chratho commented Feb 22, 2020

Can confirm its working now. Many thanks for this amazingly fast fix!

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

Successfully merging a pull request may close this issue.

3 participants