Преглед на файлове

fix: record a node's tolerated failure instead of discarding it

A node can swallow its own failure (e.g. RSS Reader's skipOnError) and
return normally so the run continues, but the reason it swallowed the
throw was discarded at the level the results panel and executions list
actually read - a run that tolerated a failure looked identical to one
where nothing went wrong.

Every node under nodes/ that reports outcome this way already follows
the same convention: success === false paired with a non-empty error
string, never success === false as ordinary, non-error data (checked
across storage-*, the *-chat nodes, nextcloud-talk, rss-reader). The
engine now reads that convention where it currently discards a
completed node's output, promotes the reason into the node's own
execution record error field (status stays completed, so continuation
is unaffected), and counts how many nodes did this per execution as a
new toleratedErrorCount field. The executions summary view picks up
the same field, matching how stopReason is already exposed there.

No node code changes needed, and no WebUI change either - the results
panel already renders a node's error field whenever it is non-empty,
regardless of status.
fszontagh преди 1 месец
родител
ревизия
c25e548875
променени са 4 файла, в които са добавени 108 реда и са изтрити 6 реда
  1. 37 0
      src/runner/workflow_engine.cpp
  2. 27 5
      src/webserver/webserver_service.cpp
  3. 8 1
      tests/nodes/rss-skip-on-error.json
  4. 36 0
      tests/nodes/tolerated-error-record.json

+ 37 - 0
src/runner/workflow_engine.cpp

@@ -268,6 +268,15 @@ nlohmann::json ExecutionResult::toJson() const {
                   return a->seq < b->seq;
               });
 
