HTTP Status Code 415 not handled by custom handler

[rite2hhh]

Description

I used OpenAPI tools to generate a Python-Connexion based server. This code-gen tool was provided an OpenAPI specification (OAS3). The OpenAPI tool gave server stubs which are bound to that spec. I am extending those for application specific response.

For starters, I decided to take care of error handling, and get that done. I wrote custom handlers for some common HTTP status codes (400, 401, 403, 404, 405, 415, 500) and used add_error_handler to add these error handlers to the connexion app, which in-turn registered these error handlers with the underlying Flask app (I am familiar with Flask).

Most all status codes show the response from the error handler I registered, but response for HTTP status code 415 doesn't do it, and sends standard response from connexion.problem.problem / connexion.exception.ProblemException.

Similar case with HTTP status code 400

Expected behavior

After an incorrect media type was sent to the server, the server should use the custom handler to handle and send the following response to the client:-

{
    "message":  "Unsupported Media Type Provided"
}

Actual behavior

Shows this instead:-

{
    "detail": "Invalid Content-type (text/plain), expected JSON data",
    "status": 415,
    "title": "Unsupported Media Type",
    "type": "about:blank"
}

Steps to reproduce

When an endpoint expecting application/json receives content of type text/plain the issue can be easily reproduced

Additional info:

errors.py

def unsupported_media(error):
    return jsonify({"message" : "Unsupported Media Type Provided"}), 415

__init__.py

app = connexion.App(__name__, specification_dir='./openapi/')
app.add_api('openapi.yaml')
app.add_error_handler(400, errors.bad_request)
app.add_error_handler(404, errors.not_found)
app.add_error_handler(415, errors.unsupported_media)
app.add_error_handler(500, errors.internal_error)

Edit (Jan 17, 2019):

I dug a little deeper into the implementation and found where Flask stores the error handlers - error_handler_spec, which is a nested dictionary created by _register_error_handler(self, key, code_or_exception, f) called by register_error_handler(self, code_or_exception, f), which itself is called by add_error_handler in connexion. The below code snippet shows that for 400 and 415 HTTP status codes, it does not use FlaskApp.common_error_handler, and should use the custom error handler function. The class field is the type of HTTPException. I have also kept other response code information for reference

400 {<class 'werkzeug.exceptions.BadRequest'>: <function bad_request at 0x7f93708e9268>}
401 {<class 'werkzeug.exceptions.Unauthorized'>: <function unauthorized at 0x7f9370343620>}
403 {<class 'werkzeug.exceptions.Forbidden'>: <function forbidden at 0x7f93700fec80>}
404 {<class 'werkzeug.exceptions.NotFound'>: <function not_found at 0x7f937010dea0>}
405 {<class 'werkzeug.exceptions.MethodNotAllowed'>: <function not_allowed at 0x7f937010df28>}
406 {<class 'werkzeug.exceptions.NotAcceptable'>: <function FlaskApp.common_error_handler at 0x7f9370c31d08>}
410 {<class 'werkzeug.exceptions.Gone'>: <function FlaskApp.common_error_handler at 0x7f9370c31d08>}
415 {<class 'werkzeug.exceptions.UnsupportedMediaType'>: <function unsupported_media at 0x7f9370115048>}
422 {<class 'werkzeug.exceptions.UnprocessableEntity'>: <function FlaskApp.common_error_handler at 0x7f9370c31d08>}
500 {<class 'werkzeug.exceptions.InternalServerError'>: <function internal_error at 0x7f93701150d0>}

As a side experiment, I ran a similar test on a pure flask application, which returned a response from the custom error handler for both HTTP status codes 400 and 415

Found the source of incorrect response I receive. It is handled by RequestBodyValidator in
connexion/decorators/validation.py #459

class RequestBodyValidator(object):
    ...
        @functools.wraps(function)
        def wrapper(request):
            if all_json(self.consumes):
                data = request.json

                empty_body = not(request.body or request.form or request.files)
                if data is None and not empty_body and not self.is_null_value_valid:
                    try:
                        ctype_is_json = is_json_mimetype(request.headers.get("Content-Type", ""))
                    except ValueError:
                        ctype_is_json = False

                    if ctype_is_json:
                        # Content-Type is json but actual body was not parsed
                        return problem(400,
                                       "Bad Request",
                                       "Request body is not valid JSON"
                                       )
                    else:
                        # the body has contents that were not parsed as JSON
                        return problem(415,
                                       "Unsupported Media Type",
                                       "Invalid Content-type ({content_type}), expected JSON data".format(
                                           content_type=request.headers.get("Content-Type", "")
                                       ))

