getsentry / raven-python

Raven is the legacy Python client for Sentry (getsentry.com) — replaced by sentry-python
https://sentry.io
BSD 3-Clause "New" or "Revised" License
1.68k stars 657 forks source link

Flask extension raises StopIteration during certain tests #514

Open maxcountryman opened 9 years ago

maxcountryman commented 9 years ago

It seems that the Flask extension is causing the Werkzeug test client to raise a StopIteration exception.

Here's part of the traceback:

_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
virtualenv/lib/python2.7/site-packages/werkzeug/test.py:772: in post
    return self.open(*args, **kw)
virtualenv/lib/python2.7/site-packages/flask/testing.py:108: in open
    follow_redirects=follow_redirects)
virtualenv/lib/python2.7/site-packages/werkzeug/test.py:736: in open
    response = self.run_wsgi_app(environ, buffered=buffered)
virtualenv/lib/python2.7/site-packages/werkzeug/test.py:659: in run_wsgi_app
    rv = run_wsgi_app(self.application, environ, buffered=buffered)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

app = <Flask 'honeycomb'>
environ = {'CONTENT_LENGTH': '29', 'CONTENT_TYPE': 'application/json', 'HTTP_CONTENT_LENGTH': '29', 'HTTP_CONTENT_TYPE': 'application/json', ...}
buffered = False

    def run_wsgi_app(app, environ, buffered=False):
        """Return a tuple in the form (app_iter, status, headers) of the
        application output.  This works best if you pass it an application that
        returns an iterator all the time.

        Sometimes applications may use the `write()` callable returned
        by the `start_response` function.  This tries to resolve such edge
        cases automatically.  But if you don't get the expected output you
        should set `buffered` to `True` which enforces buffering.

        If passed an invalid WSGI application the behavior of this function is
        undefined.  Never pass non-conforming WSGI applications to this function.

        :param app: the application to execute.
        :param buffered: set to `True` to enforce buffering.
        :return: tuple in the form ``(app_iter, status, headers)``
        """
        environ = _get_environ(environ)
        response = []
        buffer = []

        def start_response(status, headers, exc_info=None):
            if exc_info is not None:
                reraise(*exc_info)
            response[:] = [status, headers]
            return buffer.append

        app_iter = app(environ, start_response)

        # when buffering we emit the close call early and convert the
        # application iterator into a regular list
        if buffered:
            close_func = getattr(app_iter, 'close', None)
            try:
                app_iter = list(app_iter)
            finally:
                if close_func is not None:
                    close_func()

        # otherwise we iterate the application iter until we have
        # a response, chain the already received data with the already
        # collected data and wrap it in a new `ClosingIterator` if
        # we have a close callable.
        else:
            while not response:
>               buffer.append(next(app_iter))
E               StopIteration

virtualenv/lib/python2.7/site-packages/werkzeug/test.py:873: StopIteration

This only seems to happen for tests that use the test client and make POSTs. Interestingly if I use the Sentry middleware directly, I'm not able to reproduce the error leading me to believe the problem lies in the Flask extension. However, so far I haven't been able to isolate the exact cause.

aop commented 9 years ago

I'm experiencing the same problem. Also using tester to make a POST

maxcountryman commented 9 years ago

@aop What I ended up doing was writing my own version of the middleware and using that directly. Everything has been fine since.

theY4Kman commented 9 years ago

@maxcountryman Could you expand on this? I'm running into the same problem (though, using a DELETE), and I'm lost

xordoquy commented 9 years ago

Unfortunately, I don't have the time to dig through Flask to fix it by myself. If someone can come with a PR I'd be happy to include it.

maxcountryman commented 9 years ago

@xordoquy it's a bug in raven-python (or rather in the Flask extension therein) but I don't really have time to track it down. I just ended up writing my own minimal middleware. Specifically my middleware only implements the try/catch logic, invoking Sentry--that's good enough for my purposes.

class SentryMiddleware(raven.middleware.Sentry):
    def __call__(self, environ, start_response):
        self.client.http_context(self.get_http_context(environ))

        try:
            return self.application(environ, start_response)
        except Exception:
            self.client.captureException()
            raise
        finally:
            self.client.context.clear()
theY4Kman commented 9 years ago

I'm no longer sure my error stems from the same issue, but I did figure out what was giving me grief, and maybe someone will benefit from an explanation: I was returning a 204, which has no response body. Thus, when next(app_iter) was run by werkzeug, it had nothing left to iterate, and the StopIteration bubbled up. My solution was to pass buffered=True (using a custom flask.testing.FlaskClient), which doesn't call next() directly, and avoids that problem.

maxcountryman commented 9 years ago

@theY4Kman ah, that probably explains my issue too.

xordoquy commented 9 years ago

@maxcountryman what I meant is I don't have the time to learn Flask to investigate the Flask extension issue. PR welcomed !

dcramer commented 9 years ago

I've confirmed this 204 issue, though I never connected it to Sentry. I'm wondering if this is actual an error with the flask test runner. I have a bunch of code with # XXX(dcramer): 204 break tests, so we return a 200 just because of this.

maxcountryman commented 9 years ago

@dcramer The Flask test runner works fine sans Sentry.

@xordoquy Sorry, I also don't have the time, which is why I just threw together the simplest solution.

ryanio commented 9 years ago

+1. Sentry was breaking one of my tests running DELETE, adding buffered=True solved the problem. Thanks @theY4Kman!

immunda commented 9 years ago

Yep, I'm coming across this too.

hughes commented 9 years ago

+1 Test failing on 204 response when sentry is present

matteosimone commented 9 years ago

+1 Ran into this while integrating sentry, fixed with buffered=True

PhilipGarnero commented 9 years ago

This is because of sentry wrapping the default wsgi app with its own. To disable this, you can either run your app in debug, init sentry with Sentry(app, wrap_wsgi=False) or sentry.init_app(app, wrap_wsgi=False) or re-override the wsgi_app using

from raven.middleware import Sentry
if isinstance(app.wsgi_app, Sentry):
    app.wsgi_app = app.wsgi_app.application

The sentry middleware needs to be fixed in order to avoid such hacks.

clintecker commented 8 years ago

FWIW this is still happening

georgexsh commented 8 years ago

test case to reproduce:

diff --git a/tests/contrib/flask/tests.py b/tests/contrib/flask/tests.py
index 0530216..7584ae5 100644
--- a/tests/contrib/flask/tests.py
+++ b/tests/contrib/flask/tests.py
@@ -286,6 +286,11 @@ class FlaskTest(BaseTest):
     def test_check_client_type(self):
         self.assertRaises(TypeError, lambda _: Sentry(self.app, "oops, I'm putting my DSN instead"))

+    def test_options_request(self):
+        # should not raise `StopIteration`
+        rv = self.client.options('/message/')
+        assert rv.status_code == 200
+

 class FlaskLoginTest(BaseTest):
georgexsh commented 8 years ago

is it more likely a issue in werkzeug.test.run_wsgi_app? ref: https://github.com/pallets/werkzeug/issues/618