[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

@@ -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
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

fastapi/__init__.py

@@ -1,2 +1,3 @@
 from . import models
 from . import fastapi_dispatcher
+from . import error_handlers

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

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]

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 {