Output of the commands:

• python --version == 3.5.2
• pip show connexion | grep "^Version\:" == 2.2.0

[dtkav]

Hey @chinmayprabhudesai thanks for the detailed ticket.
I agree it looks like the problem is here: https://github.com/zalando/connexion/blob/master/connexion/decorators/validation.py#L129

One way to do this would be to raise the correct werkzeug exceptions for 400, 415 in connexion. Unfortunately I'm not sure this would work for the other webserver (AioHttp). It actually seems like a potential problem that the connexion exceptions inherit from werkzeug.

For a near-term fix, you can also register arbitrary exceptions.
Can you please try handling ProblemException like so:

app.add_error_handler(ProblemException, my_handler)

You should have access to the status code in your handler, as it is a property of the ProblemException.

Let me know if that works for you. I'll think on how to support status_code exception handlers without breaking aiohttp as well.

[rite2hhh]

Thank you for the response @dtkav !

Let me add some more information that I did not provide earlier (slipped my mind).
I am using Flask as my under-the-hood framework.

I tried this before I filed the ticket, but forgot to include it.
When I do the following, my client receives the desired response!

client

$ http PUT http://<hostname>/user
HTTP/1.0 415 UNSUPPORTED MEDIA TYPE
Content-Length: 109
Content-Type: application/json
Date: Fri, 25 Jan 2019 00:32:37 GMT
Server: Werkzeug/0.14.1 Python/3.5.2

{
     "message": "Unsupported Media Type Provided"
}

$ http GET http://<hostname>/user
HTTP/1.0 400 BAD REQUEST
Content-Length: 89
Content-Type: application/json
Date: Fri, 25 Jan 2019 00:34:14 GMT
Server: Werkzeug/0.14.1 Python/3.5.2

{
    "message": "Bad Request"
}

app.py

from connexion.apps import flask_app
# OR
# from flask import abort

def user_get():  # noqa: E501
    """
    """
    return flask_app.flask.abort(400)
    # OR
    # return abort(400)


def user_put():  # noqa: E501
    """
    """
    return flask_app.flask.abort(415)
    # OR
    # return abort(415)

__init__.py

import connexion
from werkzeug.exceptions import BadRequest, UnsupportedMediaType

# Create Connexion application
app = connexion.App(__name__, specification_dir='./openapi/')

# Add Error Handlers
app.add_error_handler(BadRequest.code, errors.bad_request)
app.add_error_handler(UnsupportedMediaType.code, errors.unsupported_media)

errors.py

def bad_request(error):
     return jsonify({"message" : "Bad Request"}), 400

def unsupported_media(error):
    return jsonify({"message" : "Unsupported Media Type Provided"}),  415

assuming it went through all the validation and the request reached my api or app after the abstractions provided by connexion.

I forked the main repo thinking the exact same that returning a response from the problem function was breaking it. So I created an object of the ProblemException and returned obj.to_problem() which resulted in the same response.

It later occurred to me that my request never reaches the app/api; it is handled by the validator and that is where the issue lies.

The request is handled outside the scope of under-the-hood framework

A good way for me (maybe a workaround for now) would be to create my own custom validator and use that to parse these requests!

Edit (Jan 24, 2019):

Furthermore, I noticed that RequestBodyValidator has an api, and I thought maybe that could be used, to pass the request down the line, and have something like this

