Pārlūkot izejas kodu

fix: an API that refuses the request now fails the node

The HTTP node computed `ok` from the status, logged a warning when it was
false, and returned normally. So a workflow that posted something and was
told no reported success, and a run that created nothing looked exactly
like a run that created everything.

This is not hypothetical. The tag flow being built alongside this ran
green through several rounds: every call was a completed node, the
sub-workflow reported ok, and nothing existed on the other end. The
server had been saying "slug is required" and "FOREIGN KEY constraint
failed" the whole time, into a log nobody was reading.

A 4xx or 5xx now stops the node, and the message carries what the server
actually said - the error, the message, the details, and each
validation_errors field with its own text. "HTTP 422" sends somebody to
the logs. "slug: A tag slug is required" is the answer itself.

Off by default would have been the safer-looking choice and the wrong
one: the whole failure is that nobody sees the quiet case. It is a
setting, so branching on a status yourself is still possible - the output
keeps ok, statusCode and body when the check is off.

Every http-request node in this installation is one where failing is
right: image downloads sit in loops that already continue on error, and
the blog nodes should never have carried on regardless. Checked, not
assumed.

The test's control is the point: the same refusal with the check off must
still complete, or this would have taken away a real thing to want. It
asks this server's own API without a token, which answers 401 every time
- the external status service I reached for first answered 503 when it
felt like it, failing the case for a reason unrelated to what it tests.

70 passed, 0 failed.
fszontagh 1 mēnesi atpakaļ
vecāks
revīzija
23fe52238b
2 mainītis faili ar 123 papildinājumiem un 1 dzēšanām
  1. 44 1
      nodes/core/http-request.js
  2. 79 0
      tests/nodes/http-error-status-fails.json

+ 44 - 1
nodes/core/http-request.js

@@ -125,6 +125,14 @@ const configSchema = {
       title: 'Follow Redirects',
       default: true
     },
+    failOnErrorStatus: {
+      type: 'boolean',
+      title: 'Fail on error status',
+      description: 'Stop the node when the server answers 4xx or 5xx, with what it said as the error. ' +
+                   'Turn this off only to branch on the status yourself - the output still carries ' +
+                   'ok, statusCode and body.',
+      default: true
+    },
     responseMode: {
       type: 'string',
       title: 'Response Mode',
@@ -525,8 +533,43 @@ async function execute(config, input, context) {
       result.body = response.data;
     }
 
+    // An error status fails the node.
+    //
+    // It used to return normally with ok:false and a warning in the log, so a
+    // workflow that posted to an API and was refused reported success. A run
+    // that created nothing looked exactly like a run that created everything -
+    // the tag flow here silently did nothing through several rounds of "it
+    // works", because every 422 was a completed node.
+    //
+    // The message carries what the server actually said. "HTTP 422" sends
+    // somebody to the logs; "slug is required" is the answer itself.
+    if (!result.ok && config.failOnErrorStatus !== false) {
+      let detail = '';
+      const body = result.body;
+      if (body && typeof body === 'object') {
+        const parts = [];
+        if (body.error) parts.push(String(body.error));
+        if (body.message && body.message !== body.error) parts.push(String(body.message));
+        if (body.details) parts.push(String(body.details));
+        if (body.validation_errors && typeof body.validation_errors === 'object') {
+          for (const field of Object.keys(body.validation_errors)) {
+            const msgs = body.validation_errors[field];
+            parts.push(field + ': ' + (Array.isArray(msgs) ? msgs.join(' ') : String(msgs)));
+          }
+        }
+        detail = parts.join(' - ') || JSON.stringify(body).slice(0, 300);
+      } else if (typeof body === 'string' && body) {
+        detail = body.slice(0, 300);
+      }
+      throw new Error(
+        `${config.method || 'GET'} ${url} returned ${response.status}` +
+        (detail ? ` - ${detail}` : '')
+      );
+    }
+
     if (!result.ok) {
-      smartbotic.log.warn(`HTTP request returned ${response.status}`);
+      smartbotic.log.warn(`HTTP request returned ${response.status}, continuing because ` +
+                          `"Fail on error status" is off`);
     }
 
     return result;

+ 79 - 0
tests/nodes/http-error-status-fails.json

@@ -0,0 +1,79 @@
+{
+  "name": "verify-http-error-status-fails",
+  "comment": "An API that refuses the request fails the node. It used to return normally with ok:false and a warning in the log, so a workflow that posted something and was told no reported success - a run that created nothing was indistinguishable from a run that created everything.\n\nThe second node is the control: the same refusal with the check turned off must still complete, because branching on a status yourself is a real thing to want, and this must not have taken it away.\n\nThe URL is this server's own API without a token, which answers 401 every time. An external status service answered 503 when it felt like it, failing the case for a reason unrelated to what it tests.",
+  "nodes": [
+    {
+      "id": "n1",
+      "name": "Trigger",
+      "type": "click-trigger",
+      "position": {
+        "x": 0,
+        "y": 0
+      },
+      "config": {}
+    },
+    {
+      "id": "refused",
+      "name": "Refused request",
+      "type": "http-request",
+      "position": {
+        "x": 0,
+        "y": 100
+      },
+      "config": {
+        "method": "GET",
+        "url": "http://localhost:8090/api/v1/workflows",
+        "responseMode": "text",
+        "timeout": 30000,
+        "retries": 0
+      }
+    },
+    {
+      "id": "tolerated",
+      "name": "Same refusal, tolerated",
+      "type": "http-request",
+      "position": {
+        "x": 0,
+        "y": 200
+      },
+      "config": {
+        "method": "GET",
+        "url": "http://localhost:8090/api/v1/workflows",
+        "responseMode": "text",
+        "timeout": 30000,
+        "retries": 0,
+        "failOnErrorStatus": false
+      }
+    }
+  ],
+  "connections": [
+    {
+      "sourceNodeId": "n1",
+      "sourceOutput": "main",
+      "targetNodeId": "refused",
+      "targetInput": "data"
+    },
+    {
+      "sourceNodeId": "n1",
+      "sourceOutput": "main",
+      "targetNodeId": "tolerated",
+      "targetInput": "data"
+    }
+  ],
+  "expect": {
+    "refused": {
+      "status": "failed",
+      "errorContains": "returned 401"
+    },
+    "tolerated": {
+      "status": "completed",
+      "output": {
+        "ok": false,
+        "statusCode": 401
+      }
+    }
+  },
+  "settings": {
+    "continueOnError": true
+  }
+}