Explorar o código

feat: let stop-and-error end a run without failing it

Why a deliberate stop was reported as an error: the node's only mechanism was
throw new Error(message) - the throw at <script>:38 in the reported stack - and
a thrown node is a failed node, so the execution was marked Failed. The workflow
did stop where it was told to; there was simply no way to stop without failing.

That is wrong for anything that polls. The Email OCR workflow runs its
imap-trigger every five minutes, and an empty mailbox is the normal case, not a
fault. Marking those runs failed fills the execution list with red and buries
the failures that actually matter.

The node gains a mode:

- error (default) throws exactly as before, so the error trigger keeps firing
  and existing workflows are unaffected.
- stop returns a _stop marker. The engine ends the walk, skips the remaining
  nodes, and finishes the execution as Completed with no error.

Engine support was needed because throwing is the only way a node can halt a
run on its own, and a throw is by definition a failure. _stop follows the same
shape as the existing _pause and _webhookResponse markers: the engine strips it
and stores what the node reports - stopped and reason - rather than the
mechanism that carried it.

Handled in BOTH walks. A stop raised inside a loop body ends the whole run
rather than just the iteration, since "stop the workflow" is a strange thing to
mean per-item. It travels out on ExecutionResult because executeLoopBody cannot
end the outer walk itself. This is the sixth feature that has had to be added to
executeLoopBody separately after the main walk - named ports, _webhookResponse,
_pause twice, config defaults, and now this - and every one of those was caught
by review rather than a test. The two walks should be collapsed.

When a run stops early, final_output comes from the stopping node. The previous
code read the last node in the execution order, which by definition never ran.

Verified: stop mode completes with the downstream node skipped; a stop inside a
loop completes with the node after the loop skipped; and the pre-existing
error-mode fixture still fails the run and still skips downstream, so the
default is unchanged. Full suite 45/45.
fszontagh hai 1 mes
pai
achega
da91f1548c

+ 29 - 5
nodes/core/stop-and-error.js

@@ -3,17 +3,24 @@
  * @name Stop and Error
  * @category flow-control
  * @version 1.0.0
