From e3ed9a2f3bf84e72cdc4c90e21494dc2b9c1cd56 Mon Sep 17 00:00:00 2001 From: Sebastiaan Huber Date: Tue, 6 Aug 2024 10:41:48 +0200 Subject: [PATCH] Engine: Ensure node is sealed when process excepts (#6549) Processes that hit a certain exception were not being sealed. This would cause problems when trying to export them, which only allows sealed nodes. The problem occurs when another exception occurs while handling the original exception. An example is when `Process.update_outputs` would raise a `ValueError` because an unstored node had be attached as an output. Since this method is called in `on_entered`, which is called when the process entered a new state, it would be called again when it entered the excepted state. Since the process was already excepted, the rest of the state changes is cut short by `plumpy`. This would cause the process to never go to the final `TERMINATED` state and so the `on_terminated` method would not be called, which is where the process' node is sealed. The solution is to check the current state in `on_entered` and if it is `EXCEPTED` to simply return and no longer perform any updates on the node. This should prevent any other exceptions from being hit and ensure the process transitions properly to the final terminated state. The only update that is still performed is to update the process state on the process' node, otherwise it would not properly be shown as excepted. Cherry-pick: cbf672f1ddf13b3b56b7f3666a7ed2a014bde0cc --- src/aiida/engine/processes/process.py | 12 +++++++++++- tests/engine/test_work_chain.py | 1 + 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/aiida/engine/processes/process.py b/src/aiida/engine/processes/process.py index dfe385b530..b6d83c67f0 100644 --- a/src/aiida/engine/processes/process.py +++ b/src/aiida/engine/processes/process.py @@ -414,8 +414,19 @@ def on_create(self) -> None: @override def on_entered(self, from_state: Optional[plumpy.process_states.State]) -> None: """After entering a new state, save a checkpoint and update the latest process state change timestamp.""" + from plumpy import ProcessState + from aiida.engine.utils import set_process_state_change_timestamp + super().on_entered(from_state) + + if self._state.LABEL is ProcessState.EXCEPTED: + # The process is already excepted so simply update the process state on the node and let the process + # complete the state transition to the terminal state. If another exception is raised during this exception + # handling, the process transitioning is cut short and never makes it to the terminal state. + self.node.set_process_state(self._state.LABEL) + return + # For reasons unknown, it is important to update the outputs first, before doing anything else, otherwise there # is the risk that certain outputs do not get attached before the process reaches a terminal state. Nevertheless # we need to guarantee that the process state gets updated even if the ``update_outputs`` call excepts, for @@ -431,7 +442,6 @@ def on_entered(self, from_state: Optional[plumpy.process_states.State]) -> None: self._save_checkpoint() set_process_state_change_timestamp(self.node) - super().on_entered(from_state) @override def on_terminated(self) -> None: diff --git a/tests/engine/test_work_chain.py b/tests/engine/test_work_chain.py index e7288a3806..1fbb578b2a 100644 --- a/tests/engine/test_work_chain.py +++ b/tests/engine/test_work_chain.py @@ -422,6 +422,7 @@ def illegal(self): orm.QueryBuilder().append(orm.ProcessNode, tag='node').order_by({'node': {'id': 'desc'}}).first(flat=True) ) assert node.is_excepted + assert node.is_sealed assert 'ValueError: Workflow tried returning an unstored `Data` node.' in node.exception def test_same_input_node(self):