Skip to content

Commit a242a60

Browse files
crprashantpinodeca
authored andcommitted
Replace df.break() JSON sentinel with typed NodeError
Refactor break propagation from an in-band {"__break__": true} sentinel string to a typed NodeError::Break enum so Rust's ? operator auto-propagates break through compound nodes (THEN/IF/JOIN/RACE/subtree). The loop node is now the sole break catcher, so forgetting to propagate a break is a compile error rather than a silently-ignored value. - Add NodeError { Break, Failure } + NodeResult; From<String>/<&str> map to Failure so activity/helper errors auto-convert. - Carry break across the sub-orchestration boundary via a typed SubtreeControl field on SubtreeEnvelope instead of a sentinel in result. - Preserve in-flight-upgrade safety: parse_subtree_envelope re-raises a legacy {"__break__"} sentinel (written by <=0.2.2 binaries) as NodeError::Break so breaks persisted by an older binary are not silently swallowed after upgrade. - Document the Break-is-control-flow contract on the execute() entry point. - Add 10 unit tests for envelope parsing (legacy + typed) and tighten the break-in-join-race E2E to assert the exact break value. Fixes #148
1 parent a9f86bc commit a242a60

3 files changed

Lines changed: 524 additions & 105 deletions

File tree

docs/ARCHITECTURE.md

Lines changed: 50 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,42 @@ pub async fn execute(
521521

522522
### Node Execution
523523

524+
Internal node handlers return `NodeResult`, a `Result` whose error arm is a typed
525+
`NodeError` rather than a plain `String`. This lets `df.break()` propagate through the
526+
compound nodes (`THEN`, `IF`, `JOIN`, `RACE`) automatically via the `?` operator, instead
527+
of every handler having to recognise an in-band JSON break sentinel:
528+
529+
```rust
530+
// src/orchestrations/execute_function_graph.rs
531+
pub enum NodeError {
532+
/// df.break() fired. Carries the break value. Caught only by execute_loop_node.
533+
Break(String),
534+
/// A genuine failure. Surfaces as a failed instance.
535+
Failure(String),
536+
}
537+
538+
pub type NodeResult = Result<String, NodeError>;
539+
540+
// Any `?` on an existing Result<_, String> auto-converts the error to Failure, so
541+
// activity calls and helpers need no per-call changes.
542+
impl From<String> for NodeError {
543+
fn from(e: String) -> Self {
544+
NodeError::Failure(e)
545+
}
546+
}
547+
```
548+
549+
`execute_loop_node` is the only handler that catches `NodeError::Break` (turning it into the
550+
loop's `Ok` result); `NodeError::Failure` keeps propagating. The orchestration boundary
551+
functions (`execute` / `execute_subtree`) still return `Result<String, String>` because they
552+
are registered with duroxide:
553+
554+
- `execute`: an uncaught top-level `Break` becomes a clear `Err` ("df.break() was called
555+
outside of a loop"), so the instance fails instead of completing with a sentinel value.
556+
- `execute_subtree` (used by JOIN/RACE branches): a `Break` is carried out-of-band in the
557+
subtree envelope's `control` field and re-raised as `NodeError::Break` by
558+
`parse_subtree_envelope` in the parent orchestration.
559+
524560
The orchestration walks the graph recursively:
525561

526562
```rust
@@ -559,7 +595,7 @@ async fn execute_function_node_with_vars(
559595
node_id: &str,
560596
results: &mut HashMap<String, String>,
561597
exec_ctx: &ExecutionContext,
562-
) -> Result<String, String> {
598+
) -> NodeResult {
563599
let node = graph.nodes.get(node_id).ok_or("Node not found")?;
564600

565601
ctx.trace_info(format!("Executing node {} (type: {})", node_id, node.node_type));
@@ -575,7 +611,7 @@ async fn execute_function_node_with_vars(
575611
"HTTP" => execute_http_node(ctx, node, results, exec_ctx).await?,
576612
"SIGNAL" => execute_signal_node(ctx, node).await?,
577613
"BREAK" => execute_break_node(ctx, node, node_id).await?,
578-
_ => return Err(format!("Unknown node type: {}", node.node_type)),
614+
other => return Err(NodeError::Failure(format!("Unknown node type: {other}"))),
579615
};
580616

581617
// Store named results for $variable substitution
@@ -754,20 +790,21 @@ async fn execute_loop_node(
754790
node: &FunctionNode,
755791
results: &mut HashMap<String, String>,
756792
exec_ctx: &ExecutionContext,
757-
) -> Result<String, String> {
793+
) -> NodeResult {
758794
let body_id = node.left_node.as_ref().ok_or("LOOP missing body")?;
759795

760-
// Execute loop body
761-
let body_result = execute_function_node_with_vars(
796+
// Execute loop body. A df.break() anywhere in the body (including inside nested
797+
// IF/JOIN/RACE) surfaces here as Err(NodeError::Break); the loop is the only node
798+
// that catches it. NodeError::Failure keeps propagating out via `?`.
799+
let body_result = match execute_function_node_with_vars(
762800
ctx, graph, body_id, results, exec_ctx
763-
).await?;
764-
765-
// Check for BREAK sentinel
766-
if is_break_result(&body_result) {
767-
return Ok(extract_break_value(&body_result));
768-
}
801+
).await {
802+
Ok(v) => v,
803+
Err(NodeError::Break(break_value)) => return Ok(break_value), // exit the loop
804+
Err(e @ NodeError::Failure(_)) => return Err(e),
805+
};
769806

770-
// Check while-condition if present
807+
// Check while-condition if present (conditions are SQL and cannot break)
771808
if let Some(condition_node_id) = get_condition_node(node) {
772809
let condition_result = execute_function_node_with_vars(
773810
ctx, graph, &condition_node_id, results, exec_ctx
@@ -790,7 +827,7 @@ async fn execute_loop_node(
790827
.continue_as_new(serde_json::to_string(&new_input)?)
791828
.await
792829
.map(|_| body_result)
793-
.map_err(|e| format!("continue_as_new failed: {:?}", e));
830+
.map_err(|e| NodeError::Failure(format!("continue_as_new failed: {:?}", e)));
794831
}
795832
```
796833

0 commit comments

Comments
 (0)