+    // A node that tolerated its own failure is still status Completed (the
+    // run went on) but carries a non-empty error (see the promotion above,
+    // where a node's execution record is built). Counted here so the
+    // execution as a whole says it fetched nothing *because something broke*,
+    // rather than looking like an ordinary day of completed nodes - a run
+    // that tolerated errors and a run that had nothing go wrong must not read
+    // the same way.
+    int tolerated_error_count = 0;
+
     j["nodeExecutions"] = nlohmann::json::array();
     for (const auto* result_ptr : ordered) {
         const auto& result = *result_ptr;
@@ -281,6 +290,9 @@ nlohmann::json ExecutionResult::toJson() const {
         nr["output"] = keep_full_outputs ? result.output : truncateLargeValues(result.output);
         nr["error"] = result.error;
         nr["retryCount"] = result.retry_count;
+        if (result.status == NodeStatus::Completed && !result.error.empty()) {
+            ++tolerated_error_count;
+        }
         // Absent for a node that did not run inside a loop. Present for one
         // that did - even a disabled one, recorded once for the whole loop
         // run rather than once per pass - naming the loop that owns it and,
@@ -297,6 +309,7 @@ nlohmann::json ExecutionResult::toJson() const {
         }
         j["nodeExecutions"].push_back(nr);
     }
+    j["toleratedErrorCount"] = tolerated_error_count;
 
     // Include workflow snapshot for pinning feature
     if (!workflow_snapshot.is_null()) {
@@ -1897,6 +1910,30 @@ NodeExecutionResult WorkflowEngine::executeNode(const WorkflowNode& node,
         if (script_result.success) {
             result.status = NodeStatus::Completed;
             result.output = script_result.output;
+            // A node can choose to swallow its own failure - e.g. RSS Reader's
+            // skipOnError - rather than throw, so one bad feed does not end a
+            // run with several. That is a decision about whether the run
+            // CONTINUES; it must not also decide whether the run's RECORD
+            // remembers what happened. Every node under nodes/ that reports
+            // outcome this way follows the same convention: success === false
+            // paired with a non-empty error string, never success === false as
+            // ordinary, non-error data (checked across nodes/ - storage-*,
+            // the *-chat nodes, nextcloud-talk, rss-reader). Promote that
+            // reason into the record's own `error` field, still under a
+            // Completed status so continuation is unchanged, so it survives
+            // exactly where the WebUI already renders a node's `error`
+            // regardless of status - and is not silently discarded here as it
+            // was before.
+            if (result.output.is_object()) {
+                auto succ_it = result.output.find("success");
+                auto err_it = result.output.find("error");
+                if (succ_it != result.output.end() && succ_it->is_boolean() &&
+                    succ_it->get<bool>() == false &&
+                    err_it != result.output.end() && err_it->is_string() &&
+                    !err_it->get<std::string>().empty()) {
+                    result.error = err_it->get<std::string>();
+                }
+            }
             LOG_DEBUG("Node {} completed successfully", node.id);
             break;
         } else {

+ 27 - 5
src/webserver/webserver_service.cpp

@@ -240,6 +240,10 @@ WebServerServiceConfig WebServerService::loadConfig(const std::filesystem::path&
         config.form_dispatch_threads = cfg.getOr<int>("server.form_dispatch_threads", 4);
         config.form_dispatch_queue_capacity =
             cfg.getOr<int>("server.form_dispatch_queue_capacity", 32);
+        config.workflow_dispatch_threads =
+            cfg.getOr<int>("server.workflow_dispatch_threads", 4);
+        config.workflow_dispatch_queue_capacity =
+            cfg.getOr<int>("server.workflow_dispatch_queue_capacity", 32);
 
         // Runner config
         config.runner_config.heartbeat_timeout_sec =
@@ -371,12 +375,16 @@ void WebServerService::stop() {
     LOG_INFO("Stopping WebServer service...");
 
     http_server_->stop();
-    // No new requests can arrive once the HTTP server is stopped, but the
-    // dispatch pool's own workers may still be mid-run - stop() joins them,
-    // so nothing is left running against a controller about to be destroyed.
+    // No new requests can arrive once the HTTP server is stopped, but each
+    // controller's own dispatch pool workers may still be mid-run - stop()
+    // joins them, so nothing is left running against a controller about to
+    // be destroyed.
     if (webhook_ctrl_) {
         webhook_ctrl_->stop();
     }
+    if (workflow_ctrl_) {
+        workflow_ctrl_->stop();
+    }
     ws_server_->stop();
     credential_server_->stop();
     node_sync_server_->stop();
@@ -423,9 +431,15 @@ void WebServerService::setupRoutes() {
         *storage_, *auth_middleware_, *access_, *auth_store_, *retention_);
     project_ctrl_->registerRoutes(server);
 
+    api::WorkflowController::DispatchConfig workflow_dispatch_config;
+    workflow_dispatch_config.threads = clampDispatchSetting(
+        "server.workflow_dispatch_threads", config_.workflow_dispatch_threads, 1, 64);
+    workflow_dispatch_config.queue_capacity = clampDispatchSetting(
+        "server.workflow_dispatch_queue_capacity", config_.workflow_dispatch_queue_capacity, 1, 10000);
+    workflow_dispatch_config.label_kind = "editor execute dispatch";
     workflow_ctrl_ = std::make_unique<api::WorkflowController>(
         *storage_, *auth_middleware_, *runner_registry_, *load_balancer_, *ws_server_,
-        *scheduler_, *db_watcher_, *access_, *node_store_, *retention_);
+        *scheduler_, *db_watcher_, *access_, *node_store_, *retention_, workflow_dispatch_config);
     workflow_ctrl_->registerRoutes(server);
 
     workflow_group_ctrl_ = std::make_unique<api::WorkflowGroupController>(
@@ -502,10 +516,18 @@ void WebServerService::ensureExecutionsSummaryView() {
     // A stopped run has status "completed" and an empty error, so without this
     // field the list cannot tell a run that finished its work from one that
     // deliberately ended early.
+    //
+    // toleratedErrorCount is the same problem one level down: a node can
+    // swallow its own failure (e.g. skipOnError) so the run keeps going, and
+    // the whole execution still ends up status "completed" with an empty
+    // top-level error. Without this field a run that quietly tolerated a
+    // failed feed is indistinguishable, in the list, from a run where nothing
+    // went wrong.
     auto result = storage_->createView(kExecutionsSummaryView, "executions",
                                        {"workflowId", "workflowName", "status", "triggerType",
                                         "startedAt", "finishedAt", "error", "runnerId",
-                                        "stopped", "stopReason", "stoppedNodeId"});
+                                        "stopped", "stopReason", "stoppedNodeId",
+                                        "toleratedErrorCount"});
     if (result.failed()) {
         LOG_ERROR("Could not create the executions summary view: {}. The executions "
                   "listing will fail until this is resolved.", result.error().message());

+ 8 - 1
tests/nodes/rss-skip-on-error.json

@@ -21,8 +21,15 @@
   ],
   "expectStatus": "completed",
   "expect": {
-    "dead": {"status": "completed", "output": {"success": false, "itemCount": 0}},
+    "dead": {
+      "status": "completed",
+      "errorContains": "HTTP request failed",
+      "output": {"success": false, "itemCount": 0}
+    },
     "merge": {"status": "completed", "output": {"count": 2}},
     "check": {"status": "completed", "output": {"result": {"survived": 2}}}
+  },
+  "expectExecution": {
+    "toleratedErrorCount": 1
   }
 }

+ 36 - 0
tests/nodes/tolerated-error-record.json

@@ -0,0 +1,36 @@
+{
+  "name": "verify-tolerated-error-record",
+  "comment": "A node can report success:false with a reason in its own output without ever throwing - storage-delete does this on a missing document, the same shape RSS Reader's skipOnError uses on an unreachable feed. The engine must promote that reason into the node's own execution record (status stays completed, so the run is unaffected, but error is no longer discarded) and count it at the execution level, so a run that tolerated a failure does not read the same as one where nothing went wrong. A plain success - deleting a document that is actually there - must not be counted or carry an error.",
+  "nodes": [
+    {"id": "n1", "name": "Trigger", "type": "click-trigger", "position": {"x": 0, "y": 0}, "config": {}},
+    {"id": "seed", "name": "Seed a document", "type": "storage-insert", "position": {"x": -150, "y": 100},
+     "config": {"collectionSource": "manual", "collectionManual": "@self",
+                "documentId": "tolerated-error-probe", "documentData": "{\"probe\": true}"}},
+    {"id": "deleteExisting", "name": "Delete it (plain success)", "type": "storage-delete", "position": {"x": -150, "y": 200},
+     "config": {"collectionSource": "manual", "collectionManual": "@self",
+                "documentId": "tolerated-error-probe"}},
+    {"id": "deleteMissing", "name": "Delete a document that is not there (tolerated)", "type": "storage-delete",
+     "position": {"x": 150, "y": 100},
+     "config": {"collectionSource": "manual", "collectionManual": "@self",
+                "documentId": "tolerated-error-probe-does-not-exist"}}
+  ],
+  "connections": [
+    {"sourceNodeId": "n1", "sourceOutput": "main", "targetNodeId": "seed", "targetInput": "data"},
+    {"sourceNodeId": "seed", "sourceOutput": "main", "targetNodeId": "deleteExisting", "targetInput": "data"},
+    {"sourceNodeId": "n1", "sourceOutput": "main", "targetNodeId": "deleteMissing", "targetInput": "data"}
+  ],
+  "expectStatus": "completed",
+  "expect": {
+    "deleteExisting": {
+      "status": "completed",
+      "output": {"success": true}
+    },
+    "deleteMissing": {
+      "status": "completed",
+      "output": {"success": false}
+    }
+  },
+  "expectExecution": {
+    "toleratedErrorCount": 1
+  }
+}