item error handling
This commit is contained in:
@@ -55,6 +55,12 @@ Supported policy actions:
|
||||
writes the full ordered error list once. On clean completion, `collect` writes an
|
||||
empty list and emits `done`.
|
||||
|
||||
Current sync concurrent foreach supports `skip` and `collect`. Item node
|
||||
exceptions under those policies mark the child item frame `FAILED`, store a
|
||||
pending failed item result on the parent barrier, and wake the parent to refill
|
||||
or finish. The failed frame stays visible for observability; the parent foreach
|
||||
is responsible for turning handled item failures into aggregate control flow.
|
||||
|
||||
Collected error records include item index, frame id, failing node id, error
|
||||
type/message, and the item value when it can be represented safely.
|
||||
|
||||
|
||||
@@ -30,6 +30,10 @@ Already implemented:
|
||||
- Barrier write validation rejects ambiguous sibling writes: same-path sibling
|
||||
writes require a `mergeable` reducer, and ancestor/descendant
|
||||
sibling writes are rejected.
|
||||
- Concurrent `item_error.action="skip"` and `"collect"` are supported.
|
||||
- `collect` writes ordered item error records to `collect_to`, writes an empty
|
||||
list on clean success, and emits `completed_with_errors` only when failures
|
||||
were collected.
|
||||
|
||||
## Non-Goals For Phase 4
|
||||
|
||||
@@ -112,7 +116,7 @@ Plan:
|
||||
|
||||
## Slice 4: Item Error Policies
|
||||
|
||||
Implement after barrier success commits are correct.
|
||||
Implemented after barrier success commits became correct.
|
||||
|
||||
Scope:
|
||||
|
||||
|
||||
@@ -262,6 +262,25 @@ class ForeachBarrierState:
|
||||
)
|
||||
existing.patch.changes.update(patch.changes)
|
||||
|
||||
def add_failure(self, *, error: ItemErrorRecord) -> None:
|
||||
"""Buffer one handled item failure for the foreach barrier.
|
||||
|
||||
The child frame stays `FAILED` for observability. The parent barrier
|
||||
owns whether that failed child is skipped, collected, or treated as a
|
||||
whole-run failure.
|
||||
"""
|
||||
existing = self.pending_results.get(error.index)
|
||||
if existing is not None:
|
||||
raise WorkflowExecutionError(
|
||||
f"foreach item result for index {error.index!r} already exists"
|
||||
)
|
||||
self.pending_results[error.index] = PendingItemResult(
|
||||
index=error.index,
|
||||
frame_id=error.frame_id,
|
||||
status="failed",
|
||||
error=error,
|
||||
)
|
||||
|
||||
|
||||
def item_frame_owner(frame: ExecutionFrame) -> tuple[str, str, int] | None:
|
||||
"""Return parent frame id, foreach node id, and item index for item frames."""
|
||||
|
||||
@@ -7,12 +7,16 @@ from wf_core.errors import WorkflowExecutionError
|
||||
from wf_core.models.steps import ForeachNode
|
||||
from wf_core.models.workflow import Workflow
|
||||
from wf_core.run_state import ExecutionFrame, FrameStatus, RunState, StepExecutionResult
|
||||
from wf_core.runtime.foreach_state import ForeachBarrierState
|
||||
from wf_core.runtime.foreach_state import ForeachBarrierState, ItemErrorRecord
|
||||
from wf_core.runtime.ops.flow import advance_frame, append_step_result_trace
|
||||
from wf_core.runtime.ops.frames import frame_context_values
|
||||
from wf_core.runtime.ops.index import WorkflowIndex
|
||||
from wf_core.runtime.ops.merges import ReducerDefinition
|
||||
from wf_core.runtime.ops.state import build_barrier_patch, commit_state_patch
|
||||
from wf_core.runtime.ops.state import (
|
||||
StatePatch,
|
||||
build_barrier_patch,
|
||||
commit_state_patch,
|
||||
)
|
||||
from wf_core.runtime.scheduler import (
|
||||
ForeachIterationMetadata,
|
||||
add_frame,
|
||||
@@ -120,10 +124,6 @@ def _step_foreach_concurrent(
|
||||
*,
|
||||
reducers: Mapping[str, ReducerDefinition] | None = None,
|
||||
) -> RunState:
|
||||
if step.item_error.action != "fail":
|
||||
raise WorkflowExecutionError(
|
||||
"concurrent foreach v1 only supports item_error.action='fail'"
|
||||
)
|
||||
if step.concurrent is None:
|
||||
raise WorkflowExecutionError("concurrent foreach requires concurrent policy")
|
||||
frame = run.current_frame()
|
||||
@@ -133,7 +133,7 @@ def _step_foreach_concurrent(
|
||||
elif barrier.mode != "concurrent":
|
||||
raise WorkflowExecutionError("malformed concurrent foreach barrier mode")
|
||||
|
||||
_finish_completed_children(run, barrier)
|
||||
_finish_completed_children(run, step, barrier)
|
||||
iterable = _resolve_foreach_iterable(run, frame, step)
|
||||
_admit_concurrent_children(
|
||||
run=run,
|
||||
@@ -179,18 +179,49 @@ def _resolve_foreach_iterable(
|
||||
return iterable
|
||||
|
||||
|
||||
def _finish_completed_children(run: RunState, barrier: ForeachBarrierState) -> None:
|
||||
def _finish_completed_children(
|
||||
run: RunState,
|
||||
step: ForeachNode,
|
||||
barrier: ForeachBarrierState,
|
||||
) -> None:
|
||||
for child_id in tuple(barrier.outstanding_frame_ids):
|
||||
child = run.frames[child_id]
|
||||
if child.status == FrameStatus.COMPLETED:
|
||||
barrier.finish_child(child_id)
|
||||
elif child.status == FrameStatus.FAILED:
|
||||
if step.item_error.action in {"skip", "collect"}:
|
||||
barrier.finish_child(child_id)
|
||||
barrier.add_failure(error=_item_error_record(child))
|
||||
continue
|
||||
message = child.metadata.get("error", "unknown item failure")
|
||||
raise WorkflowExecutionError(
|
||||
f"concurrent foreach item frame {child_id!r} failed: {message}"
|
||||
)
|
||||
|
||||
|
||||
def _item_error_record(child: ExecutionFrame) -> ItemErrorRecord:
|
||||
metadata = ForeachIterationMetadata.from_frame(child)
|
||||
if metadata is None:
|
||||
raise WorkflowExecutionError(
|
||||
f"failed foreach item frame {child.id!r} is missing item metadata"
|
||||
)
|
||||
error_type = child.metadata.get("error_type", "Exception")
|
||||
message = child.metadata.get("error", "unknown item failure")
|
||||
node_id = child.metadata.get("failed_at_node_id", child.node_id)
|
||||
if not all(isinstance(value, str) for value in (error_type, message, node_id)):
|
||||
raise WorkflowExecutionError(
|
||||
f"malformed failure metadata for foreach item frame {child.id!r}"
|
||||
)
|
||||
return ItemErrorRecord(
|
||||
index=metadata.loop_index,
|
||||
frame_id=child.id,
|
||||
node_id=node_id,
|
||||
error_type=error_type,
|
||||
message=message,
|
||||
item=metadata.loop_item,
|
||||
)
|
||||
|
||||
|
||||
def _admit_concurrent_children(
|
||||
*,
|
||||
run: RunState,
|
||||
@@ -261,13 +292,34 @@ def _finish_concurrent_foreach(
|
||||
barrier: ForeachBarrierState,
|
||||
reducers: Mapping[str, ReducerDefinition] | None = None,
|
||||
) -> RunState:
|
||||
next_node_id = index.next_node_id(frame.node_id, "done")
|
||||
error_records = [
|
||||
result.error.to_metadata()
|
||||
for result in sorted(
|
||||
barrier.pending_results.values(), key=lambda item: item.index
|
||||
)
|
||||
if result.status == "failed" and result.error is not None
|
||||
]
|
||||
outcome = "completed_with_errors" if error_records else "done"
|
||||
next_node_id = index.next_node_id(frame.node_id, outcome)
|
||||
success_patches = [
|
||||
result.patch
|
||||
for result in (
|
||||
barrier.pending_results[item_index]
|
||||
for item_index in sorted(barrier.pending_results)
|
||||
)
|
||||
if result.status == "succeeded"
|
||||
]
|
||||
item_patches = list(success_patches)
|
||||
if step.item_error.action == "collect":
|
||||
collect_to = step.item_error.collect_to
|
||||
if collect_to is None:
|
||||
raise WorkflowExecutionError(
|
||||
"collect item error policy requires collect_to"
|
||||
)
|
||||
item_patches.append(StatePatch(changes={str(collect_to): error_records}))
|
||||
combined = build_barrier_patch(
|
||||
workflow,
|
||||
[
|
||||
barrier.pending_results[item_index].patch
|
||||
for item_index in sorted(barrier.pending_results)
|
||||
],
|
||||
item_patches,
|
||||
run.state,
|
||||
reducers=reducers,
|
||||
)
|
||||
@@ -279,15 +331,16 @@ def _finish_concurrent_foreach(
|
||||
step_type=step.type,
|
||||
next_node_id=next_node_id,
|
||||
result=StepExecutionResult(
|
||||
outcome="done",
|
||||
outcome=outcome,
|
||||
resolved_input={
|
||||
"count": barrier.next_index,
|
||||
"index": barrier.next_index,
|
||||
"committed_items": len(barrier.pending_results),
|
||||
"committed_items": len(success_patches),
|
||||
"failed_items": len(error_records),
|
||||
},
|
||||
output={},
|
||||
state_changes=state_changes,
|
||||
),
|
||||
)
|
||||
advance_frame(run, frame, outcome="done", next_node_id=next_node_id)
|
||||
advance_frame(run, frame, outcome=outcome, next_node_id=next_node_id)
|
||||
return run
|
||||
|
||||
@@ -196,6 +196,11 @@ def resolve_no_ready_frames(run: RunState) -> RunStatus:
|
||||
"""Classify an empty ready queue into terminal, paused, or deadlocked state."""
|
||||
if run.status == RunStatus.INTERRUPTED:
|
||||
return RunStatus.INTERRUPTED
|
||||
if any(
|
||||
frame.parent_frame_id is None and frame.status == FrameStatus.COMPLETED
|
||||
for frame in run.frames.values()
|
||||
):
|
||||
return RunStatus.COMPLETED
|
||||
if any(frame.status == FrameStatus.FAILED for frame in run.frames.values()):
|
||||
return RunStatus.FAILED
|
||||
if run.frames and all(
|
||||
|
||||
@@ -27,8 +27,12 @@ from wf_core.runtime.ops.nodes import (
|
||||
execute_node_use,
|
||||
execute_node_use_async,
|
||||
)
|
||||
from wf_core.runtime.scheduler import select_next_frame
|
||||
from wf_core.run_state import FrameStatus, RunState
|
||||
from wf_core.runtime.scheduler import (
|
||||
ForeachIterationMetadata,
|
||||
select_next_frame,
|
||||
wake_parent_for_child_progress,
|
||||
)
|
||||
from wf_core.run_state import ExecutionFrame, FrameStatus, RunState
|
||||
|
||||
from .preparation import prepare_step
|
||||
|
||||
@@ -84,6 +88,7 @@ def step_workflow(
|
||||
|
||||
if isinstance(step, NodeUse):
|
||||
node_def = index.node_defs[step.node]
|
||||
try:
|
||||
step_result = execute_node_use(
|
||||
workflow,
|
||||
run,
|
||||
@@ -92,6 +97,10 @@ def step_workflow(
|
||||
registry,
|
||||
reducers=reducers,
|
||||
)
|
||||
except Exception as exc:
|
||||
if _mark_handled_item_failure(run, index, frame, exc):
|
||||
return run
|
||||
raise
|
||||
elif isinstance(step, ConditionNode):
|
||||
step_result = handle_condition_step(run, step)
|
||||
elif isinstance(step, JoinNode):
|
||||
@@ -116,6 +125,31 @@ def step_workflow(
|
||||
)
|
||||
|
||||
|
||||
def _mark_handled_item_failure(
|
||||
run: RunState,
|
||||
index: WorkflowIndex,
|
||||
frame: ExecutionFrame,
|
||||
exc: Exception,
|
||||
) -> bool:
|
||||
"""Record skip/collect item failures without failing the whole run here."""
|
||||
metadata = ForeachIterationMetadata.from_frame(frame)
|
||||
if metadata is None or frame.parent_frame_id is None:
|
||||
return False
|
||||
owner_step = index.nodes_by_id.get(metadata.foreach_node_id)
|
||||
if not isinstance(owner_step, ForeachNode):
|
||||
return False
|
||||
if owner_step.item_error.action == "fail":
|
||||
return False
|
||||
|
||||
frame.status = FrameStatus.FAILED
|
||||
frame.metadata["error"] = str(exc)
|
||||
frame.metadata["error_type"] = type(exc).__name__
|
||||
frame.metadata["failed_at_node_id"] = frame.node_id
|
||||
wake_parent_for_child_progress(run, frame.id)
|
||||
run.sync_from_current_frame()
|
||||
return True
|
||||
|
||||
|
||||
async def step_workflow_async(
|
||||
workflow: Workflow,
|
||||
run: RunState,
|
||||
@@ -137,6 +171,7 @@ async def step_workflow_async(
|
||||
|
||||
if isinstance(step, NodeUse):
|
||||
node_def = index.node_defs[step.node]
|
||||
try:
|
||||
step_result = await execute_node_use_async(
|
||||
workflow,
|
||||
run,
|
||||
@@ -145,6 +180,10 @@ async def step_workflow_async(
|
||||
registry,
|
||||
reducers=reducers,
|
||||
)
|
||||
except Exception as exc:
|
||||
if _mark_handled_item_failure(run, index, frame, exc):
|
||||
return run
|
||||
raise
|
||||
elif isinstance(step, ConditionNode):
|
||||
step_result = handle_condition_step(run, step)
|
||||
elif isinstance(step, JoinNode):
|
||||
|
||||
@@ -96,43 +96,6 @@ def test_sync_concurrent_foreach_respects_max_active_by_refill_trace() -> None:
|
||||
assert all(entry.resolved_input["active_count"] < 2 for entry in loop_entries)
|
||||
|
||||
|
||||
def test_sync_concurrent_foreach_rejects_non_fail_item_policy_for_now() -> None:
|
||||
workflow = _workflow(
|
||||
state_schema=StateSchema.from_field_map(
|
||||
{
|
||||
"items": StateField(type="array"),
|
||||
"seen": StateField(
|
||||
type="array",
|
||||
reducer=ReducerRef(name="wf.std.append"),
|
||||
),
|
||||
"errors": StateField(type="array"),
|
||||
}
|
||||
),
|
||||
foreach=ForeachNode.model_validate(
|
||||
{
|
||||
"id": "each",
|
||||
"type": "foreach",
|
||||
"over": "state.items",
|
||||
"as": "item",
|
||||
"mode": "concurrent",
|
||||
"concurrent": {"max_active": 2, "max_outstanding": 2},
|
||||
"item_error": {"action": "collect", "collect_to": "state.errors"},
|
||||
}
|
||||
),
|
||||
include_completed_with_errors=True,
|
||||
)
|
||||
|
||||
with pytest.raises(
|
||||
WorkflowExecutionError,
|
||||
match="only supports item_error.action='fail'",
|
||||
):
|
||||
execute_workflow(
|
||||
workflow,
|
||||
{"items": ["a"]},
|
||||
{"record": lambda payload, _ctx: {"outcome": "ok", "output": payload}},
|
||||
)
|
||||
|
||||
|
||||
def test_sync_concurrent_foreach_fails_run_on_item_runtime_error() -> None:
|
||||
workflow = _workflow(
|
||||
state_schema=StateSchema.from_field_map(
|
||||
|
||||
@@ -0,0 +1,153 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Any
|
||||
|
||||
from wf_core import (
|
||||
END,
|
||||
Edge,
|
||||
ForeachNode,
|
||||
NodeDef,
|
||||
NodeUse,
|
||||
ReducerRef,
|
||||
SchemaRef,
|
||||
StateField,
|
||||
StateSchema,
|
||||
Workflow,
|
||||
execute_workflow,
|
||||
)
|
||||
|
||||
|
||||
def test_concurrent_foreach_skip_emits_completed_with_errors() -> None:
|
||||
workflow = _workflow(item_error={"action": "skip"})
|
||||
|
||||
run = execute_workflow(
|
||||
workflow,
|
||||
{"items": ["a", "b", "c"]},
|
||||
{"record": _fail_on_b},
|
||||
)
|
||||
|
||||
assert run.state["seen"] == ["a", "c"]
|
||||
assert run.frames["root:each:1"].status == "failed"
|
||||
foreach_entries = [entry for entry in run.trace if entry.step_type == "foreach"]
|
||||
assert foreach_entries[-1].outcome == "completed_with_errors"
|
||||
assert foreach_entries[-1].resolved_input["failed_items"] == 1
|
||||
assert foreach_entries[-1].state_changes["state.seen"] == ["a", "c"]
|
||||
|
||||
|
||||
def test_concurrent_foreach_collect_writes_ordered_error_records() -> None:
|
||||
workflow = _workflow(item_error={"action": "collect", "collect_to": "state.errors"})
|
||||
|
||||
run = execute_workflow(
|
||||
workflow,
|
||||
{"items": ["a", "b", "c"]},
|
||||
{"record": _fail_on_b},
|
||||
)
|
||||
|
||||
assert run.state["seen"] == ["a", "c"]
|
||||
assert len(run.state["errors"]) == 1
|
||||
error = run.state["errors"][0]
|
||||
assert error["index"] == 1
|
||||
assert error["frame_id"] == "root:each:1"
|
||||
assert error["node_id"] == "record"
|
||||
assert error["error_type"] == "ValueError"
|
||||
assert error["message"] == "bad item"
|
||||
assert error["item"] == "b"
|
||||
foreach_entries = [entry for entry in run.trace if entry.step_type == "foreach"]
|
||||
assert foreach_entries[-1].outcome == "completed_with_errors"
|
||||
assert foreach_entries[-1].state_changes["state.errors"] == run.state["errors"]
|
||||
|
||||
|
||||
def test_concurrent_foreach_collect_writes_empty_list_on_clean_success() -> None:
|
||||
workflow = _workflow(item_error={"action": "collect", "collect_to": "state.errors"})
|
||||
|
||||
run = execute_workflow(
|
||||
workflow,
|
||||
{"items": ["a", "b"]},
|
||||
{"record": lambda payload, _ctx: {"outcome": "ok", "output": payload}},
|
||||
)
|
||||
|
||||
assert run.state["seen"] == ["a", "b"]
|
||||
assert run.state["errors"] == []
|
||||
foreach_entries = [entry for entry in run.trace if entry.step_type == "foreach"]
|
||||
assert foreach_entries[-1].outcome == "done"
|
||||
assert foreach_entries[-1].state_changes["state.errors"] == []
|
||||
|
||||
|
||||
def _fail_on_b(payload: dict[str, Any], _ctx: object) -> dict[str, Any]:
|
||||
if payload["value"] == "b":
|
||||
raise ValueError("bad item")
|
||||
return {"outcome": "ok", "output": payload}
|
||||
|
||||
|
||||
def _workflow(*, item_error: dict[str, object]) -> Workflow:
|
||||
return Workflow(
|
||||
name="concurrent_foreach_item_errors",
|
||||
input_schema=SchemaRef(
|
||||
type="object",
|
||||
properties={"items": {"type": "array"}},
|
||||
),
|
||||
state_schema=StateSchema.from_field_map(
|
||||
{
|
||||
"items": StateField(type="array"),
|
||||
"seen": StateField(
|
||||
type="array",
|
||||
reducer=ReducerRef(name="wf.std.append"),
|
||||
),
|
||||
"errors": StateField(type="array"),
|
||||
}
|
||||
),
|
||||
output_schema=SchemaRef(
|
||||
type="object",
|
||||
properties={"seen": {"type": "array"}, "errors": {"type": "array"}},
|
||||
),
|
||||
node_defs=[
|
||||
NodeDef(
|
||||
name="record",
|
||||
input_schema=SchemaRef(
|
||||
type="object",
|
||||
properties={"value": {}, "seen": {}},
|
||||
required=["value", "seen"],
|
||||
),
|
||||
output_schema=SchemaRef(
|
||||
type="object",
|
||||
properties={"value": {}, "seen": {}},
|
||||
required=["seen"],
|
||||
),
|
||||
outcomes=["ok"],
|
||||
)
|
||||
],
|
||||
start="each",
|
||||
nodes=[
|
||||
ForeachNode.model_validate(
|
||||
{
|
||||
"id": "each",
|
||||
"type": "foreach",
|
||||
"over": "state.items",
|
||||
"as": "item",
|
||||
"mode": "concurrent",
|
||||
"concurrent": {"max_active": 2, "max_outstanding": 2},
|
||||
"item_error": item_error,
|
||||
}
|
||||
),
|
||||
NodeUse.model_validate(
|
||||
{
|
||||
"id": "record",
|
||||
"type": "node",
|
||||
"node": "record",
|
||||
"input": [
|
||||
{"target": "value", "path": "context.item"},
|
||||
{"target": "seen", "path": "context.item"},
|
||||
],
|
||||
"output": [{"source": "seen", "target": "state.seen"}],
|
||||
}
|
||||
),
|
||||
],
|
||||
edges=[
|
||||
Edge.model_validate({"from": "each", "outcome": "loop", "to": "record"}),
|
||||
Edge.model_validate({"from": "record", "outcome": "ok", "to": END}),
|
||||
Edge.model_validate({"from": "each", "outcome": "done", "to": END}),
|
||||
Edge.model_validate(
|
||||
{"from": "each", "outcome": "completed_with_errors", "to": END}
|
||||
),
|
||||
],
|
||||
)
|
||||
Reference in New Issue
Block a user