diff --git a/fastapi/README.rst b/fastapi/README.rst index 413887e5..5b66b54a 100644 --- a/fastapi/README.rst +++ b/fastapi/README.rst @@ -1268,66 +1268,25 @@ you be consistent when writing a route handler for a search route. 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. +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. -.. code-block:: 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-block:: 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. FastAPI addons directory structure ================================== diff --git a/fastapi/__init__.py b/fastapi/__init__.py index d5429650..f4d0f5d3 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 a7b4d9b2..c95d3692 100644 --- a/fastapi/error_handlers.py +++ b/fastapi/error_handlers.py @@ -1,80 +1,36 @@ # Copyright 2022 ACSONE SA/NV # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). -import logging +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 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") - ) +# 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. -async def _odoo_missing_error_handler( - request: Request, _exc: odoo.exceptions.MissingError +def pass_through_exception_handler( + self, request: Request, exc: Exception ) -> JSONResponse: - _rollback(request, "MissingError") - return await http_exception_handler( - request, HTTPException(HTTP_404_NOT_FOUND, "MissingError") - ) + raise exc -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) +def pass_through_websocket_exception_handler( + self, websocket: WebSocket, exc: WebSocketException +) -> None: + raise exc -async def _odoo_exception_handler(request: Request, exc: Exception) -> JSONResponse: - _rollback(request, "Exception") - _logger.exception("Unhandled exception", exc_info=exc) - return await http_exception_handler( - request, HTTPException(HTTP_500_INTERNAL_SERVER_ERROR, str(exc)) - ) +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 41587a73..e5957fe1 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 @@ -32,19 +39,65 @@ def dispatch(self, endpoint, args): with self._manage_odoo_env(uid): for r in app(environ, self._make_response): data.write(r) + if self.inner_exception: + raise self.inner_exception return self.request.make_response( data.getvalue(), headers=self.headers, status=self.status ) 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] self.headers = dict(headers_tuple) + self.inner_exception = None + # in case of exception, the method asgi_done_callback of the + # ASGIResponder will trigger an "a2wsgi.error" event with the exception + # instance stored in a tuple with the type of the exception and the traceback. + # The event loop will then be notified and then call the `error_response` + # method of the ASGIResponder. This method will then call the + # `_make_response` method provided as callback to the app with the tuple + # of the exception as content. In this case, we store the exception + # instance in the `inner_exception` attribute to be able to raise it + # in the `dispatch` method. + if ( + isinstance(content, tuple) + and len(content) == 3 + and isinstance(content[1], Exception) + ): + self.inner_exception = content[1] def _get_environ(self): try: diff --git a/fastapi/models/fastapi_endpoint.py b/fastapi/models/fastapi_endpoint.py index 22b85243..a7608e27 100644 --- a/fastapi/models/fastapi_endpoint.py +++ b/fastapi/models/fastapi_endpoint.py @@ -4,17 +4,17 @@ import logging from functools import partial from itertools import chain -from typing import Any, Awaitable, Callable, Dict, List, Tuple, Type, Union +from typing import Any, Callable, Dict, List, Tuple 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__) @@ -201,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): @@ -216,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]: @@ -226,38 +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[ - Union[int, Type[Exception]], - Callable[[Request, Exception], Union[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, - 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.rst b/fastapi/readme/USAGE.rst index ef6f8240..f03942a3 100644 --- a/fastapi/readme/USAGE.rst +++ b/fastapi/readme/USAGE.rst @@ -1191,66 +1191,25 @@ you be consistent when writing a route handler for a search route. 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. +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. -.. code-block:: 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-block:: 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. FastAPI addons directory structure ================================== diff --git a/fastapi/readme/newsfragments/422.bugfix b/fastapi/readme/newsfragments/422.bugfix new file mode 100644 index 00000000..1d71cd56 --- /dev/null +++ b/fastapi/readme/newsfragments/422.bugfix @@ -0,0 +1,25 @@ +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/routers/demo_router.py b/fastapi/routers/demo_router.py index 96614e3e..e316d8df 100644 --- a/fastapi/routers/demo_router.py +++ b/fastapi/routers/demo_router.py @@ -6,12 +6,16 @@ """ from typing import Annotated +from psycopg2 import errorcodes +from psycopg2.errors import OperationalError + from odoo.api import Environment from odoo.exceptions import AccessError, MissingError, UserError, ValidationError +from odoo.service.model import MAX_TRIES_ON_CONCURRENCY_FAILURE from odoo.addons.base.models.res_partner import Partner -from fastapi import APIRouter, Depends, HTTPException, status +from fastapi import APIRouter, Depends, HTTPException, Query, status from ..dependencies import authenticated_partner, fastapi_endpoint, odoo_env from ..models import FastapiEndpoint @@ -84,3 +88,33 @@ async def endpoint_app_info( # It also show you how you can specify a dependency to force the security # even if the method doesn't require the authenticated partner as parameter return DemoEndpointAppInfo.model_validate(endpoint) + + +_CPT = 0 + + +@router.get("/demo/retrying") +async def retrying( + nbr_retries: Annotated[int, Query(gt=1, lt=MAX_TRIES_ON_CONCURRENCY_FAILURE)] +) -> int: + """This method is used in the test suite to check that the retrying + functionality in case of concurrency error on the database is working + correctly for retryable exceptions. + + The output will be the number of retries that have been done. + + This method is mainly used to test the retrying functionality + """ + global _CPT + if _CPT < nbr_retries: + _CPT += 1 + raise FakeConcurrentUpdateError("fake error") + tryno = _CPT + _CPT = 0 + return tryno + + +class FakeConcurrentUpdateError(OperationalError): + @property + def pgcode(self): + return errorcodes.SERIALIZATION_FAILURE diff --git a/fastapi/static/description/index.html b/fastapi/static/description/index.html index 966d71cf..02899be9 100644 --- a/fastapi/static/description/index.html +++ b/fastapi/static/description/index.html @@ -8,10 +8,11 @@ /* :Author: David Goodger (goodger@python.org) -:Id: $Id: html4css1.css 8954 2022-01-20 10:10:25Z milde $ +:Id: $Id: html4css1.css 9511 2024-01-13 09:50:07Z milde $ :Copyright: This stylesheet has been placed in the public domain. Default cascading style sheet for the HTML output of Docutils. +Despite the name, some widely supported CSS2 features are used. See https://docutils.sourceforge.io/docs/howto/html-stylesheets.html for how to customize this style sheet. @@ -274,7 +275,7 @@ margin-left: 2em ; margin-right: 2em } -pre.code .ln { color: grey; } /* line numbers */ +pre.code .ln { color: gray; } /* line numbers */ pre.code, code { background-color: #eeeeee } pre.code .comment, code .comment { color: #5C6576 } pre.code .keyword, code .keyword { color: #3B0D06; font-weight: bold } @@ -300,7 +301,7 @@ span.pre { white-space: pre } -span.problematic { +span.problematic, pre.problematic { color: red } span.section-subtitle { @@ -424,7 +425,7 @@

