🐛 adds tests for S3TransferError; refactoring flaky CI test; better logging for long running task errors (ITISFoundation#3525)

Co-authored-by: Andrei Neagu <[email protected]>

Author: GitHK

packages/service-library/src/servicelib/long_running_tasks/_task.py

@@ -254,7 +254,7 @@ def get_task_result_old(self, task_id: TaskId) -> TaskResult:
                 error = TaskExceptionError(
                     task_id=task_id, exception=exception, traceback=formatted_traceback
                 )
-                logger.warning("%s", f"{error}")
+                logger.warning("Task %s finished with error: %s", task_id, f"{error}")
                 return TaskResult(result=None, error=f"{error}")
         except asyncio.CancelledError:
             error = TaskCancelledError(task_id=task_id)

packages/service-library/tests/fastapi/long_running_tasks/test_long_running_tasks_context_manager.py

@@ -122,7 +122,7 @@ async def test_task_result_times_out(
 
     url = parse_obj_as(AnyHttpUrl, "http://backgroud.testserver.io")
     client = Client(app=bg_task_app, async_client=async_client, base_url=url)
-    timeout = TASK_SLEEP_INTERVAL / 2
+    timeout = TASK_SLEEP_INTERVAL / 10
     with pytest.raises(TaskClientTimeoutError) as exec_info:
         async with periodic_task_result(
             client,

packages/simcore-sdk/tests/unit/test_node_ports_common_file_io_utils.py

@@ -1,31 +1,81 @@
 # pylint: disable=redefined-outer-name
 # pylint: disable=unused-argument
 
-from asyncio import BaseEventLoop
+from dataclasses import dataclass
+from typing import AsyncIterable
 
 import pytest
-from aiohttp import web
-from aiohttp.test_utils import TestClient
+from aiohttp import ClientResponse, ClientSession
+from aioresponses import aioresponses
 from simcore_sdk.node_ports_common.file_io_utils import (
     ExtendedClientResponseError,
+    _check_for_aws_http_errors,
     _raise_for_status,
 )
 
-TEST_ERROR = "OPSIE there was an error here"
+A_TEST_ROUTE = "http://a-fake-address:1249/test-route"
 
 
 @pytest.fixture
-def test_client(loop: BaseEventLoop, aiohttp_client) -> TestClient:
-    async def raise_error(request):
-        return web.Response(body=TEST_ERROR, status=web.HTTPBadRequest.status_code)
+async def client_session() -> AsyncIterable[ClientSession]:
+    async with ClientSession() as session:
+        yield session
 
-    app = web.Application()
-    app.router.add_get("/", raise_error)
-    return loop.run_until_complete(aiohttp_client(app))
 
+async def test_raise_for_status(
+    aioresponses_mocker: aioresponses, client_session: ClientSession
+):
+    aioresponses_mocker.get(
+        A_TEST_ROUTE, body="OPSIE there was an error here", status=400
+    )
 
-async def test_raise_for_status(test_client: TestClient):
-    resp = await test_client.get("/")
-    with pytest.raises(ExtendedClientResponseError) as exe_info:
-        await _raise_for_status(resp)
-    assert TEST_ERROR in f"{exe_info.value}"
+    async with client_session.get(A_TEST_ROUTE) as resp:
+        assert isinstance(resp, ClientResponse)
+
+        with pytest.raises(ExtendedClientResponseError) as exe_info:
+            await _raise_for_status(resp)
+        assert "OPSIE there was an error here" in f"{exe_info.value}"
+
+
+@dataclass
+class _TestParams:
+    will_retry: bool
+    status_code: int
+    body: str = ""
+
+
+@pytest.mark.parametrize(
+    "test_params",
+    [
+        _TestParams(
+            will_retry=True,
+            status_code=400,
+            body='<?xml version="1.0" encoding="UTF-8"?><Error><Code>RequestTimeout</Code>'
+            "<Message>Your socket connection to the server was not read from or written to within "
+            "the timeout period. Idle connections will be closed.</Message>"
+            "<RequestId>7EE901348D6C6812</RequestId><HostId>"
+            "FfQE7jdbUt39E6mcQq/"
+            "ZeNR52ghjv60fccNT4gCE4IranXjsGLG+L6FUyiIxx1tAuXL9xtz2NAY7ZlbzMTm94fhY3TBiCBmf"
+            "</HostId></Error>",
+        ),
+        _TestParams(will_retry=True, status_code=500),
+        _TestParams(will_retry=True, status_code=503),
+        _TestParams(will_retry=False, status_code=400),
+        _TestParams(will_retry=False, status_code=200),
+        _TestParams(will_retry=False, status_code=399),
+    ],
+)
+async def test_check_for_aws_http_errors(
+    aioresponses_mocker: aioresponses,
+    client_session: ClientSession,
+    test_params: _TestParams,
+):
+    aioresponses_mocker.get(
+        A_TEST_ROUTE, body=test_params.body, status=test_params.status_code
+    )
+
+    async with client_session.get(A_TEST_ROUTE) as resp:
+        try:
+            await _raise_for_status(resp)
+        except ExtendedClientResponseError as exception:
+            assert _check_for_aws_http_errors(exception) is test_params.will_retry