Improving error handlers to satisfy both template and API server use cases

[TGoddessana]

So I'm using the flask server for two things.

1. a template-based server-side rendering server. It returns a web page.
2. Building an API server using flask-smorest.

Here I ran into a problem with handling errors. If I wanted the server to handle errors like 404 in the template use case, I could register an error handler function to return the appropriate template. like this:

def _register_error_handlers(app):
    for errcode in [401, 403, 404, 500]:
        @app.errorhandler(errcode)
        def render_error(error):
            error_code = getattr(error, "code", 500)
            return render_template(f"error_{error_code}.html"), error_code

But the problem is that this overrides the error handler defined by flask-smorest. If I throw a 404 error in the API use case (flask_smorest.abort()), it will be caught by the error handler I defined "for templates" and will result in an HTML error, as opposed to the intended JSON response.

It wasn't easy to find a way to handle both of these methods. Here's what I think is the ideal error handling provided by flask-smoret.

from flask import abort as flask_abort # errors in the default flask
from flask_smorest import abort as api_abort # api errors

# pseudocode...
@blp.response(UserSchema)
def users_api():
    # omit
    if not user:
        api_abort(404) # this returns a 404 JSON response (caught by the error handler currently present in flask-smorest)

@blp.response
def users_view():
    # similarly omit
    if not user:
        flask_abort() # this raises an HttpException from werkzeug.

I was thinking that flask-smorest could arbitrarily subclass HTTPException and register it in ErrorHandlerMixin, but I'd like to know what you think about this.

[lafrech]

Related to #51 and #174.

As I wrote in #51, the use case of API and website seems like a corner case to me. I don't mind supporting it if it doesn't make things more complicated for the typical use case.

The way it works currently, any HttpException is treated by flask-smorest handler. Per your suggestion, the user would have to use flask_smorest.import abort explicitly. Doesn't seem like a huge constraint, I think I already do that in my code. Could be an issue for third-party code such as e.g. authentication libs. The user would have to wrap the code to catch the error and raise it with the proper abort.

I don't know. Would you like to elaborate more about "flask-smorest could arbitrarily subclass HTTPException and register it in ErrorHandlerMixin"?

[TGoddessana]

How are you? Thank you very much for maintaining the project.

I don't know. Would you like to elaborate more about "flask-smorest could arbitrarily subclass HTTPException and register it in ErrorHandlerMixin"?

Yes, I tried to create a minimal example to reproduce this. I think this satisfies my use case.

from flask import Flask
from flask import abort
from flask_smorest import Api
from flask_smorest import abort as api_abort

app = Flask(__name__)

app.config['API_TITLE'] = 'My API'
app.config['API_VERSION'] = 'v1'
app.config['OPENAPI_VERSION'] = '3.0.2'

api = Api(app)

@app.route("/api")
def helloapi():
    api_abort(404)

@app.route("/website")
def hellowebsite():
    abort(404)

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

This is the example application code. As mentioned, you can see that I'm importing abort from flask_smorest under the name api_abort.

# flask_smorest.exceptions.py

class ApiException(wexc.HTTPException):
    """
    Base class for all API exceptions,
    Flask-Smoest will catch this exception and return the response in json format
    """

class ApiNotFound(ApiException, wexc.NotFound):
    """404 Not Found"""

class ApiMethodNotAllowed(ApiException, wexc.MethodNotAllowed):
    """405 Method Not Allowed"""

class ApiConflict(ApiException, wexc.Conflict):
    """409 Conflict"""

def find_exception_by_code(code: int) -> ApiException:
    """Find an apiexception, by code"""

    for exception in ApiException.__subclasses__():
        if exception.code == code:
            return exception

And like above, we define a class called ApiException in flask-smorest.exception and subclass it and several exception classes provided by WerkZeug to throw Api{exception} . I also created a helper function called find_exception_by_code to make it easier to find exceptions that subclass ApiException with a status code.

