From 5ae9f6d7f09dba5d99bf439fc0e46e884df8eb4b Mon Sep 17 00:00:00 2001 From: "Laurent Mignon (ACSONE)" Date: Tue, 19 Mar 2024 14:42:35 +0100 Subject: [PATCH] [FIX] fastapi: Transactions handling refactoring This change is a complete rewrite of the way the transactions are managed when integrating a fastapi application into Odoo. In the previous implementation, specifics error handlers were put in place to catch exception occurring in the handling of requests made to a fastapi application and to rollback the transaction in case of error. This was done by registering specifics error handlers methods to the fastapi application using the 'add_exception_handler' method of the fastapi application. In this implementation, the transaction was rolled back in the error handler method. This approach was not working as expected for several reasons: - The handling of the error at the fastapi level prevented the retry mechanism to be triggered in case of a DB concurrency error. This is because the error was catch at the fastapi level and never bubbled up to the early stage of the processing of the request where the retry mechanism is implemented. - The cleanup of the environment and the registry was not properly done in case of error. In the **'odoo.service.model.retrying'** method, you can see that the cleanup process is different in case of error raised by the database and in case of error raised by the application. This change fix these issues by ensuring that errors are no more catch at the fastapi level and bubble up the fastapi processing stack through the event loop required to transform WSGI to ASGI. As result the transactional nature of the requests to the fastapi applications is now properly managed by the Odoo framework. --- fastapi/README.rst | 84 ++++------------ fastapi/__init__.py | 1 + fastapi/error_handlers.py | 123 ++++-------------------- fastapi/fastapi_dispatcher.py | 45 ++++++++- fastapi/models/fastapi_endpoint.py | 62 ++++-------- fastapi/readme/USAGE.md | 81 ++++------------ fastapi/readme/newsfragments/422.bugfix | 40 +++++--- fastapi/static/description/index.html | 78 ++++----------- fastapi/tests/common.py | 3 +- fastapi/tests/test_fastapi.py | 109 +++++++++++++++++++++ fastapi/tests/test_fastapi_demo.py | 83 +--------------- 11 files changed, 276 insertions(+), 433 deletions(-) diff --git a/fastapi/README.rst b/fastapi/README.rst index cea7090f9..6d2a41ef7 100644 --- a/fastapi/README.rst +++ b/fastapi/README.rst @@ -1304,70 +1304,26 @@ The **'odoo.addons.fastapi.schemas.Paging'** and not designed to be extended to not introduce a dependency between the **'odoo-addon-fastapi'** module and the **'odoo-addon-extendable'** -Customization of the error handling -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -The error handling a very important topic in the design of the fastapi -integration with odoo. It must ensure that the error messages are -properly return to the client and that the transaction is properly roll -backed. The **'fastapi'** module provides a way to register custom error -handlers. The **'odoo.addons.fastapi.error_handlers'** module provides -the default error handlers that are registered by default when a new -instance of the **'FastAPI'** class is created. When an app is -initialized in 'fastapi.endpoint' model, the method -\_get_app_exception_handlers is called to get a dictionary of error -handlers. This method is designed to be overridden in a custom module to -provide custom error handlers. You can override the handler for a -specific exception class or you can add a new handler for a new -exception or even replace all the handlers by your own handlers. -Whatever you do, you must ensure that the transaction is properly roll -backed. - -Some could argue that the error handling can't be extended since the -error handlers are global method not defined in an odoo model. Since the -method providing the the error handlers definitions is defined on the -'fastapi.endpoint' model, it's not a problem at all, you just need to -think another way to do it that by inheritance. - -A solution could be to develop you own error handler to be able to -process the error and chain the call to the default error handler. - -.. code:: python - - class MyCustomErrorHandler(): - def __init__(self, next_handler): - self.next_handler = next_handler - - def __call__(self, request: Request, exc: Exception) -> JSONResponse: - # do something with the error - response = self.next_handler(request, exc) - # do something with the response - return response - -With this solution, you can now register your custom error handler by -overriding the method \_get_app_exception_handlers in your custom -module. - -.. code:: python - - class FastapiEndpoint(models.Model): - _inherit = "fastapi.endpoint" - - def _get_app_exception_handlers( - self, - ) -> Dict[ - Union[int, Type[Exception]], - Callable[[Request, Exception], Union[Response, Awaitable[Response]]], - ]: - handlers = super()._get_app_exception_handlers() - access_error_handler = handlers.get(odoo.exceptions.AccessError) - handlers[odoo.exceptions.AccessError] = MyCustomErrorHandler(access_error_handler) - return handlers - -In the previous example, we extend the error handler for the -'AccessError' exception for all the endpoints. You can do the same for a -specific app by checking the 'app' field of the 'fastapi.endpoint' -record before registering your custom error handler. +Error handling +~~~~~~~~~~~~~~ + +The error handling is a very important topic in the design of the +fastapi integration with odoo. By default, when instantiating the +fastapi app, the fastapi library declare a default exception handler +that will catch any exception raised by the route handlers and return a +proper error response. This is done to ensure that the serving of the +app is not interrupted by an unhandled exception. If this implementation +makes sense for a native fastapi app, it's not the case for the fastapi +integration with odoo. The transactional nature of the calls to odoo's +api is implemented at the root of the request processing by odoo. To +ensure that the transaction is properly managed, the integration with +odoo must ensure that the exceptions raised by the route handlers +properly bubble up to the handling of the request by odoo. This is done +by the monkey patching of the registered exception handler of the +fastapi app in the **'odoo.addons.fastapi.models.error_handlers'** +module. As a result, it's no longer possible to define a custom +exception handler in your fastapi app. If you add a custom exception +handler in your app, it will be ignored. FastAPI addons directory structure ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/fastapi/__init__.py b/fastapi/__init__.py index d54296502..f4d0f5d36 100644 --- a/fastapi/__init__.py +++ b/fastapi/__init__.py @@ -1,2 +1,3 @@ from . import models from . import fastapi_dispatcher +from . import error_handlers diff --git a/fastapi/error_handlers.py b/fastapi/error_handlers.py index f5f760777..ca054a581 100644 --- a/fastapi/error_handlers.py +++ b/fastapi/error_handlers.py @@ -1,119 +1,36 @@ # Copyright 2022 ACSONE SA/NV # License LGPL-3.0 or later (http://www.gnu.org/licenses/LGPL). -import logging -from psycopg2.errors import IntegrityError, OperationalError +from starlette.exceptions import WebSocketException from starlette.middleware.errors import ServerErrorMiddleware +from starlette.middleware.exceptions import ExceptionMiddleware from starlette.responses import JSONResponse -from starlette.status import ( - HTTP_400_BAD_REQUEST, - HTTP_403_FORBIDDEN, - HTTP_404_NOT_FOUND, - HTTP_500_INTERNAL_SERVER_ERROR, -) - -import odoo -from odoo.service.model import PG_CONCURRENCY_ERRORS_TO_RETRY, _as_validation_error +from starlette.websockets import WebSocket from fastapi import Request -from fastapi.exception_handlers import http_exception_handler -from fastapi.exceptions import HTTPException - -from .context import odoo_env_ctx - -_logger = logging.getLogger(__name__) - - -def _rollback(request: Request, reason: str) -> None: - cr = odoo_env_ctx.get().cr - if cr is not None: - _logger.debug("rollback on %s", reason) - cr.rollback() - - -async def _odoo_user_error_handler( - request: Request, exc: odoo.exceptions.UserError -) -> JSONResponse: - _rollback(request, "UserError") - return await http_exception_handler( - request, HTTPException(HTTP_400_BAD_REQUEST, exc.args[0]) - ) - - -async def _odoo_access_error_handler( - request: Request, _exc: odoo.exceptions.AccessError -) -> JSONResponse: - _rollback(request, "AccessError") - return await http_exception_handler( - request, HTTPException(HTTP_403_FORBIDDEN, "AccessError") - ) - -async def _odoo_missing_error_handler( - request: Request, _exc: odoo.exceptions.MissingError -) -> JSONResponse: - _rollback(request, "MissingError") - return await http_exception_handler( - request, HTTPException(HTTP_404_NOT_FOUND, "MissingError") - ) - - -async def _odoo_validation_error_handler( - request: Request, exc: odoo.exceptions.ValidationError -) -> JSONResponse: - _rollback(request, "ValidationError") - return await http_exception_handler( - request, HTTPException(HTTP_400_BAD_REQUEST, exc.args[0]) - ) - - -async def _odoo_http_exception_handler( - request: Request, exc: HTTPException -) -> JSONResponse: - _rollback(request, "HTTPException") - return await http_exception_handler(request, exc) - - -async def _odoo_exception_handler(request: Request, exc: Exception) -> JSONResponse: - # let the OperationalError bubble up to the retrying mechanism - # We can't define a specific handler for OperationalError because since we - # want to let it bubble up to the retrying mechanism, it will be handled by - # the default handler at the end of the chain. - if ( - isinstance(exc, OperationalError) - and exc.pgcode in PG_CONCURRENCY_ERRORS_TO_RETRY - ): - raise exc +# we need to monkey patch the ServerErrorMiddleware and ExceptionMiddleware classes +# to ensure that all the exceptions that are handled by these specific +# middlewares are let to bubble up to the retrying mechanism and the +# dispatcher error handler to ensure that appropriate action are taken +# regarding the transaction, environment, and registry. These middlewares +# are added by default by FastAPI when creating an application and it's not +# possible to remove them. So we need to monkey patch them. - _rollback(request, "Exception") - _logger.exception("Unhandled exception", exc_info=exc) - return await http_exception_handler( - request, HTTPException(HTTP_500_INTERNAL_SERVER_ERROR, str(exc)) - ) - -async def _pg_integrity_error_handler( - request: Request, exc: IntegrityError +def pass_through_exception_handler( + self, request: Request, exc: Exception ) -> JSONResponse: - """Handle IntegrityError from the database as a ValidationError""" - validation_error = _as_validation_error(odoo_env_ctx.get(), exc) - return await _odoo_validation_error_handler(request, validation_error) - - -# we need to monkey patch the ServerErrorMiddleware to ensure that the -# OperationalError is not handled by the default handler but is let to bubble up -# to the retrying mechanism -original_error_response_method = ServerErrorMiddleware.error_response + raise exc -def error_response(self, request: Request, exc: Exception) -> JSONResponse: - if ( - isinstance(exc, OperationalError) - and exc.pgcode in PG_CONCURRENCY_ERRORS_TO_RETRY - ): - raise exc - return original_error_response_method(self, request, exc) +def pass_through_websocket_exception_handler( + self, websocket: WebSocket, exc: WebSocketException +) -> None: + raise exc -ServerErrorMiddleware.error_response = error_response +ServerErrorMiddleware.error_response = pass_through_exception_handler +ExceptionMiddleware.http_exception = pass_through_exception_handler +ExceptionMiddleware.websocket_exception = pass_through_websocket_exception_handler diff --git a/fastapi/fastapi_dispatcher.py b/fastapi/fastapi_dispatcher.py index e4451cebf..dda4b20e4 100644 --- a/fastapi/fastapi_dispatcher.py +++ b/fastapi/fastapi_dispatcher.py @@ -4,10 +4,17 @@ from contextlib import contextmanager from io import BytesIO -from werkzeug.exceptions import InternalServerError +from starlette import status +from starlette.exceptions import HTTPException +from werkzeug.exceptions import HTTPException as WerkzeugHTTPException +from odoo.exceptions import AccessDenied, AccessError, MissingError, UserError from odoo.http import Dispatcher, request +from fastapi.encoders import jsonable_encoder +from fastapi.exceptions import RequestValidationError, WebSocketRequestValidationError +from fastapi.utils import is_body_allowed_for_status_code + from .context import odoo_env_ctx @@ -39,10 +46,38 @@ def dispatch(self, endpoint, args): ) def handle_error(self, exc): - # At this stage all the normal exceptions are handled by FastAPI - # and we should only have InternalServerError occurring after the - # FastAPI app has been called. - return InternalServerError() # pragma: no cover + headers = getattr(exc, "headers", None) + status_code = status.HTTP_500_INTERNAL_SERVER_ERROR + details = "Internal Server Error" + if isinstance(exc, WerkzeugHTTPException): + status_code = exc.code + details = exc.description + elif isinstance(exc, HTTPException): + status_code = exc.status_code + details = exc.detail + elif isinstance(exc, RequestValidationError): + status_code = status.HTTP_422_UNPROCESSABLE_ENTITY + details = jsonable_encoder(exc.errors()) + elif isinstance(exc, WebSocketRequestValidationError): + status_code = status.WS_1008_POLICY_VIOLATION + details = jsonable_encoder(exc.errors()) + elif isinstance(exc, AccessDenied | AccessError): + status_code = status.HTTP_403_FORBIDDEN + details = "AccessError" + elif isinstance(exc, MissingError): + status_code = status.HTTP_404_NOT_FOUND + details = "MissingError" + elif isinstance(exc, UserError): + status_code = status.HTTP_400_BAD_REQUEST + details = exc.args[0] + body = {} + if is_body_allowed_for_status_code(status_code): + # use the same format as in + # fastapi.exception_handlers.http_exception_handler + body = {"detail": details} + return self.request.make_json_response( + body, status=status_code, headers=headers + ) def _make_response(self, status_mapping, headers_tuple, content): self.status = status_mapping[:3] diff --git a/fastapi/models/fastapi_endpoint.py b/fastapi/models/fastapi_endpoint.py index b27ad4f84..de465a9a7 100644 --- a/fastapi/models/fastapi_endpoint.py +++ b/fastapi/models/fastapi_endpoint.py @@ -2,21 +2,20 @@ # License LGPL-3.0 or later (http://www.gnu.org/licenses/LGPL). import logging -from collections.abc import Awaitable, Callable +from collections.abc import Callable from functools import partial from itertools import chain from typing import Any -import psycopg2.errors from a2wsgi import ASGIMiddleware from starlette.middleware import Middleware +from starlette.routing import Mount -import odoo from odoo import _, api, exceptions, fields, models, tools -from fastapi import APIRouter, Depends, FastAPI, HTTPException, Request, Response +from fastapi import APIRouter, Depends, FastAPI -from .. import dependencies, error_handlers +from .. import dependencies _logger = logging.getLogger(__name__) @@ -202,8 +201,24 @@ def get_app(self, root_path): return None app = FastAPI() app.mount(record.root_path, record._get_app()) + self._clear_fastapi_exception_handlers(app) return ASGIMiddleware(app) + def _clear_fastapi_exception_handlers(self, app: FastAPI) -> None: + """ + Clear the exception handlers of the given fastapi app. + + This method is used to ensure that the exception handlers are handled + by odoo and not by fastapi. We therefore need to remove all the handlers + added by default when instantiating a FastAPI app. Since apps can be + mounted recursively, we need to apply this method to all the apps in the + mounted tree. + """ + app.exception_handlers = {} + for route in app.routes: + if isinstance(route, Mount): + self._clear_fastapi_exception_handlers(route.app) + @api.model @tools.ormcache("root_path") def get_uid(self, root_path): @@ -217,8 +232,6 @@ def _get_app(self) -> FastAPI: for router in self._get_fastapi_routers(): app.include_router(router=router) app.dependency_overrides.update(self._get_app_dependencies_overrides()) - for exception, handler in self._get_app_exception_handlers().items(): - app.add_exception_handler(exception, handler) return app def _get_app_dependencies_overrides(self) -> dict[Callable, Callable]: @@ -227,41 +240,6 @@ def _get_app_dependencies_overrides(self) -> dict[Callable, Callable]: dependencies.company_id: partial(lambda a: a, self.company_id.id), } - def _get_app_exception_handlers( - self, - ) -> dict[ - (int | type[Exception]), - Callable[[Request, Exception], (Response | Awaitable[Response])], - ]: - """Return a dict of exception handlers to register on the app - - The key is the exception class or status code to handle. - The value is the handler function. - - If you need to register your own handler, you can do it by overriding - this method and calling super(). Changes done in this way will be applied - to all the endpoints. If you need to register a handler only for a specific - endpoint, you can do it by overriding the _get_app_exception_handlers method - and conditionally returning your specific handlers only for the endpoint - you want according to the self.app value. - - Be careful to not forget to roll back the transaction when you implement - your own error handler. If you don't, the transaction will be committed - and the changes will be applied to the database. - """ - self.ensure_one() - return { - Exception: error_handlers._odoo_exception_handler, - psycopg2.errors.IntegrityError: error_handlers._pg_integrity_error_handler, - HTTPException: error_handlers._odoo_http_exception_handler, - odoo.exceptions.UserError: error_handlers._odoo_user_error_handler, - odoo.exceptions.AccessError: error_handlers._odoo_access_error_handler, - odoo.exceptions.MissingError: error_handlers._odoo_missing_error_handler, - odoo.exceptions.ValidationError: ( - error_handlers._odoo_validation_error_handler - ), - } - def _prepare_fastapi_app_params(self) -> dict[str, Any]: """Return the params to pass to the Fast API app constructor""" return { diff --git a/fastapi/readme/USAGE.md b/fastapi/readme/USAGE.md index 0071595dd..131d7f7b5 100644 --- a/fastapi/readme/USAGE.md +++ b/fastapi/readme/USAGE.md @@ -1206,69 +1206,23 @@ The **'odoo.addons.fastapi.schemas.Paging'** and not designed to be extended to not introduce a dependency between the **'odoo-addon-fastapi'** module and the **'odoo-addon-extendable'** -### Customization of the error handling - -The error handling a very important topic in the design of the fastapi -integration with odoo. It must ensure that the error messages are -properly return to the client and that the transaction is properly roll -backed. The **'fastapi'** module provides a way to register custom error -handlers. The **'odoo.addons.fastapi.error_handlers'** module provides -the default error handlers that are registered by default when a new -instance of the **'FastAPI'** class is created. When an app is -initialized in 'fastapi.endpoint' model, the method -\_get_app_exception_handlers is called to get a dictionary of error -handlers. This method is designed to be overridden in a custom module to -provide custom error handlers. You can override the handler for a -specific exception class or you can add a new handler for a new -exception or even replace all the handlers by your own handlers. -Whatever you do, you must ensure that the transaction is properly roll -backed. - -Some could argue that the error handling can't be extended since the -error handlers are global method not defined in an odoo model. Since the -method providing the the error handlers definitions is defined on the -'fastapi.endpoint' model, it's not a problem at all, you just need to -think another way to do it that by inheritance. - -A solution could be to develop you own error handler to be able to -process the error and chain the call to the default error handler. - -``` python -class MyCustomErrorHandler(): - def __init__(self, next_handler): - self.next_handler = next_handler - - def __call__(self, request: Request, exc: Exception) -> JSONResponse: - # do something with the error - response = self.next_handler(request, exc) - # do something with the response - return response -``` - -With this solution, you can now register your custom error handler by -overriding the method \_get_app_exception_handlers in your custom -module. - -``` python -class FastapiEndpoint(models.Model): - _inherit = "fastapi.endpoint" - - def _get_app_exception_handlers( - self, - ) -> Dict[ - Union[int, Type[Exception]], - Callable[[Request, Exception], Union[Response, Awaitable[Response]]], - ]: - handlers = super()._get_app_exception_handlers() - access_error_handler = handlers.get(odoo.exceptions.AccessError) - handlers[odoo.exceptions.AccessError] = MyCustomErrorHandler(access_error_handler) - return handlers -``` - -In the previous example, we extend the error handler for the -'AccessError' exception for all the endpoints. You can do the same for a -specific app by checking the 'app' field of the 'fastapi.endpoint' -record before registering your custom error handler. +### Error handling + +The error handling is a very important topic in the design of the fastapi integration +with odoo. By default, when instantiating the fastapi app, the fastapi library +declare a default exception handler that will catch any exception raised by the +route handlers and return a proper error response. This is done to ensure that +the serving of the app is not interrupted by an unhandled exception. If this +implementation makes sense for a native fastapi app, it's not the case for the +fastapi integration with odoo. The transactional nature of the calls to +odoo's api is implemented at the root of the request processing by odoo. To ensure +that the transaction is properly managed, the integration with odoo must ensure +that the exceptions raised by the route handlers properly bubble up to the +handling of the request by odoo. This is done by the monkey patching of the +registered exception handler of the fastapi app in the +**'odoo.addons.fastapi.models.error_handlers'** module. As a result, it's no +longer possible to define a custom exception handler in your fastapi app. If you +add a custom exception handler in your app, it will be ignored. ### FastAPI addons directory structure @@ -1392,3 +1346,4 @@ The **'odoo-addon-fastapi'** module is still in its early stage of development. It will evolve over time to integrate your feedback and to provide the missing features. It's now up to you to try it and to provide your feedback. + diff --git a/fastapi/readme/newsfragments/422.bugfix b/fastapi/readme/newsfragments/422.bugfix index d9098148b..1d71cd568 100644 --- a/fastapi/readme/newsfragments/422.bugfix +++ b/fastapi/readme/newsfragments/422.bugfix @@ -1,15 +1,25 @@ -This change fix an issue that prevented the system to retry a call to the -application in case of a DB concurrency error if this error is defined as -retryable. - -By default, any call to the application is retried in case of a DB concurrency -if retryable. The 'retry' mechanism is implemented at the early stage of the processing -of a request in case of specific postgreSQL errors. When a call is made to -a fastapi application, the specific handler for the request is placed further -down the stack. Before this change, any error that occurred in the specific -fastapi handler was catch by the Fastapi processing stack and never bubbled up -to the early stage of the processing where the 'retry' mechanism is implemented. - -This change fix this issue by putting in place a way to make some specific -exceptions bubble up the fastapi processing stack through the event loop required -to transform WSGI to ASGI. +This change is a complete rewrite of the way the transactions are managed when +integrating a fastapi application into Odoo. + +In the previous implementation, specifics error handlers were put in place to +catch exception occurring in the handling of requests made to a fastapi application +and to rollback the transaction in case of error. This was done by registering +specifics error handlers methods to the fastapi application using the 'add_exception_handler' +method of the fastapi application. In this implementation, the transaction was +rolled back in the error handler method. + +This approach was not working as expected for several reasons: + +- The handling of the error at the fastapi level prevented the retry mechanism + to be triggered in case of a DB concurrency error. This is because the error + was catch at the fastapi level and never bubbled up to the early stage of the + processing of the request where the retry mechanism is implemented. +- The cleanup of the environment and the registry was not properly done in case + of error. In the **'odoo.service.model.retrying'** method, you can see that + the cleanup process is different in case of error raised by the database + and in case of error raised by the application. + +This change fix these issues by ensuring that errors are no more catch at the +fastapi level and bubble up the fastapi processing stack through the event loop +required to transform WSGI to ASGI. As result the transactional nature of the +requests to the fastapi applications is now properly managed by the Odoo framework. diff --git a/fastapi/static/description/index.html b/fastapi/static/description/index.html index 2c0c0aed9..a39ccf7d3 100644 --- a/fastapi/static/description/index.html +++ b/fastapi/static/description/index.html @@ -428,7 +428,7 @@

