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

ValidationError causes HTTP/500 in Flask instead of HTTP/422 in Python 2 #122

Closed
frol opened this issue Sep 22, 2016 · 8 comments

Comments

@frol
Copy link
Contributor

commented Sep 22, 2016

Here are the symptoms: http://stackoverflow.com/questions/37321835/flask-error-handler-not-able-to-handle-assertion-error/39624893#39624893

Here is the case where I personally hit the issue with Python 2.x and PyPy in Python 2.x mode only: https://travis-ci.org/frol/flask-restplus-server-example/builds/161685419

After troubleshooting, I discovered that this issue stems from the FlaskParser.handle_error which calls abort function with exc= argument, which then is set as "data" on the raised HTTPException, which in its turn is tried to be json-dumped and fails with TypeError, and now the new exception is in sys.exc_info(), which causes Flask to assert here.

Searching git history for the introduced exc= in FlaskParser, I found it here: 6f8088c

Is there a reason for exc= to be passed? How can we fix this?

Python 3 seems to have changed something with sys.exc_info() behaviour since even after the TypeError caused by json.dumps(), sys.exc_info() still reports HTTPException while in Python 2 it returns TypeError.

@nickw444

This comment has been minimized.

Copy link

commented Oct 13, 2016

I'm not entirely sure if this is an error on my part, but I think this has caused issued with a few of my project's error handlers.

My error handlers are no longer being trapped into. Simply making a flask error handler for HTTP 422 which prints "hello world" doesn't run, and the browser responds with:

{
  "messages": {
    "field": ["Some error."]
  }
}

Rolling back to 1.3.4 fixes this (as this was the old functionality).

I'm running Python 3.5 by the way

@frol

This comment has been minimized.

Copy link
Contributor Author

commented Oct 13, 2016

@nickw444 Your error handlers were working because HTTP/500 occurred during the default handling, so you, actually, depended on a buggy implementation. You need to override Webargs Parser error_handler and use your customized Parser class.

@nickw444

This comment has been minimized.

Copy link

commented Oct 13, 2016

Interesting. I just isolated this into it's own thing. I think this actually is happening due to something with the way I'm using Flask-Restful.

The sample:

"""Requires: flask-restful, flask, webargs"""
from flask import Flask, jsonify
from webargs.flaskparser import use_args
from webargs import fields
from flask_restful import Api, Resource

app = Flask(__name__)
api = Api(app)

args = {
    'field': fields.Integer(required=True)
}


class MyResource(Resource):
    @use_args(args)
    def get(self):
        return "Hello World2"

api.add_resource(MyResource, '/test')


@app.errorhandler(422)
def errorhandler(err):
    print(err)
    return jsonify({'error': 'whoops'})


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


if __name__ == '__main__':
    app.run(debug=True)

Accessing the flask endpoint /:

Response:

$ curl -v http://127.0.0.1:5000/
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5000 (#0)
> GET / HTTP/1.1
> Host: 127.0.0.1:5000
> User-Agent: curl/7.49.1
> Accept: */*
>
* HTTP 1.0, assume close after body
< HTTP/1.0 200 OK
< Content-Type: application/json
< Content-Length: 24
< Server: Werkzeug/0.11.11 Python/3.5.1
< Date: Thu, 13 Oct 2016 07:41:57 GMT
<
{
  "error": "whoops"
}

As expected. The error gets handled with the error handler, which returns a 200 (In reality I would still return the 422).

Using the flask-restful resource:

$ curl -v http://127.0.0.1:5000/test
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5000 (#0)
> GET /test HTTP/1.1
> Host: 127.0.0.1:5000
> User-Agent: curl/7.49.1
> Accept: */*
>
* HTTP 1.0, assume close after body
< HTTP/1.0 422 UNPROCESSABLE ENTITY
< Content-Type: application/json
< Content-Length: 104
< Server: Werkzeug/0.11.11 Python/3.5.1
< Date: Thu, 13 Oct 2016 07:42:41 GMT
<
{
    "messages": {
        "field": [
            "Missing data for required field."
        ]
    }
}
* Closing connection 0

The thing to note here is that it's still returning a 422, but it's not being trapped by the error handler. I think i'm going to point the finger at flask-restful here. I'll try dig into the error handling there and see what the deal is.

If you've got any suggestions, that would be fantastic.

@nickw444

This comment has been minimized.

Copy link

commented Oct 13, 2016

Just had a quick play with the flask-restful code. It looks like it's hijacking the handling of all HTTPExceptions, stopping the app error handler from getting it's hands on it. And this makes sense why it was working before, since a non-http error was being thrown.

@frol

This comment has been minimized.

Copy link
Contributor Author

commented Oct 13, 2016

Here is what happened before my fix:

  1. Request is received
  2. Webargs parsing it in FlaskParser
  3. If validation failed, handle_error is called
  4. handle_error called abort(status_code, messages=error.messages, exc=error)
  5. abort patches the HTTPException passing all **kwargs (messages and exc) to HTTPException.data
  6. Flask-RESTful gets this data, and tries to serialize it to JSON
  7. json.dumps fails because it cannot serialize exc object passed through abort (steps 4-5)
  8. Flask-RESTful catches JSON-Encode-Error (TypeError) and passes this handling to your app.error_handler, and here the Python 2 and Python 3 difference jumps in, Python 3 has the long story of exceptions and the original exception was HTTPException/422, but Python 2 overrides this info with the latest JSON-Encode-Error (TypeError) and, as a result, you have HTTP/500.

In any case, Webargs abort implementation didn't work, it led to the JSON-Encode-Error exception which was handled expectedly on Python 3 only by a pure luck.

@nickw444

This comment has been minimized.

Copy link

commented Oct 13, 2016

Wow that makes perfect sense! Thanks for the in depth explanation.

So from here it looks like I have 2 choices

  1. Override error_handler in webargs.flaskparser to return the error message to the client?
  2. Submit a pull for restful to allow propagation of HTTPExceptions (which is unlikely to be merged)

I guess option 1 would be the one to take. It's unfortunate that flask-restful doesn't have options to allow bubbling up the exception.

@frol

This comment has been minimized.

Copy link
Contributor Author

commented Oct 13, 2016

I took the option 1 in my project: https://github.com/frol/flask-restplus-server-example/blob/8118f66e3c9c505da1b92f3373c4b3c583af3bf6/app/extensions/api/webargs_parser.py

I am not sure how this can be handled in a better way, so if you happen to find a better solution, please, share.

@nickw444

This comment has been minimized.

Copy link

commented Oct 14, 2016

Thanks for the example. I ended up implementing this kind of differently. I wanted to break out of Flask-Restful entirely if a validation exception occurred, since I didn't want the response to be subject to the flask-restful representation handler response.

So now my parser just raises the original ValidationError, which can be handled in Flask using a standard error handler for ValidationError.

This works well, because I can use the same handler inside, and outside of Flask-Restful.

https://github.com/TwoPiCode/twopi-flask-utils/blob/master/twopi_flask_utils/webargs/__init__.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.