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

Teardown functions sometimes receive spurious exceptions from Gunicorn #984

Closed
astonm opened this issue Feb 19, 2014 · 4 comments
Closed

Comments

@astonm
Copy link

astonm commented Feb 19, 2014

While using Flask-SQLAlchemy's SQLALCHEMY_COMMIT_ON_TEARDOWN feature, I noticed certain successful-looking requests didn't have their results committed to the database. After further investigation I discovered some exceptions from Gunicorn end up being passed into the teardown function Flask-SQLAlchemy uses to implement the post-request commit functionality.

I was able to reproduce the issue on a minimal case with the code below (as test.py):

from flask import Flask, redirect, request, url_for

app = Flask(__name__)
app.debug = True

@app.route("/")
def index():
    return "Hello World!"

@app.teardown_request
def teardown_request(exception=None):
    if exception:
        app.logger.error("Unexpected exception {}".format(exception))

if __name__ == "__main__":
    app.run()

With the app run with the following line:

gunicorn test:app -w 1

I fairly consistently get the following output:

2014-02-19 14:00:34 [4459] [INFO] Starting gunicorn 18.0
2014-02-19 14:00:34 [4459] [INFO] Listening at: http://127.0.0.1:8000 (4459)
2014-02-19 14:00:34 [4459] [INFO] Using worker: sync
2014-02-19 14:00:34 [4462] [INFO] Booting worker with pid: 4462
--------------------------------------------------------------------------------
ERROR in test [/Users/aston/.envs/flask-gunicorn-bug/test.py:12]:
Unexpected exception [Errno 35] Resource temporarily unavailable
--------------------------------------------------------------------------------

Errno 35 is EAGAIN.

I'm running on a Mac, but I first noticed this behavior on Heroku (it's Errno 11 there). Also, the behavior isn't consistent, and though I found it more consistently locally with app.debug = True, I noticed the behavior with debugging off (again, on Heroku).

There's been some discussion about this issue in Gunicorn's issues section here, but they consider EAGAIN a non-fatal exception (and as you can see from running the example, the requests always successfully return data), so the fact that Flask sees it and thinks it's real is a problem.

I believe Gunicorn could help fix this bug by clearing sys.exc_info() after ignoring the exception (i.e. around here: https://github.com/tilgovi/gunicorn/blob/master/gunicorn/workers/sync.py#L43:L45), but it'd still be racy.

The real fix, I believe is for Flask not to throw arbitrary "if not exc: exc = sys.exc_info()[1]" in areas where no exception is being explicitly caught by the code (e.g. here). Another alternative, if that behavior is generally desirable, would be to ignore any exceptions with a stack trace that doesn't go deep enough to include any Flask or user-provided code.

@DasIch
Copy link
Contributor

DasIch commented Mar 13, 2014

We do have to inspect sys.exc_info in several places simply due to Flask's design, that cannot be changed. Making assumptions by looking at the tracebacks might turn out to be a very expensive operation on e.g. PyPy. Overall I don't think there is a way we can really solve this within Flask.

@dcosson
Copy link

dcosson commented Nov 5, 2014

In case anyone else comes across this, I just ran into this issue and adding this extra teardown handler is what I'm using instead of SQLALCHEMY_COMMIT_ON_TEARDOWN. I was concerned it might get called after flask-sqlalchemy calls session.remove() but flask calls the teardown functions in reverse order so that's not an issue.

@EuphyZhao
Copy link

Looking at this issue at Pycon 2016 Sprints day. Is this fixed already? I can't reproduce the same error with the code provided by @astonm.

@astonm
Copy link
Author

astonm commented Jun 2, 2016

I'm no longer able to repro, either.

Maybe the change from pulling sys.exc_info()[1] when exc was None to instead checking for _sentinel may have done the trick?

@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

5 participants