Odoo FastAPI

  • Overall considerations when you develop an fastapi app
  • Miscellaneous
    • @@ -1599,63 +1599,25 @@

    Development of a search route ha not designed to be extended to not introduce a dependency between the ‘odoo-addon-fastapi’ module and the ‘odoo-addon-extendable’

    -
    -

    Customization of the error handling

    -

    The error handling a very important topic in the design of the fastapi -integration with odoo. It must ensure that the error messages are -properly return to the client and that the transaction is properly roll -backed. The ‘fastapi’ module provides a way to register custom error -handlers. The ‘odoo.addons.fastapi.error_handlers’ module provides -the default error handlers that are registered by default when a new -instance of the ‘FastAPI’ class is created. When an app is -initialized in ‘fastapi.endpoint’ model, the method -_get_app_exception_handlers is called to get a dictionary of error -handlers. This method is designed to be overridden in a custom module to -provide custom error handlers. You can override the handler for a -specific exception class or you can add a new handler for a new -exception or even replace all the handlers by your own handlers. -Whatever you do, you must ensure that the transaction is properly roll -backed.

    -

    Some could argue that the error handling can’t be extended since the -error handlers are global method not defined in an odoo model. Since the -method providing the the error handlers definitions is defined on the -‘fastapi.endpoint’ model, it’s not a problem at all, you just need to -think another way to do it that by inheritance.

    -

    A solution could be to develop you own error handler to be able to -process the error and chain the call to the default error handler.

    -
    -class MyCustomErrorHandler():
