Skip to content

Conversation

@jpcaruana
Copy link
Collaborator

@jpcaruana jpcaruana commented Apr 14, 2020

this fix does not work, I dont understand why

I also tried a simple except: with the same result

  • 8c6624b : show the same error as in my sentry issue
  • 1ef405e : does not work, but fails differently

any idea @MickaelBergem ?

does not fix #14

@jpcaruana
Copy link
Collaborator Author

jpcaruana commented Apr 14, 2020

same error as in my sentry issue:

tests/test_middleware_flask.py:107:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
../../../Library/Caches/pypoetry/virtualenvs/howfast-apm-nKXpZzMa-py3.6/lib/python3.6/site-packages/werkzeug/test.py:1029: in get
    return self.open(*args, **kw)
../../../Library/Caches/pypoetry/virtualenvs/howfast-apm-nKXpZzMa-py3.6/lib/python3.6/site-packages/flask/testing.py:227: in open
    follow_redirects=follow_redirects,
../../../Library/Caches/pypoetry/virtualenvs/howfast-apm-nKXpZzMa-py3.6/lib/python3.6/site-packages/werkzeug/test.py:993: in open
    response = self.run_wsgi_app(environ.copy(), buffered=buffered)
../../../Library/Caches/pypoetry/virtualenvs/howfast-apm-nKXpZzMa-py3.6/lib/python3.6/site-packages/werkzeug/test.py:884: in run_wsgi_app
    rv = run_wsgi_app(self.application, environ, buffered=buffered)
../../../Library/Caches/pypoetry/virtualenvs/howfast-apm-nKXpZzMa-py3.6/lib/python3.6/site-packages/werkzeug/test.py:1119: in run_wsgi_app
    app_rv = app(environ, start_response)
../../../Library/Caches/pypoetry/virtualenvs/howfast-apm-nKXpZzMa-py3.6/lib/python3.6/site-packages/flask/app.py:2463: in __call__
    return self.wsgi_app(environ, start_response)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <howfast_apm.flask.HowFastFlaskMiddleware object at 0x110d6ca20>, environ = {'HTTP_HOST': 'localhost', 'HTTP_USER_AGENT': 'werkzeug/0.15.5', 'PATH_INFO': '/error', 'QUERY_STRING': '', ...}
start_response = <function run_wsgi_app.<locals>.start_response at 0x110d3cf28>

    def __call__(self, environ, start_response):
        if not self.app_id:
            # HF APM not configured, return early to save some time
            # TODO: wouldn't it be better to just not replace the WSGI app?
            return self.wsgi_app(environ, start_response)

        uri = environ.get('PATH_INFO')

        if is_in_blacklist(uri, self.endpoints_blacklist):
            # Endpoint blacklist, return now
            return self.wsgi_app(environ, start_response)

        method = environ.get('REQUEST_METHOD')

        response_status: str = None

        def _start_response_wrapped(status, *args, **kwargs):
            nonlocal response_status
            # We wrap the start_response callback to access the response status line (eg "200 OK")
            response_status = status
            return start_response(status, *args, **kwargs)

        time_request_started = datetime.now(timezone.utc)

        try:
            # Time the function execution
            start = timer()
            return_value = self.wsgi_app(environ, _start_response_wrapped)
            # Stop the timer as soon as possible to get the best measure of the function's execution time
            end = timer()
        except Exception:
            # The WSGI app raised an exception, let's still save the point before raising the
            # exception again
            # First, "stop" the timer now to get the good measure of the function's execution time
            end = timer()
            # The real response status will actually be set by the server that interacts with the
            # WSGI app, but we cannot instrument it from here, so we just assume a common string.
            response_status = "500 INTERNAL SERVER ERROR"
            raise
        finally:
>           elapsed = end - start
E           UnboundLocalError: local variable 'end' referenced before assignment

howfast_apm/flask.py:94: UnboundLocalError

@jpcaruana
Copy link
Collaborator Author

with my failing fix:

tests/test_middleware_flask.py:107:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
../../../Library/Caches/pypoetry/virtualenvs/howfast-apm-nKXpZzMa-py3.6/lib/python3.6/site-packages/werkzeug/test.py:1029: in get
    return self.open(*args, **kw)
../../../Library/Caches/pypoetry/virtualenvs/howfast-apm-nKXpZzMa-py3.6/lib/python3.6/site-packages/flask/testing.py:227: in open
    follow_redirects=follow_redirects,
../../../Library/Caches/pypoetry/virtualenvs/howfast-apm-nKXpZzMa-py3.6/lib/python3.6/site-packages/werkzeug/test.py:993: in open
    response = self.run_wsgi_app(environ.copy(), buffered=buffered)
../../../Library/Caches/pypoetry/virtualenvs/howfast-apm-nKXpZzMa-py3.6/lib/python3.6/site-packages/werkzeug/test.py:884: in run_wsgi_app
    rv = run_wsgi_app(self.application, environ, buffered=buffered)
../../../Library/Caches/pypoetry/virtualenvs/howfast-apm-nKXpZzMa-py3.6/lib/python3.6/site-packages/werkzeug/test.py:1119: in run_wsgi_app
    app_rv = app(environ, start_response)
../../../Library/Caches/pypoetry/virtualenvs/howfast-apm-nKXpZzMa-py3.6/lib/python3.6/site-packages/flask/app.py:2463: in __call__
    return self.wsgi_app(environ, start_response)
howfast_apm/flask.py:81: in __call__
    return_value = self.wsgi_app(environ, _start_response_wrapped)
../../../Library/Caches/pypoetry/virtualenvs/howfast-apm-nKXpZzMa-py3.6/lib/python3.6/site-packages/flask/app.py:2446: in wsgi_app
    response = self.full_dispatch_request()
../../../Library/Caches/pypoetry/virtualenvs/howfast-apm-nKXpZzMa-py3.6/lib/python3.6/site-packages/flask/app.py:1949: in full_dispatch_request
    rv = self.dispatch_request()
../../../Library/Caches/pypoetry/virtualenvs/howfast-apm-nKXpZzMa-py3.6/lib/python3.6/site-packages/flask/app.py:1935: in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

    @app.route('/error')
    def error():
>       raise SystemExit()
E       SystemExit

tests/test_middleware_flask.py:31: SystemExit

@jpcaruana
Copy link
Collaborator Author

I also tried with raise GeneratorExit() with the same effect

@jpcaruana
Copy link
Collaborator Author

@jpcaruana jpcaruana requested a review from MickaelBergem April 14, 2020 15:00
@jpcaruana jpcaruana marked this pull request as draft April 14, 2020 15:00
@jpcaruana jpcaruana added bug Something isn't working help wanted Extra attention is needed labels Apr 14, 2020
@MickaelBergem
Copy link
Collaborator

Thank you for the test! So if I understand correctly, the APM middleware raises (with the error you reported) when the underlying Flask endpoint raises a SystemExit exception. I'll investigate.

@MickaelBergem
Copy link
Collaborator

It looks like your fix is working! I've tested by raising a SystemExit() from a Flask endpoint, and the middleware doesn't fail anymore (the server sends an empty reply, as when a SystemExit is raised in usual Flask applications):

* Connected to localhost (127.0.0.1) port 8081 (#0)
> GET /boom HTTP/1.1
> Host: localhost:8081
> User-Agent: curl/7.69.1
> Accept: */*
> 
* Empty reply from server
* Connection #0 to host localhost left intact
curl: (52) Empty reply from server

In the application log, the point is indeed saved but the base logger doesn't log the failed response (again, as usual with Flask + SystemExit).

The question is now "why doesn't PyTest catch the SystemExit() on tester.get" - might very well be by design. I'm looking into it.

@jpcaruana
Copy link
Collaborator Author

The question is now "why doesn't PyTest catch the SystemExit() on tester.get" - might very well be by design.

That's also my thinking

@MickaelBergem
Copy link
Collaborator

So after investigating a bit, it looks like Flask does not catch Errors like SystemExit, meaning they propagate up to the WSGI handler (gunicorn, etc) that decides what to do with it. Here there is only pytest, which has no reason to catch it. I've updated the test to make sure we take this into account.

@MickaelBergem MickaelBergem marked this pull request as ready for review April 16, 2020 13:47
@MickaelBergem MickaelBergem merged commit f364c4a into master Apr 16, 2020
@MickaelBergem MickaelBergem deleted the fix/local-variable-ennd-referenced-before-assignment branch April 16, 2020 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working help wanted Extra attention is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UnboundLocalError: local variable 'end' referenced before assignment

3 participants