PR feedback

Author: andystaples

durabletask/entities/entity_instance_id.py

@@ -1,5 +1,9 @@
 class EntityInstanceId:
     def __init__(self, entity: str, key: str):
+        if not entity or not key:
+            raise ValueError("Entity name and key cannot be empty.")
+        if "@" in key:
+            raise ValueError("Entity key cannot contain '@' symbol.")
         self.entity = entity.lower()
         self.key = key
 
@@ -35,14 +39,10 @@ def parse(entity_id: str) -> "EntityInstanceId":
         ValueError
             If the input string is not in the correct format.
         """
+        if not entity_id.startswith("@"):
+            raise ValueError("Entity ID must start with '@'.")
         try:
-            if not entity_id.startswith("@"):
-                raise ValueError("Entity ID must start with '@'.")
             _, entity, key = entity_id.split("@", 2)
-            if not entity or not key:
-                raise ValueError("Entity name and key cannot be empty.")
-            if "@" in key:
-                raise ValueError("Entity instance ID string should not contain more than two '@' symbols.")
-            return EntityInstanceId(entity=entity, key=key)
         except ValueError as ex:
-            raise ValueError(f"Invalid entity ID format: {entity_id}", ex)
+            raise ValueError(f"Invalid entity ID format: {entity_id}") from ex
+        return EntityInstanceId(entity=entity, key=key)

durabletask/worker.py

@@ -905,7 +905,7 @@ def set_complete(
         if result is not None:
             try:
                 result_json = result if is_result_encoded else shared.to_json(result)
-            except TypeError:
+            except (ValueError, TypeError):
                 result_json = shared.to_json(str(JsonEncodeOutputException(result)))
         action = ph.new_complete_orchestration_action(
             self.next_sequence_number(), status, result_json
@@ -1779,7 +1779,7 @@ def process_event(
                 if not entity_task:
                     if not ctx.is_replaying:
                         self._logger.warning(
-                            f"{ctx.instance_id}: Ignoring unexpected entityOperationCompleted event with request ID = {request_id}."
+                            f"{ctx.instance_id}: Ignoring unexpected entityOperationFailed event with request ID = {request_id}."
                         )
                     return
                 failure = EntityOperationFailedException(
@@ -1801,7 +1801,7 @@ def process_event(
                 if action and action.HasField("sendEntityMessage"):
                     if action.sendEntityMessage.HasField("entityOperationCalled"):
                         entity_id, event_id = self._parse_entity_event_sent_input(event)
-                        ctx._entity_task_id_map[event_id] = (entity_id, event.entityOperationCalled.operation, event.eventId)
+                        ctx._entity_task_id_map[event_id] = (entity_id, action.sendEntityMessage.entityOperationCalled.operation, event.eventId)
                     elif action.sendEntityMessage.HasField("entityLockRequested"):
                         entity_id, event_id = self._parse_entity_event_sent_input(event)
                         ctx._entity_lock_task_id_map[event_id] = (entity_id, event.eventId)

tests/durabletask-azuremanaged/entities/test_dts_entity_failure_handling.py

@@ -86,8 +86,6 @@ def test_orchestrator(ctx: task.OrchestrationContext, _):
         try:
             yield ctx.call_entity(entity_id, "fail")
         except task.TaskFailedError as e:
-            # Need to return the exception wrapped in a list to avoid
-            # https://github.com/microsoft/durabletask-python/issues/108
             return e.details.message  # returning just the message to avoid issues with JSON serialization of FailureDetails
 
     # Start a worker, which will connect to the sidecar in a background thread
@@ -157,11 +155,11 @@ def test_orchestrator(ctx: task.OrchestrationContext, _):
         with (yield ctx.lock_entities([entity_id])):
             try:
                 yield ctx.call_entity(entity_id, "fail")
-            except task.TaskFailedError as e:
+            except task.TaskFailedError:
                 exception_count += 1
         try:
             yield ctx.call_entity(entity_id, "fail")
-        except task.TaskFailedError as e:
+        except task.TaskFailedError:
             exception_count += 1
         return exception_count