-    def __init__(self, next_handler):
-        self.next_handler = next_handler
-
-    def __call__(self, request: Request, exc: Exception) -> JSONResponse:
-        # do something with the error
-        response = self.next_handler(request, exc)
-        # do something with the response
-        return response
-
    -

    With this solution, you can now register your custom error handler by -overriding the method _get_app_exception_handlers in your custom -module.

    -
    -class FastapiEndpoint(models.Model):
-    _inherit = "fastapi.endpoint"
-
-    def _get_app_exception_handlers(
-        self,
-    ) -> Dict[
-        Union[int, Type[Exception]],
-        Callable[[Request, Exception], Union[Response, Awaitable[Response]]],
-    ]:
-        handlers = super()._get_app_exception_handlers()
-        access_error_handler = handlers.get(odoo.exceptions.AccessError)
-        handlers[odoo.exceptions.AccessError] = MyCustomErrorHandler(access_error_handler)
-        return handlers
-
    -

    In the previous example, we extend the error handler for the -‘AccessError’ exception for all the endpoints. You can do the same for a -specific app by checking the ‘app’ field of the ‘fastapi.endpoint’ -record before registering your custom error handler.

    +
    +

    Error handling

    +

    The error handling is a very important topic in the design of the +fastapi integration with odoo. By default, when instantiating the +fastapi app, the fastapi library declare a default exception handler +that will catch any exception raised by the route handlers and return a +proper error response. This is done to ensure that the serving of the +app is not interrupted by an unhandled exception. If this implementation +makes sense for a native fastapi app, it’s not the case for the fastapi +integration with odoo. The transactional nature of the calls to odoo’s +api is implemented at the root of the request processing by odoo. To +ensure that the transaction is properly managed, the integration with +odoo must ensure that the exceptions raised by the route handlers +properly bubble up to the handling of the request by odoo. This is done +by the monkey patching of the registered exception handler of the +fastapi app in the ‘odoo.addons.fastapi.models.error_handlers’ +module. As a result, it’s no longer possible to define a custom +exception handler in your fastapi app. If you add a custom exception +handler in your app, it will be ignored.

    FastAPI addons directory structure

    diff --git a/fastapi/tests/common.py b/fastapi/tests/common.py index a47def4e1..734a6d54d 100644 --- a/fastapi/tests/common.py +++ b/fastapi/tests/common.py @@ -111,7 +111,8 @@ def _create_test_client( dependencies[authenticated_partner_impl] = partial(lambda a: a, partner) if partner and optionally_authenticated_partner_impl in dependencies: raise ValueError( - "You cannot provide an override for the optionally_authenticated_partner_impl " + "You cannot provide an override for the " + "optionally_authenticated_partner_impl " "dependency when creating a test client with a partner." ) if partner or optionally_authenticated_partner_impl not in dependencies: diff --git a/fastapi/tests/test_fastapi.py b/fastapi/tests/test_fastapi.py index b2c7a7181..dcc7385b6 100644 --- a/fastapi/tests/test_fastapi.py +++ b/fastapi/tests/test_fastapi.py @@ -3,8 +3,15 @@ import os import unittest +from contextlib import contextmanager +from odoo import sql_db from odoo.tests.common import HttpCase +from odoo.tools import mute_logger + +from fastapi import status + +from ..schemas import DemoExceptionType @unittest.skipIf(os.getenv("SKIP_HTTP_CASE"), "EndpointHttpCase skipped") @@ -21,6 +28,13 @@ def setUpClass(cls): ) lang.active = True + @contextmanager + def _mocked_commit(self): + with unittest.mock.patch.object( + sql_db.TestCursor, "commit", return_value=None + ) as mocked_commit: + yield mocked_commit + def _assert_expected_lang(self, accept_language, expected_lang): route = "/fastapi_demo/demo/lang" response = self.url_open(route, headers={"Accept-language": accept_language}) @@ -48,3 +62,98 @@ def test_retrying(self): response = self.url_open(route, timeout=20) self.assertEqual(response.status_code, 200) self.assertEqual(int(response.content), nbr_retries) + + @mute_logger("odoo.http") + def assert_exception_processed( + self, + exception_type: DemoExceptionType, + error_message: str, + expected_message: str, + expected_status_code: int, + ) -> None: + with self._mocked_commit() as mocked_commit: + route = ( + "/fastapi_demo/demo/exception?" + f"exception_type={exception_type.value}&error_message={error_message}" + ) + response = self.url_open(route, timeout=200) + mocked_commit.assert_not_called() + self.assertDictEqual( + response.json(), + { + "detail": expected_message, + }, + ) + self.assertEqual(response.status_code, expected_status_code) + + def test_user_error(self) -> None: + self.assert_exception_processed( + exception_type=DemoExceptionType.user_error, + error_message="test", + expected_message="test", + expected_status_code=status.HTTP_400_BAD_REQUEST, + ) + + def test_validation_error(self) -> None: + self.assert_exception_processed( + exception_type=DemoExceptionType.validation_error, + error_message="test", + expected_message="test", + expected_status_code=status.HTTP_400_BAD_REQUEST, + ) + + def test_bare_exception(self) -> None: + self.assert_exception_processed( + exception_type=DemoExceptionType.bare_exception, + error_message="test", + expected_message="Internal Server Error", + expected_status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + ) + + def test_access_error(self) -> None: + self.assert_exception_processed( + exception_type=DemoExceptionType.access_error, + error_message="test", + expected_message="AccessError", + expected_status_code=status.HTTP_403_FORBIDDEN, + ) + + def test_missing_error(self) -> None: + self.assert_exception_processed( + exception_type=DemoExceptionType.missing_error, + error_message="test", + expected_message="MissingError", + expected_status_code=status.HTTP_404_NOT_FOUND, + ) + + def test_http_exception(self) -> None: + self.assert_exception_processed( + exception_type=DemoExceptionType.http_exception, + error_message="test", + expected_message="test", + expected_status_code=status.HTTP_409_CONFLICT, + ) + + @mute_logger("odoo.http") + def test_request_validation_error(self) -> None: + with self._mocked_commit() as mocked_commit: + route = "/fastapi_demo/demo/exception?exception_type=BAD&error_message=" + response = self.url_open(route, timeout=200) + mocked_commit.assert_not_called() + self.assertEqual(response.status_code, status.HTTP_422_UNPROCESSABLE_ENTITY) + + def test_no_commit_on_exception(self) -> None: + # this test check that the way we mock the cursor is working as expected + # and that the transaction is rolled back in case of exception. + with self._mocked_commit() as mocked_commit: + url = "/fastapi_demo/demo" + response = self.url_open(url, timeout=600) + self.assertEqual(response.status_code, 200) + mocked_commit.assert_called_once() + + self.assert_exception_processed( + exception_type=DemoExceptionType.http_exception, + error_message="test", + expected_message="test", + expected_status_code=status.HTTP_409_CONFLICT, + ) diff --git a/fastapi/tests/test_fastapi_demo.py b/fastapi/tests/test_fastapi_demo.py index 5cd9fef55..1692e69f3 100644 --- a/fastapi/tests/test_fastapi_demo.py +++ b/fastapi/tests/test_fastapi_demo.py @@ -2,17 +2,14 @@ # License LGPL-3.0 or later (http://www.gnu.org/licenses/LGPL). from functools import partial -from unittest import mock from requests import Response -from odoo.tools import mute_logger - from fastapi import status from ..dependencies import fastapi_endpoint from ..routers import demo_router -from ..schemas import DemoEndpointAppInfo, DemoExceptionType +from ..schemas import DemoEndpointAppInfo from .common import FastAPITransactionCase @@ -34,36 +31,6 @@ def setUpClass(cls) -> None: {"name": "FastAPI Demo"} ) - @mute_logger("odoo.addons.fastapi.error_handlers") - def assert_exception_processed( - self, - exception_type: DemoExceptionType, - error_message: str, - expected_message: str, - expected_status_code: int, - ) -> None: - demo_app = self.env.ref("fastapi.fastapi_endpoint_demo") - with self._create_test_client( - demo_app._get_app(), raise_server_exceptions=False - ) as test_client, mock.patch.object( - self.env.cr.__class__, "rollback" - ) as mock_rollback: - response: Response = test_client.get( - "/demo/exception", - params={ - "exception_type": exception_type.value, - "error_message": error_message, - }, - ) - mock_rollback.assert_called_once() - self.assertEqual(response.status_code, expected_status_code) - self.assertDictEqual( - response.json(), - { - "detail": expected_message, - }, - ) - def test_hello_world(self) -> None: with self._create_test_client() as test_client: response: Response = test_client.get("/demo/") @@ -94,51 +61,3 @@ def test_endpoint_info(self) -> None: response.json(), DemoEndpointAppInfo.model_validate(demo_app).model_dump(by_alias=True), ) - - def test_user_error(self) -> None: - self.assert_exception_processed( - exception_type=DemoExceptionType.user_error, - error_message="test", - expected_message="test", - expected_status_code=status.HTTP_400_BAD_REQUEST, - ) - - def test_validation_error(self) -> None: - self.assert_exception_processed( - exception_type=DemoExceptionType.validation_error, - error_message="test", - expected_message="test", - expected_status_code=status.HTTP_400_BAD_REQUEST, - ) - - def test_bare_exception(self) -> None: - self.assert_exception_processed( - exception_type=DemoExceptionType.bare_exception, - error_message="test", - expected_message="test", - expected_status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - ) - - def test_access_error(self) -> None: - self.assert_exception_processed( - exception_type=DemoExceptionType.access_error, - error_message="test", - expected_message="AccessError", - expected_status_code=status.HTTP_403_FORBIDDEN, - ) - - def test_missing_error(self) -> None: - self.assert_exception_processed( - exception_type=DemoExceptionType.missing_error, - error_message="test", - expected_message="MissingError", - expected_status_code=status.HTTP_404_NOT_FOUND, - ) - - def test_http_exception(self) -> None: - self.assert_exception_processed( - exception_type=DemoExceptionType.http_exception, - error_message="test", - expected_message="test", - expected_status_code=status.HTTP_409_CONFLICT, - )