Odoo FastAPI

  • Development of a search route handler
    • -
  • Customization of the error handling
    • +
  • Error handling
  • FastAPI addons directory structure @@ -1524,58 +1525,23 @@

    Development of a search route ha 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

    @@ -1798,7 +1764,9 @@

    Contributors

    Maintainers

    This module is maintained by the OCA.

    -Odoo Community Association + +Odoo Community Association +

    OCA, or the Odoo Community Association, is a nonprofit organization whose mission is to support the collaborative development of Odoo features and promote its widespread use.

    diff --git a/fastapi/tests/test_fastapi.py b/fastapi/tests/test_fastapi.py index f3b22aad..dcc7385b 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}) @@ -38,3 +52,108 @@ def test_lang(self): self._assert_expected_lang("en,fr;q=0.7,en-GB;q=0.3", b'"en_US"') self._assert_expected_lang("fr-FR,en;q=0.7,en-GB;q=0.3", b'"fr_BE"') self._assert_expected_lang("fr-FR;q=0.1,en;q=1.0,en-GB;q=0.8", b'"en_US"') + + def test_retrying(self): + """Test that the retrying mechanism is working as expected with the + FastAPI endpoints. + """ + nbr_retries = 3 + route = f"/fastapi_demo/demo/retrying?nbr_retries={nbr_retries}" + 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 5cd9fef5..1692e69f 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, - )