class RequestBodyValidator(object):

    ...

    def __call__(self, function):
    """
    :type function: types.FunctionType
    :rtype: types.FunctionType
    """

        @functools.wraps(function)
        def wrapper(request):
            if all_json(self.consumes):
                data = request.json

                empty_body = not(request.body or request.form or request.files)
                if data is None and not empty_body and not self.is_null_value_valid:
                    try:
                        ctype_is_json = is_json_mimetype(request.headers.get("Content-Type", ""))
                    except ValueError:
                        ctype_is_json = False

                    if ctype_is_json:
                        # Content-Type is json but actual body was not parsed
                        response = ProblemException(400,
                                                    "Bad Request",
                                                    "Request body is not valid JSON"
                                                    ).to_problem()
                        return self.api.get_response(response)
                    else:
                        # the body has contents that were not parsed as JSON
                        response = ProblemException(415,
                                                    "Unsupported Media Type",
                                                    "Invalid Content-type ({content_type}), expected JSON data".format(
                                                     content_type=request.headers.get("Content-Type", ""))
                                                    )
                        return self.api.get_response(response)

But at the moment I did not understand the structure very well.

I followed where RequestBodyValidator was used and found it under connexion/operations/abstract.py used as follows:

# imports here

# other definitions here

VALIDATOR_MAP = {
    'parameter': ParameterValidator,
    'body': RequestBodyValidator,
    'response': ResponseValidator,
}


@six.add_metaclass(abc.ABCMeta)
class AbstractOperation(SecureOperation):

    ...

    def __init__(self, api, method, path, operation, resolver,
                 app_security=None, security_schemes=None,
                 validate_responses=False, strict_validation=False,
                 randomize_endpoint=None, validator_map=None,
                 pythonic_params=False, uri_parser_class=None,
                 pass_context_arg_name=None):
        """
        :param api: api that this operation is attached to
        :type api: apis.AbstractAPI

        ...

        :param validator_map: Custom validators for the types "parameter", "body" and "response".
        :type validator_map: dict

        """
        self._api = api

        ...

        self._validator_map = dict(VALIDATOR_MAP)
        self._validator_map.update(validator_map or {})

Maybe adding my api to this validator will allow me to use the api in RequestBodyValidator and then my app/api handles it.

@dtkav, you would be the better judge here because you understand the code base a lot better than I do, and I think if it is done this way, AioHttp should not have a problem as well (not really sure about this). And/or, as mentioned earlier create a new validator and update the validator map!

[slothkong]

Dang! you carved out all the wrapping of connexion. If you dont mind oversimplifying the issue, to make the app able to return a simple 400 error, it is also necessary to use app.error_handler()?
In the README example they show the status can be simply returned along the desired response:

# api.py file

def foo_get(message):
    # do something
    return 'You send the message: {}'.format(message), 200

On the other side, if a 4XX error type is added to the .yaml API description and OpenApi Tool is used , then an Error class is autogenerated. However, returning a Error object does not actually set the status of the HTTP response, so is possible to send to the client an error as request body but a 200 OK as status (in the header), What obvious thing did I miss? Should I start implementing these app.error_handler()s or is it necessary to manually extract the message and code attributes of the Error object and return them as in the README example?

[rite2hhh]

Hello @slothkong,

If you dont mind oversimplifying the issue, to make the app able to return a simple 400 error, it is also necessary to use app.error_handler()?

No. It's not necessary/mandatory to implement error_handler's. In my specific case, the spec I was using required me to provide a specific body in the error response and hence my implementation is such. You can use the standard error response body from werkzeug or other such library, and you won't have to worry about implementing these error_handler's.

On the other side, if a 4XX error type is added to the .yaml API description and OpenApi Tool is used , then an Error class is autogenerated. However, returning a Error object does not actually set the status of the HTTP response,

Not really sure about this.

So is possible to send to the client an error as request body but a 200 OK as status (in the header), What obvious thing did I miss?

You should avoid doing something like that. It defeats the purpose of the HTTP status codes. Each status code came into existence for a use case. You should use correct HTTP status codes; it's the first identifier a client would use to enforce correct responses. If you say 200 OK, but it's actually 400 Bad Request, or 401 Unauthorized or 405 Method Not Allowed; a client will be provided misleading info even though your body may indicate error messages from these exact errors. It is bad practice (and may also be against the RFC - but don't quote me on it)

Should I start implementing these app.error_handler()s or is it necessary to manually extract the message and code attributes of the Error object and return them as in the README example?

Again, if you are happy with the error description provided in the underlying packages, you can just let your framework handler all errors

Hope these clarify your concerns.

[RobbeSneyders]

Can no longer reproduce