(BSR)[API] feat: add catch_cinema_provider_request_timeout decorator

api/src/pcapi/core/external_bookings/decorators.py

@@ -0,0 +1,91 @@
+import functools
+import inspect
+import logging
+from typing import Any
+from typing import Callable
+from typing import TypeVar
+
+from pcapi.utils.requests import exceptions as requests_exception
+
+from . import exceptions
+from . import models
+
+
+logger = logging.getLogger(__name__)
+
+
+class ExternalBookingDecoratorException(Exception):
+    pass
+
+
+def _extract_logging_information(client_func: Callable, *args: list[Any], **kwargs: Any) -> dict:
+    """
+    Return a dictionary to be passed to the `extra` parameter of the logging function.
+
+    The returned dictionary contains the following keys:
+        - "cinema_id" (int): The ID of the cinema from the pivot table.
+        - "client" (str): The client used for the request (e.g., `EMSClientAPI`).
+        - "method" (str): The instance method used for the request (e.g., `book_ticket`).
+        - "method_params" (dict): The parameters passed to the method. Example:
+            {
+                "show_id": "12345",             # ID of the show
+                "booking": "<Booking #1>",      # Stringified Booking instance
+                "beneficiary": "<User #3>",     # Stringified User instance
+            }
+    """
+    self = args[0]
+    assert isinstance(self, models.ExternalBookingsClientAPI)  # to make mypy happy
+    # as client_func is an instance method of `ExternalBookingsClientAPI`, self should have a `cinema_id`
+    cinema_id = self.cinema_id
+
+    func_parameters = [param.name for param in inspect.signature(client_func).parameters.values()]
+    method_params = {}
+
+    arg_index = 1  # as the first argument is `self`, start from index 1
+    for arg_value in args[1:]:
+        arg_name = func_parameters[arg_index]
+        method_params[arg_name] = str(arg_value)
+        arg_index += 1
+
+    for k, v in kwargs.items():
+        method_params[k] = str(v)
+
+    return {
+        "client": self.__class__.__name__,  # class name
+        "method": client_func.__name__,  # method name
+        "cinema_id": cinema_id,
+        "method_params": method_params,
+    }
+
+
+def _check_wrapped_function_is_an_instance_method_of_external_booking_client_api(args: list) -> None:
+    if len(args) == 0 or not isinstance(args[0], models.ExternalBookingsClientAPI):
+        raise ExternalBookingDecoratorException(
+            "`catch_request_timeout` can only be applied to an instance method of a class inheriting from `ExternalBookingsClientAPI`"
+        )
+
+
+# typing to ensure that mypy understands the wrapped function signature has not changed
+F = TypeVar("F", bound=Callable[..., Any])
+
+
+def catch_cinema_provider_request_timeout(client_func: F) -> F:
+    """
+    Decorator that catches HTTP Timeout errors, logs them and then raises `ExternalBookingTimeoutException`.
+    This decorator can only be applied to instance methods of classes inheriting from `ExternalBookingsClientAPI`
+    """
+
+    @functools.wraps(client_func)
+    def wrapped_func(*args, **kwargs):  # type: ignore[no-untyped-def]
+        _check_wrapped_function_is_an_instance_method_of_external_booking_client_api(args)
+        try:
+            return client_func(*args, **kwargs)
+        except (requests_exception.Timeout, requests_exception.ReadTimeout) as e:
+            extra = _extract_logging_information(client_func, *args, **kwargs)
+            logger.error(
+                "Cinema Provider API Request Timeout",
+                extra=dict(extra, request={"method": e.request.method, "url": e.request.url}),
+            )
+            raise exceptions.ExternalBookingTimeoutException()
+
+    return wrapped_func  # type: ignore[return-value]

api/src/pcapi/core/external_bookings/exceptions.py

@@ -2,6 +2,10 @@ class ExternalBookingException(Exception):
     pass
 
 
+class ExternalBookingTimeoutException(Exception):
+    pass
+
+
 class ExternalBookingSoldOutError(Exception):
     pass
 

api/tests/core/external_bookings/test_decorator.py

