refactored

Author: Andrei Neagu

packages/service-library/src/servicelib/long_running_interfaces/_client.py

@@ -10,8 +10,9 @@
 
 from ._errors import (
     AlreadyStartedError,
-    NoMoreRetryAttemptsError,
+    FinishedWithError,
     TimedOutError,
+    UnexpectedNoMoreRetryAttemptsError,
     UnexpectedResultTypeError,
     UnexpectedStatusError,
 )
@@ -128,8 +129,26 @@ async def _decrease_remaining_attempts_or_raise(
             remaining_attempts = await self._format_remaining_attempts(
                 unique_id, retry_count
             )
-            raise NoMoreRetryAttemptsError(
+            # report remote error if there is one
+            if last_result and last_result.error is not None:
+                _logger.warning(
+                    "unique_id='%s', finished with error from remote: %s='%s'\n%s",
+                    unique_id,
+                    last_result.error.error_type,
+                    last_result.error.error_message,
+                    last_result.error.traceback,
+                )
+                raise FinishedWithError(
+                    unique_id=unique_id,
+                    error=last_result.error.error_type,
+                    message=last_result.error.error_message,
+                    traceback=last_result.error.traceback,
+                )
+
+            # NOTE: this edge case should not happen
+            raise UnexpectedNoMoreRetryAttemptsError(
                 unique_id=unique_id,
+                retry_count=retry_count,
                 remaining_attempts=remaining_attempts,
                 last_result=last_result,
             )

packages/service-library/src/servicelib/long_running_interfaces/_errors.py

@@ -11,8 +11,12 @@ class UnexpectedJobNotFoundError(BaseLongRunningError):
     )
 
 
-class NoMoreRetryAttemptsError(BaseLongRunningError):
-    msg_template = "attempt {remaining_attempts} for unique_id='{unique_id}' with last_result='{last_result}'"
+class FinishedWithError(BaseLongRunningError):
+    msg_template = "unique_id='{unique_id}' finished with error='{error}' message='{message}'\n{traceback}"
+
+
+class UnexpectedNoMoreRetryAttemptsError(FinishedWithError):
+    msg_template = "attempt {remaining_attempts} of {retry_count} for unique_id='{unique_id}' with last_result='{last_result}'"
 
 
 class UnexpectedStatusError(BaseLongRunningError):

packages/service-library/src/servicelib/long_running_interfaces/_models.py

@@ -38,6 +38,7 @@ def unique_id(self) -> JobUniqueId:
 
 
 class ErrorModel(BaseModel):
+    error_type: type
     error_message: str
     traceback: str
 

packages/service-library/src/servicelib/long_running_interfaces/_rpc/server.py

@@ -125,5 +125,9 @@ async def result(self, unique_id: JobUniqueId) -> ResultModel:
         except Exception as e:  # pylint:disable=broad-exception-caught
             formatted_traceback = "\n".join(traceback.format_tb(e.__traceback__))
             return ResultModel(
-                error=ErrorModel(error_message=f"{e}", traceback=formatted_traceback)
+                error=ErrorModel(
+                    error_type=type(e),
+                    error_message=f"{e}",
+                    traceback=formatted_traceback,
+                )
             )

packages/service-library/tests/long_running_interfaces/test_workflow.py

@@ -8,12 +8,9 @@
 from typing import Any
 
 import pytest
-from pydantic import NonNegativeInt
+from pydantic import NonNegativeInt, ValidationError
 from servicelib.long_running_interfaces import Client, LongRunningNamespace, Server
-from servicelib.long_running_interfaces._errors import (
-    NoMoreRetryAttemptsError,
-    TimedOutError,
-)
+from servicelib.long_running_interfaces._errors import FinishedWithError, TimedOutError
 from servicelib.long_running_interfaces_runners.asyncio_tasks import (
     AsyncioTasksJobInterface,
     AsyncTaskRegistry,
@@ -80,16 +77,6 @@ async def sleep_forever_f() -> None:
     await server.teardown()
 
 
-# Figure out how to cause interruption in SERVER & CLIENT code, in order to simulate shakyness in the tests
-# write something that cancels and restarts the job runtime
-
-# TODO: add the following tests
-# start a job and wait for it to finish
-# start a job that takes too much time
-# start a job that raises an error
-# start a job that after 95% of timeout raises an errors x times in a row (we can test if it can successfully finish)
-
-
 @dataclass
 class _CustomClass:
     number: float
@@ -123,11 +110,6 @@ async def test_workflow(
     assert type(result) is expected_type
 
 
-# TODO: count error in logs to be sure soemthing is worng
-# TODO: count that it's missing with retry_count=0 and that it raises a vlidation error
-# TODO: figure out a way to raise the error
-
-
 async def test_timeout_error(server: Server, client: Client):
     with pytest.raises(TimedOutError):
         await client.ensure_result(
@@ -147,13 +129,38 @@ async def test_timeout_during_failing_retry(server: Server, client: Client):
 
 
 @pytest.mark.parametrize("retry_count", [1, 2])
-async def test_stops_after_n_retry_attempts(
+async def test_raisese_after_n_retry_attempts(
     server: Server, client: Client, retry_count: NonNegativeInt
 ):
-    with pytest.raises(NoMoreRetryAttemptsError):
+    with pytest.raises(FinishedWithError):
         await client.ensure_result(
             "raising_f",
             expected_type=type(None),
             timeout=timedelta(seconds=10),
             retry_count=retry_count,
         )
+
+
+async def test_timeout_error_retry_count_zero(server: Server, client: Client):
+    with pytest.raises(ValidationError) as exec_info:
+        await client.ensure_result(
+            "some_f",
+            expected_type=type(None),
+            timeout=timedelta(seconds=10),
+            retry_count=0,
+        )
+
+    assert "retry_count" in f"{exec_info.value}"
+    assert "Input should be greater than 0" in f"{exec_info.value}"
+
+
+# Figure out how to cause interruption in SERVER & CLIENT code, in order to
+#   simulate shakyness in the tests
+# write something that cancels and restarts the job runtime
+
+# TODO: add the following tests
+# start a job and wait for it to finish
+# start a job that takes too much time
+# start a job that raises an error
+# start a job that after 95% of timeout raises an errors x times in a row
+#   (we can test if it can successfully finish)