-
Notifications
You must be signed in to change notification settings - Fork 73
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
Improving error handlers to satisfy both template and API server use cases #640
Comments
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 I don't know. Would you like to elaborate more about "flask-smorest could arbitrarily subclass HTTPException and register it in ErrorHandlerMixin"? |
How are you?
Yes, I tried to create a minimal example to reproduce this. 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} . 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... 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. |
Thanks for the details. This makes sense. There might be a way to create all the exceptions dynamically from the 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. |
thanks :) and i'm happy to start a PR.
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.. |
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) |
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). |
So I'm using the flask server for two things.
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:
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.
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.
The text was updated successfully, but these errors were encountered: