python-restx / flask-restx

Fork of Flask-RESTPlus: Fully featured framework for fast, easy and documented API development with Flask
https://flask-restx.readthedocs.io/en/latest/
Other
2.16k stars 334 forks source link

Wrong status code and message on responses when handling `HTTPException`s #569

Closed lkk7 closed 10 months ago

lkk7 commented 1 year ago

Issue

There are two issues in handler_error method, in api.py.

  1. There might be a situation in Werkzeug where it raises an HTTPException(some_response). The code is in the response, but NOT in the HTTPException. Currently, Flask-RESTX checks gets the code only from the exception itself which will cause an error:

    ValueError: None is not a valid HTTPStatus
  2. This code in the method is wrong:

    default_data = {"message": getattr(e, "description", code.phrase)}

    getattr returns the default value (code.phrase) only if e.description doesn't exist, which will never happen. It's always set to None by default, so it exists.

The final result of these problems is that

  1. You get an error when an HTTPException is raised without a status code (which is expected), even if its response has a status code.
  2. If you fix that error and go further, you'll receive a null message instead of the default one.

Reproduction Steps

  1. Add and run the test in tests/test_errors.py. Conveniently it catches both problems.

    # ... Add this import
    from werkzeug import Response
    # ...
    
    def test_handle_error_http_exception_response_code_only(self, app):
        api = restx.Api(app)
        http_exception = HTTPException(response=Response(status=401))
    
        response = api.handle_error(http_exception) # <-- Will fail with "ValueError: None is not a valid HTTPStatus"
        assert response.status_code == 401 # <-- Doesn't get there, but when we fix the status code it will go further
        assert json.loads(response.data.decode()) == {
            "message": "Unauthorized",   # <-- Will fail, message is None
        }

Expected Behavior

The code and message should be set as expected (401, "Unauthorized")

Actual Behavior

Message is set to None, but it doesn't even happen because before that, we get: ValueError: None is not a valid HTTPStatus due to code being None.

Environment

lkk7 commented 1 year ago

Code ready https://github.com/python-restx/flask-restx/pull/570