def abort(
    http_status_code: int, exc: Exception | None = None, **kwargs: typing.Any
) -> typing.NoReturn:
    """Raise a HTTPException for the given http_status_code. Attach any keyword
    arguments to the exception for later processing.

    From Flask-Restful. See NOTICE file for license information.
    """
    try:
        raise find_exception_by_code(http_status_code)(**kwargs)
    except ApiException as err:
        err.data = kwargs
        err.exc = exc
        raise err

    # try:
    #     flask.abort(http_status_code)
    # except HTTPException as err:
    #     err.data = kwargs  # type: ignore
    #     err.exc = exc  # type: ignore
    #     raise err

Now modify abort() in webargs.flaskparser like this. This is for simplicity's sake and may break any use cases or backwards compatibility, but I think it's a good example to show what I'm trying to accomplish. Find the classes that subclass ApiException, and throw exceptions on them. Even if it's not a generic code like 404, 403, 503, but something you define yourself, you can still throw an exception.

class ErrorHandlerMixin:
    # ...

    def _register_error_handlers(self):
        """Register error handlers in Flask app

        This method registers a default error handler for ``HTTPException``.
        """

        # set scope
        self._app.register_error_handler(ApiException, self.handle_http_exception)

Now, in the ErrorHandlerMixin, we explicitly specify the scope for handling exceptions as above. This way, flask-smoerst can only handle ApiExceptions...

(in /api route)

(in /website route)

I think there is still a lot to consider (how do we maintain backwards compatibility? If flask-smorest.abort() is given an exception other than ApiException as a parameter, how should it be handled? etc...), but I think this can be a big help in running API servers and template-based servers simultaneously by clearly separating the responsibility for handling API-related and non-API-related errors.

[lafrech]

Thanks for the details. This makes sense.

There might be a way to create all the exceptions dynamically from the HttpException subclasses. And we could make find_exception_by_code a classmethod.

I haven't been giving too much thought to backward compatibility, but it might even be a no breaker. When given a non Api exception, re-raise.

This looks like it is worth going further.

[TGoddessana]

thanks :) and i'm happy to start a PR.

There might be a way to create all the exceptions dynamically from the HttpException subclasses. And we could make find_exception_by_code a classmethod.

wolud you explain about this more? what i thought was like this... (Above, we have defined a new class called ApiException, but looking at the original source code, it seems that FlaskSmorestError inherits from werkzeug's HttpException to create an API error. I think it would be more consistent to inherit from FlaskSmorestError.)

class FlaskSmorestError(Exception):
    """Generic flask-smorest exception"""

class MissingAPIParameterError(FlaskSmorestError):
    """Missing API parameter"""

class BadRequest(wexc.BadRequest, FlaskSmorestError):
    ...

class Unauthorized(wexc.Unauthorized, FlaskSmorestError):
    ...

class Forbidden(wexc.Forbidden, FlaskSmorestError):
    ...

class NotFound(wexc.NotFound, FlaskSmorestError):
    ...

class MethodNotAllowed(wexc.MethodNotAllowed, FlaskSmorestError):
    ...

class NotAcceptable(wexc.NotAcceptable, FlaskSmorestError):
    ...

but this have too much duplicated code, i'm wondering is there any good solution for this..

[lafrech]

I was thinking of something along the lines of

exceptions.py

import sys
module = sys.modules[__name__]
for exc in ...:  # Iterate over Werkzeug exceptions
    class Exc(exc, FlaskSmorestError):
        ...
    setattr(module, exc.__name__, SmorestExc)

[TGoddessana]

Sorry for the delay.. I just submitted a PR #644, but the test I arbitrarily overridden is not passing. I thought that when a user arbitrarily throws an ApiException(), a JSON 500 error should be thrown, but I'm having trouble finding an elegant way to implement it (the current implementation returns a 500 Internal Error html). I would appreciate it if you could take a look at it when you have time :)