- * @description Fail the workflow deliberately with a message, picked up by the error trigger
+ * @description End the workflow deliberately, either as a failure or as an ordinary stop
  * @icon octagon-x
  */
 
 const configSchema = {
     type: 'object',
     properties: {
+        mode: {
+            type: 'string',
+            title: 'Mode',
+            enum: ['error', 'stop'],
+            default: 'error',
+            description: 'error fails the run, which is what the error trigger reacts to and what shows red in the execution list. stop ends it as an ordinary completed run. Use stop for a condition that is expected, such as a scheduled poll finding nothing to do - marking those failed every few minutes buries the real failures'
+        },
         message: {
             type: 'string',
             title: 'Message',
-            description: 'The error text. Expressions are resolved, so it can carry values from the run',
+            description: 'The reason for ending the run. Expressions are resolved, so it can carry values from the run',
             default: 'Workflow stopped'
         }
     },
@@ -29,13 +36,30 @@ const inputSchema = {
 
 const outputSchema = {
     type: 'object',
-    properties: {}
+    properties: {
+        stopped: { type: 'boolean', description: 'True when the run was ended by this node in stop mode' },
+        reason: { type: 'string', description: 'The message the run was ended with' }
+    }
 };
 
 async function execute(config, input, context) {
     const message = config.message || 'Workflow stopped';
-    smartbotic.log.warn('Stop and Error: ' + message);
-    throw new Error(message);
+
+    // Default to failing. This node has always thrown, and a workflow relying
+    // on the error trigger firing must keep working after this option arrived.
+    if ((config.mode || 'error') !== 'stop') {
+        smartbotic.log.warn('Stop and Error: ' + message);
+        throw new Error(message);
+    }
+
+    // Throwing is the only way a node can halt a run by itself, and a throw is
+    // by definition a failure. Ending a run without failing needs the engine's
+    // cooperation, which is what this marker asks for: it stops the walk, skips
+    // the remaining nodes, and still finishes the execution as completed.
+    smartbotic.log.info('Stop: ' + message);
+    return {
+        _stop: { reason: message }
+    };
 }
 
 module.exports = { configSchema, inputSchema, outputSchema, execute };

+ 75 - 2
src/runner/workflow_engine.cpp

@@ -697,6 +697,30 @@ Result<ExecutionResult> WorkflowEngine::execute(const Workflow& workflow,
                 result.webhook_response = node_result.output["_webhookResponse"];
             }
 
+            // A node asking to stop ends the walk without failing the run. The
+            // distinction matters for anything that polls: a scheduled workflow
+            // that finds nothing to do has completed successfully, and marking
+            // it failed every five minutes buries real failures in noise.
+            if (node_result.status == NodeStatus::Completed &&
+                node_result.output.contains("_stop")) {
+                const auto& stop = node_result.output["_stop"];
+
+                result.stop_requested = true;
+                result.stopped_node_id = node_id;
+                result.stop_reason = stop.value("reason", "");
+
+                // The marker is engine plumbing. What the node reports is that
+                // it stopped and why, not the mechanism that carried it.
+                auto& stored_node = result.node_results[node_id];
+                stored_node.output.erase("_stop");
+                stored_node.output["stopped"] = true;
+                stored_node.output["reason"] = result.stop_reason;
+
+                LOG_INFO("Execution {} stopped at node {}: {}", result.execution_id, node_id,
+                         result.stop_reason.empty() ? "no reason given" : result.stop_reason);
+                break;
+            }
+
             // A node asking to pause ends this pass. The execution is stored as
             // Waiting with everything computed so far, and a later resume picks
             // it up from here rather than starting again.
@@ -773,6 +797,13 @@ Result<ExecutionResult> WorkflowEngine::execute(const Workflow& workflow,
                     break;
                 }
 
+                // A body node asked to stop the run. The loop already unwound
+                // its own iterations; this ends the outer walk too, so nodes
+                // after the loop do not run.
+                if (result.stop_requested) {
+                    break;
+                }
+
                 continue;  // Loop handles its own downstream execution
             }
 
@@ -791,8 +822,16 @@ Result<ExecutionResult> WorkflowEngine::execute(const Workflow& workflow,
         if (result.status == ExecutionStatus::Running) {
             result.status = ExecutionStatus::Completed;
 
-            // Get output from last executed node
-            if (!execution_order.empty()) {
+            if (result.stop_requested) {
+                // The last node in the order never ran - the walk ended early on
+                // purpose - so the useful final output is the node that stopped
+                // it, not an entry that will not be found.
+                auto it = result.node_results.find(result.stopped_node_id);
+                if (it != result.node_results.end()) {
+                    result.final_output = it->second.output;
+                }
+            } else if (!execution_order.empty()) {
+                // Get output from last executed node
                 auto it = result.node_results.find(execution_order.back());
                 if (it != result.node_results.end()) {
                     result.final_output = it->second.output;
@@ -1821,6 +1860,10 @@ bool WorkflowEngine::executeLoopBody(
         }
     };
 
+    // Set when a body node asks to stop the whole run, so both the body-node
+    // loop and the per-item loop can unwind.
+    bool stopped_in_body = false;
+
     // Execute body for each item
     for (size_t i = 0; i < ctx.items.size(); ++i) {
         ctx.current_index = i;
@@ -2092,6 +2135,30 @@ bool WorkflowEngine::executeLoopBody(
                 result.webhook_response = body_result.output["_webhookResponse"];
             }
 
+            // A stop raised inside a loop body ends the whole run, not just the
+            // iteration - "stop the workflow" would be a strange thing to mean
+            // per-item. The flag travels out on the result because this walk
+            // cannot end the outer one itself.
+            if (body_result.status == NodeStatus::Completed &&
+                body_result.output.contains("_stop")) {
+                const auto& stop = body_result.output["_stop"];
+
+                result.stop_requested = true;
+                result.stopped_node_id = body_node_id;
+                result.stop_reason = stop.value("reason", "");
+
+                auto& stored_body = result.node_results[result_key];
+                stored_body.output.erase("_stop");
+                stored_body.output["stopped"] = true;
+                stored_body.output["reason"] = result.stop_reason;
+
+                LOG_INFO("Execution {} stopped at node {} during loop iteration {}: {}",
+                         result.execution_id, body_node_id, i,
+                         result.stop_reason.empty() ? "no reason given" : result.stop_reason);
+                stopped_in_body = true;
+                break;
+            }
+
             if (callback) {
                 nlohmann::json event_data = {
                     {"executionId", result.execution_id},
@@ -2139,6 +2206,12 @@ bool WorkflowEngine::executeLoopBody(
         if (iteration_failed && !ctx.continue_on_error) {
             break;
         }
+
+        // A stop ends the remaining items too. Results collected so far are
+        // kept - they were produced before anyone asked to stop.
+        if (stopped_in_body) {
+            break;
+        }
     }
 
     // Copy results back (ctx was passed by const ref, but we used a mutable copy)

+ 12 - 0
src/runner/workflow_engine.hpp

@@ -108,6 +108,18 @@ struct ExecutionResult {
     std::string pause_token;           // Must be presented to resume
     int64_t pause_expires_at = 0;      // Milliseconds since the epoch, 0 for never
 
+    // A node asked to end the run early without failing it. The execution still
+    // finishes as Completed - this is an ordinary outcome, not an error - and
+    // these fields record that the remaining nodes were skipped deliberately
+    // rather than never reached.
+    //
+    // It lives on the result rather than being handled locally because a stop
+    // can be raised inside a loop body, which runs in a separate walk and has
+    // to hand the decision back to the caller.
+    bool stop_requested = false;
+    std::string stopped_node_id;
+    std::string stop_reason;
+
     nlohmann::json toJson() const;
 };
 

+ 22 - 0
tests/nodes/stop-and-error-stop-in-loop.json

@@ -0,0 +1,22 @@
+{
+  "name": "verify-stop-and-error-stop-in-loop",
+  "nodes": [
+    {"id": "n1", "name": "Trigger", "type": "click-trigger", "position": {"x": 0, "y": 0}, "config": {}},
+    {"id": "items", "name": "Items", "type": "code", "position": {"x": 0, "y": 100},
+     "config": {"code": "return { items: ['alpha', 'beta', 'gamma'] };"}},
+    {"id": "loop", "name": "Loop", "type": "loop", "position": {"x": 0, "y": 200},
+     "config": {"inputField": "data.result.items"}},
+    {"id": "stop", "name": "Stop", "type": "stop-and-error", "position": {"x": 0, "y": 300},
+     "config": {"mode": "stop", "message": "stopped from inside the loop"}},
+    {"id": "after", "name": "After", "type": "code", "position": {"x": 200, "y": 300},
+     "config": {"code": "return { marker: 'should not run' };"}}
+  ],
+  "connections": [
+    {"sourceNodeId": "n1", "sourceOutput": "main", "targetNodeId": "items", "targetInput": "data"},
+    {"sourceNodeId": "items", "sourceOutput": "main", "targetNodeId": "loop", "targetInput": "data"},
+    {"sourceNodeId": "loop", "sourceOutput": "loop", "targetNodeId": "stop", "targetInput": "data"},
+    {"sourceNodeId": "loop", "sourceOutput": "done", "targetNodeId": "after", "targetInput": "data"}
+  ],
+  "expectStatus": "completed",
+  "expectMissing": ["after"]
+}

+ 19 - 0
tests/nodes/stop-and-error-stop-mode.json

@@ -0,0 +1,19 @@
+{
+  "name": "verify-stop-and-error-stop-mode",
+  "nodes": [
+    {"id": "n1", "name": "Trigger", "type": "click-trigger", "position": {"x": 0, "y": 0}, "config": {}},
+    {"id": "n2", "name": "Stop", "type": "stop-and-error", "position": {"x": 0, "y": 100},
+     "config": {"mode": "stop", "message": "nothing to do this poll"}},
+    {"id": "n3", "name": "After", "type": "code", "position": {"x": 0, "y": 200},
+     "config": {"code": "return { marker: 'should not run' };"}}
+  ],
+  "connections": [
+    {"sourceNodeId": "n1", "sourceOutput": "main", "targetNodeId": "n2", "targetInput": "data"},
+    {"sourceNodeId": "n2", "sourceOutput": "main", "targetNodeId": "n3", "targetInput": "data"}
+  ],
+  "expectStatus": "completed",
+  "expect": {
+    "n2": {"status": "completed", "output": {"stopped": true, "reason": "nothing to do this poll"}}
+  },
+  "expectMissing": ["n3"]
+}