@@ -0,0 +1,139 @@
+import logging
+from unittest.mock import Mock
+
+import pytest
+from requests.exceptions import ReadTimeout
+from requests.exceptions import Timeout
+
+from pcapi.core.bookings import models as bookings_models
+from pcapi.core.external_bookings import models as external_bookings_models
+from pcapi.core.external_bookings.decorators import ExternalBookingDecoratorException
+from pcapi.core.external_bookings.decorators import catch_cinema_provider_request_timeout
+from pcapi.core.external_bookings.exceptions import ExternalBookingTimeoutException
+from pcapi.core.users import models as users_models
+
+
+class FakeExternalBookingClientAPI(external_bookings_models.ExternalBookingsClientAPI):
+    def __init__(self, cinema_id: str, connector) -> None:
+        self.cinema_id = cinema_id
+        self.connector = connector
+
+    @catch_cinema_provider_request_timeout
+    def get_film_showtimes_stocks(self, film_id):
+        return self.connector.make_request(film_id)
+
+    @catch_cinema_provider_request_timeout
+    def book_ticket(self, show_id, booking, beneficiary):
+        return self.connector.make_request(show_id, booking, beneficiary)
+
+
+class FakeClass:
+    @catch_cinema_provider_request_timeout
+    def function_that_cannot_be_decorated(self, a):
+        pass
+
+
+@catch_cinema_provider_request_timeout
+def another_func_that_cannot_be_decorated():
+    pass
+
+
+class CatchCinemaProviderRequestTimeoutTest:
+    def test_should_raise_error_because_wrapped_function_is_not_an_instance_method(self):
+        fake_class = FakeClass()
+
+        with pytest.raises(ExternalBookingDecoratorException):
+            fake_class.function_that_cannot_be_decorated("coucou")
+
+        with pytest.raises(ExternalBookingDecoratorException):
+            another_func_that_cannot_be_decorated()
+
+    def test_should_raise_understandable_error_if_the_decorated_func_is_called_with_incorrect_params(self):
+        client = FakeExternalBookingClientAPI(cinema_id=1, connector=Mock())
+
+        with pytest.raises(TypeError) as exception:
+            client.get_film_showtimes_stocks(12, show_id=1)
+            assert (
+                str(exception.value)
+                == "TypeError: get_film_showtimes_stocks() got an unexpected keyword argument 'show_id'"
+            )
+
+        with pytest.raises(TypeError) as exception:
+            client.get_film_showtimes_stocks(12, 13, 24)
+            assert (
+                str(exception.value)
+                == "TypeError: get_film_showtimes_stocks() takes 1 positional arguments but 3 were given"
+            )
+
+    @pytest.mark.parametrize(
+        "args_list,kwargs_dict,exception_class,request_mock,expected_extra",
+        [
+            (
+                [12345],
+                {"booking": bookings_models.Booking(id=1), "beneficiary": users_models.User(id=3)},
+                ReadTimeout,
+                Mock(url="https://unproviderado.re/mais/dont/lurl/est/un/peu/flaky", method="POST"),
+                {
+                    "cinema_id": 789,
+                    "method": "book_ticket",
+                    "client": "FakeExternalBookingClientAPI",
+                    "method_params": {
+                        "show_id": "12345",
+                        "booking": "<Booking #1>",
+                        "beneficiary": "<User #3>",
+                    },
+                    "request": {"url": "https://unproviderado.re/mais/dont/lurl/est/un/peu/flaky", "method": "POST"},
+                },
+            ),
+            (
+                [678, bookings_models.Booking(id=1), users_models.User(id=3)],
+                {},
+                Timeout,
+                Mock(url="https://matthieu.reve/de/les/débrancher", method="GET"),
+                {
+                    "cinema_id": 789,
+                    "method": "book_ticket",
+                    "client": "FakeExternalBookingClientAPI",
+                    "method_params": {
+                        "show_id": "678",
+                        "booking": "<Booking #1>",
+                        "beneficiary": "<User #3>",
+                    },
+                    "request": {"url": "https://matthieu.reve/de/les/débrancher", "method": "GET"},
+                },
+            ),
+            (
+                [],
+                {
+                    "show_id": 562,
+                    "booking": bookings_models.Booking(id=4567),
+                    "beneficiary": users_models.User(id=12345767),
+                },
+                ReadTimeout,
+                Mock(url="https://moi.aussi/faut/bien/admettre", method="PUT"),
+                {
+                    "cinema_id": 789,
+                    "method": "book_ticket",
+                    "client": "FakeExternalBookingClientAPI",
+                    "cinema_id": 789,
+                    "method_params": {
+                        "show_id": "562",
+                        "booking": "<Booking #4567>",
+                        "beneficiary": "<User #12345767>",
+                    },
+                    "request": {"url": "https://moi.aussi/faut/bien/admettre", "method": "PUT"},
+                },
+            ),
+        ],
+    )
+    def test_should_log_error(self, args_list, kwargs_dict, exception_class, request_mock, expected_extra, caplog):
+        connector = Mock()
+        connector.make_request.side_effect = exception_class(request=request_mock)
+        client = FakeExternalBookingClientAPI(cinema_id=789, connector=connector)
+
+        with caplog.at_level(logging.ERROR):
+            with pytest.raises(ExternalBookingTimeoutException):
+                client.book_ticket(*args_list, **kwargs_dict)
+
+        assert caplog.records[0].extra == expected_extra
+        assert caplog.messages[0] == "Cinema Provider API Request Timeout"