🎨 Fixes Deprecation Warning on redis (#5996)

Author: pcrespov

packages/pytest-simcore/src/pytest_simcore/redis_service.py

@@ -69,7 +69,7 @@ async def redis_client(
     yield client
 
     await client.flushall()
-    await client.close(close_connection_pool=True)
+    await client.aclose(close_connection_pool=True)
 
 
 @pytest.fixture()
@@ -86,7 +86,7 @@ async def redis_locks_client(
     yield client
 
     await client.flushall()
-    await client.close(close_connection_pool=True)
+    await client.aclose(close_connection_pool=True)
 
 
 @tenacity.retry(
@@ -103,4 +103,4 @@ async def wait_till_redis_responsive(redis_url: URL | str) -> None:
             msg = f"{redis_url=} not available"
             raise ConnectionError(msg)
     finally:
-        await client.close(close_connection_pool=True)
+        await client.aclose(close_connection_pool=True)

packages/service-library/src/servicelib/aiohttp/rest_middlewares.py

@@ -18,7 +18,7 @@
 from .rest_models import ErrorItemType, ErrorType, LogMessageType
 from .rest_responses import (
     create_data_response,
-    create_error_response,
+    create_http_error,
     is_enveloped_from_map,
     is_enveloped_from_text,
     wrap_as_envelope,
@@ -44,7 +44,7 @@ def error_middleware_factory(
     _is_prod: bool = is_production_environ()
 
     def _process_and_raise_unexpected_error(request: web.BaseRequest, err: Exception):
-        resp = create_error_response(
+        http_error = create_http_error(
             err,
             "Unexpected Server error",
             web.HTTPInternalServerError,
@@ -58,11 +58,11 @@ def _process_and_raise_unexpected_error(request: web.BaseRequest, err: Exception
                 request.remote,
                 request.method,
                 request.path,
-                resp.status,
+                http_error.status,
                 exc_info=err,
                 stack_info=True,
             )
-        raise resp
+        raise http_error
 
     @web.middleware
     async def _middleware_handler(request: web.Request, handler: Handler):
@@ -115,22 +115,22 @@ async def _middleware_handler(request: web.Request, handler: Handler):
             raise
 
         except NotImplementedError as err:
-            error_response = create_error_response(
+            http_error = create_http_error(
                 err,
                 f"{err}",
                 web.HTTPNotImplemented,
                 skip_internal_error_details=_is_prod,
             )
-            raise error_response from err
+            raise http_error from err
 
         except asyncio.TimeoutError as err:
-            error_response = create_error_response(
+            http_error = create_http_error(
                 err,
                 f"{err}",
                 web.HTTPGatewayTimeout,
                 skip_internal_error_details=_is_prod,
             )
-            raise error_response from err
+            raise http_error from err
 
         except Exception as err:  # pylint: disable=broad-except
             _process_and_raise_unexpected_error(request, err)

packages/service-library/src/servicelib/aiohttp/rest_models.py

@@ -27,4 +27,4 @@ class ErrorType:
     logs: list[LogMessageType] = field(default_factory=list)
     errors: list[ErrorItemType] = field(default_factory=list)
     status: int = 400
-    message: str = "Unexpected client error"
+    message: str = "Unexpected error"

packages/service-library/src/servicelib/aiohttp/rest_responses.py

@@ -13,6 +13,7 @@
 from servicelib.aiohttp.status import HTTP_200_OK
 
 from ..mimetype_constants import MIMETYPE_APPLICATION_JSON
+from ..status_utils import get_code_description
 from .rest_models import ErrorItemType, ErrorType
 
 _ENVELOPE_KEYS = ("data", "error")
@@ -65,18 +66,20 @@ def create_data_response(
 
         response = web.json_response(payload, dumps=json_dumps, status=status)
     except (TypeError, ValueError) as err:
-        response = create_error_response(
-            [
-                err,
-            ],
-            str(err),
-            web.HTTPInternalServerError,
-            skip_internal_error_details=skip_internal_error_details,
+        response = exception_to_response(
+            create_http_error(
+                [
+                    err,
+                ],
+                str(err),
+                web.HTTPInternalServerError,
+                skip_internal_error_details=skip_internal_error_details,
+            )
         )
     return response
 
 
-def create_error_response(
+def create_http_error(
     errors: list[Exception] | Exception,
     reason: str | None = None,
     http_error_cls: type[HTTPError] = web.HTTPInternalServerError,
@@ -94,18 +97,23 @@ def create_error_response(
     # TODO: guarantee no throw!
 
     is_internal_error: bool = http_error_cls == web.HTTPInternalServerError
+    default_message = reason or get_code_description(http_error_cls.status_code)
 
     if is_internal_error and skip_internal_error_details:
         error = ErrorType(
             errors=[],
             status=http_error_cls.status_code,
+            message=default_message,
         )
     else:
+        items = [ErrorItemType.from_error(err) for err in errors]
         error = ErrorType(
-            errors=[ErrorItemType.from_error(err) for err in errors],
+            errors=items,
             status=http_error_cls.status_code,
+            message=items[0].message if items else default_message,
         )
 
+    assert not http_error_cls.empty_body  # nosec
     payload = wrap_as_envelope(error=asdict(error))
 
     return http_error_cls(
@@ -115,6 +123,18 @@ def create_error_response(
     )
 
 
+def exception_to_response(exc: HTTPError) -> web.Response:
+    # Returning web.HTTPException is deprecated so here we have a converter to a response
+    # so it can be used as
+    # SEE https://github.com/aio-libs/aiohttp/issues/2415
+    return web.Response(
+        status=exc.status,
+        headers=exc.headers,
+        reason=exc.reason,
+        text=exc.text,
+    )
+
+
 # Inverse map from code to HTTPException classes
 def _collect_http_exceptions(exception_cls: type[HTTPException] = HTTPException):
     def _pred(obj) -> bool:

packages/service-library/tests/aiohttp/test_rest_responses.py

@@ -5,6 +5,7 @@
 import itertools
 
 import pytest
+from aiohttp import web
 from aiohttp.web_exceptions import (
     HTTPBadRequest,
     HTTPError,
@@ -14,10 +15,14 @@
     HTTPNotModified,
     HTTPOk,
 )
+from servicelib.aiohttp import status
 from servicelib.aiohttp.rest_responses import (
     _STATUS_CODE_TO_HTTP_ERRORS,
+    create_http_error,
+    exception_to_response,
     get_http_error,
 )
+from servicelib.mimetype_constants import MIMETYPE_APPLICATION_JSON
 
 #
 
@@ -53,3 +58,29 @@ def test_collected_http_errors_map(status_code: int, http_error_cls: type[HTTPEr
 
     assert http_error_cls != HTTPError
     assert issubclass(http_error_cls, HTTPError)
+
+
+@pytest.mark.parametrize("skip_details", [True, False])
+def tests_exception_to_response(skip_details: bool):
+    exception = create_http_error(
+        errors=[RuntimeError("foo")],
+        reason="Something whent wrong",
+        http_error_cls=web.HTTPInternalServerError,
+        skip_internal_error_details=skip_details,
+    )
+
+    # For now until deprecated SEE https://github.com/aio-libs/aiohttp/issues/2415
+    assert isinstance(exception, Exception)
+    assert isinstance(exception, web.Response)
+    assert hasattr(exception, "__http_exception__")
+
+    # until they have exception.make_response(), we user
+    response = exception_to_response(exception)
+    assert isinstance(response, web.Response)
+    assert not isinstance(response, Exception)
+    assert not hasattr(response, "__http_exception__")
+
+    assert response.content_type == MIMETYPE_APPLICATION_JSON
+    assert response.status == status.HTTP_500_INTERNAL_SERVER_ERROR
+    assert response.text
+    assert response.body

services/web/server/src/simcore_service_webserver/director_v2/_handlers.py

@@ -10,7 +10,11 @@
 from models_library.utils.json_serialization import json_dumps
 from pydantic import BaseModel, Field, ValidationError, parse_obj_as
 from pydantic.types import NonNegativeInt
-from servicelib.aiohttp.rest_responses import create_error_response, get_http_error
+from servicelib.aiohttp.rest_responses import (
+    create_http_error,
+    exception_to_response,
+    get_http_error,
+)
 from servicelib.common_headers import (
     UNDEFINED_DEFAULT_SIMCORE_USER_AGENT_VALUE,
     X_SIMCORE_USER_AGENT,
@@ -162,15 +166,21 @@ async def start_computation(request: web.Request) -> web.Response:
         return envelope_json_response(data, status_cls=web.HTTPCreated)
 
     except DirectorServiceError as exc:
-        return create_error_response(
-            exc,
-            reason=exc.reason,
-            http_error_cls=get_http_error(exc.status) or web.HTTPServiceUnavailable,
+        return exception_to_response(
+            create_http_error(
+                exc,
+                reason=exc.reason,
+                http_error_cls=get_http_error(exc.status) or web.HTTPServiceUnavailable,
+            )
         )
     except UserDefaultWalletNotFoundError as exc:
-        return create_error_response(exc, http_error_cls=web.HTTPNotFound)
+        return exception_to_response(
+            create_http_error(exc, http_error_cls=web.HTTPNotFound)
+        )
     except WalletNotEnoughCreditsError as exc:
-        return create_error_response(exc, http_error_cls=web.HTTPPaymentRequired)
+        return exception_to_response(
+            create_http_error(exc, http_error_cls=web.HTTPPaymentRequired)
+        )
 
 
 @routes.post(f"/{VTAG}/computations/{{project_id}}:stop", name="stop_computation")
@@ -203,7 +213,7 @@ async def stop_computation(request: web.Request) -> web.Response:
         raise web.HTTPNoContent(content_type=MIMETYPE_APPLICATION_JSON)
 
     except DirectorServiceError as exc:
-        return create_error_response(
+        return create_http_error(
             exc,
             reason=exc.reason,
             http_error_cls=get_http_error(exc.status) or web.HTTPServiceUnavailable,
@@ -252,10 +262,10 @@ async def get_computation(request: web.Request) -> web.Response:
             dumps=json_dumps,
         )
     except DirectorServiceError as exc:
-        return create_error_response(
+        return create_http_error(
             exc,
             reason=exc.reason,
             http_error_cls=get_http_error(exc.status) or web.HTTPServiceUnavailable,
         )
     except ValidationError as exc:
-        return create_error_response(exc, http_error_cls=web.HTTPInternalServerError)
+        return create_http_error(exc, http_error_cls=web.HTTPInternalServerError)

services/web/server/tests/integration/01/test_garbage_collection.py

@@ -87,7 +87,7 @@ async def __delete_all_redis_keys__(redis_settings: RedisSettings):
         decode_responses=True,
     )
     await client.flushall()
-    await client.close(close_connection_pool=True)
+    await client.aclose(close_connection_pool=True)
 
 
 @pytest.fixture(scope="session")

services/web/server/tests/unit/with_dbs/conftest.py

@@ -579,7 +579,7 @@ async def redis_locks_client(
     yield client
 
     await client.flushall()
-    await client.close(close_connection_pool=True)
+    await client.aclose(close_connection_pool=True)
 
 
 # SOCKETS FIXTURES  --------------------------------------------------------