Sfoglia il codice sorgente

feat: retry with backoff in the HTTP helper, and on the RSS reader

A scheduled 35photo2anime pass died on "HTTP request failed: Timeout was
reached" while fetching the feed. Nothing was wrong with the workflow - the
feed was briefly unreachable, and one unlucky request threw the whole run
away.

Retrying belongs in smartbotic.http.request rather than in each node: half a
dozen nodes already carry their own retry loop, each slightly different, and
the ones that do not are one bad minute from the same failure. The helper now
takes retries, retryDelayMs, retryMaxDelayMs and retryOnStatus, doubling the
wait each attempt up to the cap.

What counts as worth retrying is deliberate. A timeout, a refused connection
or a name that would not resolve are the network being briefly unwell; 408,
425, 429, 500, 502, 503 and 504 are the far end being briefly unwell. A 404
or a 401 is an answer, and repeating the question does not change it. A
Retry-After from the server wins over the backoff, capped - being told to
wait and then not waiting is how a rate limit becomes a ban.

Off unless asked for. A POST that timed out may already have been received,
and a helper that quietly gained the ability to repeat it would do the thing
twice.

The RSS reader now retries twice by default, which is the case that started
this - reading a feed is safe to repeat, and a feed that blinks should not end
a scheduled run. The HTTP Request node gains the same three settings, at 0.

New fixture: one attempt against an unreachable address takes ~1s and says
"after 1 attempt(s)"; the same with retries 2 takes ~6s - 1s, wait 1s, 1s,
wait 2s, 1s - and says "after 3 attempt(s)", so both the count and the
doubling are pinned. 61/61.
fszontagh 1 mese fa
parent
commit
b0c7927c10

+ 25 - 0
docs/nodes.md

@@ -757,3 +757,28 @@ credential, because the shape is the same.
 
 The base URL is a setting, so any of these nodes also reaches a proxy, a
 gateway or a self-hosted service that speaks the same API.
+
+## Retrying an HTTP request
+
+`smartbotic.http.request` takes retry options, so a node does not need its own
+loop and its own sleep:
+
+```javascript
+smartbotic.http.request({
+  method: 'GET', url: feedUrl, timeout: 30000,
+  retries: 2,               // extra attempts, 0 (off) unless asked for
+  retryDelayMs: 2000,       // doubles each attempt
+  retryMaxDelayMs: 30000,   // and is capped here
+  retryOnStatus: [429, 503] // defaults to 408, 425, 429, 500, 502, 503, 504
+});
+```
+
+Retried: a timeout, a refused connection, a name that would not resolve, and
+the statuses above. Not retried: a 404 or a 401 - those are answers, and asking
+again does not change them. A `Retry-After` header from the server wins over the
+backoff, capped at `retryMaxDelayMs`, because being told to wait and then not
+waiting is how a rate limit becomes a ban.
+
+It is off by default on purpose. A POST that timed out may already have been
+received at the far end, and repeating it would do the thing twice - so the
+caller opts in where repeating is safe.

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

@@ -57,6 +57,24 @@ const configSchema = {
       title: 'Timeout (ms)',
       default: 30000
     },
+    retries: {
+      type: 'number',
+      title: 'Retries',
+      default: 0,
+      description: 'Extra attempts when the request times out or answers 408, 425, 429, 500, 502, 503 or 504. Off by default: a POST that timed out may already have been received at the far end, and repeating it would do the thing twice'
+    },
+    retryDelayMs: {
+      type: 'number',
+      title: 'Retry Delay (ms)',
+      default: 1000,
+      description: 'Wait before the first retry. Doubles on each further attempt, and a Retry-After from the server wins over both'
+    },
+    retryMaxDelayMs: {
+      type: 'number',
+      title: 'Max Retry Delay (ms)',
+      default: 30000,
+      description: 'Upper bound for the backoff'
+    },
     followRedirects: {
       type: 'boolean',
       title: 'Follow Redirects',
@@ -278,7 +296,12 @@ async function execute(config, input, context) {
       headers: headers,
       body: body,
       timeout: config.timeout,
-      followRedirects: config.followRedirects
+      followRedirects: config.followRedirects,
+      // Retrying is off unless asked for, and the waiting is done by the HTTP
+      // helper rather than a loop here.
+      retries: Number(config.retries) || 0,
+      retryDelayMs: Number(config.retryDelayMs) || 1000,
+      retryMaxDelayMs: Number(config.retryMaxDelayMs) || 30000
     });
 
     const responseHeaders = response.headers || {};

+ 20 - 0
nodes/rss/rss-reader.js

@@ -42,6 +42,18 @@ const configSchema = {
             title: 'Timeout (ms)',
             default: 30000
         },
+        retries: {
+            type: 'number', title: 'Retries', default: 2,
+            description: 'Extra attempts when the feed times out or answers 429, 500, 502, 503 or 504. A feed that is briefly unreachable should not end a scheduled run'
+        },
+        retryDelayMs: {
+            type: 'number', title: 'Retry Delay (ms)', default: 2000,
+            description: 'Wait before the first retry. Doubles on each further attempt, and a Retry-After from the server wins over both'
+        },
+        retryMaxDelayMs: {
+            type: 'number', title: 'Max Retry Delay (ms)', default: 30000,
+            description: 'Upper bound for the backoff'
+        },
         detectNewItems: {
             type: 'boolean',
             title: 'Detect New Items',
@@ -716,6 +728,14 @@ async function execute(config, input) {
         method: 'GET',
         url: url,
         timeout: timeout,
+        // Reading a feed is safe to repeat, and a feed that times out once is
+        // the usual reason a scheduled run dies - the request is retried rather
+        // than the whole pass being thrown away. Handled by the HTTP helper, so
+        // the waiting happens without this node holding a loop.
+        retries: config.retries !== undefined && config.retries !== null && config.retries !== ''
+            ? Number(config.retries) : 2,
+        retryDelayMs: Number(config.retryDelayMs) || 2000,
+        retryMaxDelayMs: Number(config.retryMaxDelayMs) || 30000,
         headers: {
             'Accept': 'application/rss+xml, application/atom+xml, application/xml, text/xml, */*',
             'User-Agent': userAgent

+ 130 - 1
src/runner/engine/script_engine.cpp

@@ -925,6 +925,42 @@ static CurlResponse performHttpRequest(
     return response;
 }
 
+
+// Which failures are worth trying again. A timeout, a refused connection or a
+// name that would not resolve are the network being briefly unwell; a 500 or a
+// 503 is the far end being briefly unwell; a 429 is being told to slow down.
+// A 404 or a 401 is an answer, and repeating the question does not change it.
+static bool isRetryableStatus(long status, const std::vector<long>& extra) {
+    for (long code : extra) {
+        if (code == status) return true;
+    }
+    return false;
+}
+
+// How long to wait before the next attempt: double each time, capped, and
+// honour Retry-After when the server sends one - being told to wait and then
+// not waiting is how a rate limit becomes a ban.
+static long backoffFor(int attempt, long base_ms, long cap_ms, const nlohmann::json& headers) {
+    long delay = base_ms;
+    for (int i = 1; i < attempt && delay < cap_ms; ++i) {
+        delay *= 2;
+    }
+    if (delay > cap_ms) delay = cap_ms;
+
+    if (headers.is_object() && headers.contains("retry-after")) {
+        try {
+            // Seconds only. The HTTP-date form exists but is rare from APIs, and
+            // guessing wrong is worse than falling back to the backoff.
+            const long asked = std::stol(headers["retry-after"].get<std::string>()) * 1000;
+            if (asked > 0) {
+                delay = asked > cap_ms ? cap_ms : asked;
+            }
+        } catch (...) {
+        }
+    }
+    return delay;
+}
+
 // JavaScript HTTP request function
 static JSValue js_http_request(JSContext* ctx, JSValue this_val, int argc, JSValue* argv) {
     if (argc < 1 || !JS_IsObject(argv[0])) {
@@ -1130,9 +1166,102 @@ static JSValue js_http_request(JSContext* ctx, JSValue this_val, int argc, JSVal
     }
     JS_FreeValue(ctx, follow_val);
 
+    // Retry settings. Off unless asked for: a request that is not safe to repeat
+    // - a payment, a post - must not start retrying itself because a helper
+    // gained the ability. A node that wants it says so.
+    int retries = 0;
+    JSValue retries_val = JS_GetPropertyStr(ctx, options, "retries");
+    if (JS_IsNumber(retries_val)) {
+        double r;
+        JS_ToFloat64(ctx, &r, retries_val);
+        retries = static_cast<int>(r);
+        if (retries < 0) retries = 0;
+        if (retries > 20) retries = 20;  // a runaway loop guard, not a real limit
+    }
+    JS_FreeValue(ctx, retries_val);
+
+    long retry_delay_ms = 1000;
+    JSValue retry_delay_val = JS_GetPropertyStr(ctx, options, "retryDelayMs");
+    if (JS_IsNumber(retry_delay_val)) {
+        double d;
+        JS_ToFloat64(ctx, &d, retry_delay_val);
+        if (d > 0) retry_delay_ms = static_cast<long>(d);
+    }
+    JS_FreeValue(ctx, retry_delay_val);
+
+    long retry_max_delay_ms = 30000;
+    JSValue retry_max_val = JS_GetPropertyStr(ctx, options, "retryMaxDelayMs");
+    if (JS_IsNumber(retry_max_val)) {
+        double d;
+        JS_ToFloat64(ctx, &d, retry_max_val);
+        if (d > 0) retry_max_delay_ms = static_cast<long>(d);
+    }
+    JS_FreeValue(ctx, retry_max_val);
+
+    // Too many requests, and the five ways a server says "not now".
+    std::vector<long> retry_statuses = {408, 425, 429, 500, 502, 503, 504};
+    JSValue retry_status_val = JS_GetPropertyStr(ctx, options, "retryOnStatus");
+    if (JS_IsArray(ctx, retry_status_val)) {
+        retry_statuses.clear();
+        int64_t count = 0;
+        JSValue len_val = JS_GetPropertyStr(ctx, retry_status_val, "length");
+        JS_ToInt64(ctx, &count, len_val);
+        JS_FreeValue(ctx, len_val);
+        for (int64_t i = 0; i < count; ++i) {
+            JSValue entry = JS_GetPropertyUint32(ctx, retry_status_val, static_cast<uint32_t>(i));
+            if (JS_IsNumber(entry)) {
+                double code;
+                JS_ToFloat64(ctx, &code, entry);
+                retry_statuses.push_back(static_cast<long>(code));
+            }
+            JS_FreeValue(ctx, entry);
+        }
+    }
+    JS_FreeValue(ctx, retry_status_val);
+
     // Perform the request
     try {
-        CurlResponse response = performHttpRequest(method, url, headers, body, timeout_ms, follow_redirects);
+        CurlResponse response;
+        const int attempts = retries + 1;
+        std::string last_transport_error;
+
+        for (int attempt = 1; attempt <= attempts; ++attempt) {
+            bool retryable = false;
+            try {
+                response = performHttpRequest(method, url, headers, body, timeout_ms, follow_redirects);
+                last_transport_error.clear();
+                retryable = isRetryableStatus(response.status_code, retry_statuses);
+                if (!retryable) {
+                    break;
+                }
+            } catch (const std::exception& transport) {
+                // No response at all - timed out, refused, unresolvable. Worth
+                // another go; if this was the last one it is rethrown below.
+                last_transport_error = transport.what();
+                retryable = true;
+                response.status_code = 0;
+                response.body.clear();
+                response.headers.clear();
+            }
+
+            if (attempt == attempts) {
+                if (!last_transport_error.empty()) {
+                    throw std::runtime_error(last_transport_error + " (after " +
+                                             std::to_string(attempts) + " attempt(s))");
+                }
+                break;
+            }
+
+            const long wait_ms = backoffFor(attempt, retry_delay_ms, retry_max_delay_ms,
+                                            parseHeaders(response.headers));
+            LOG_WARN("http.request {} {} - {}, waiting {}ms then attempt {} of {}",
+                     method, url,
+                     last_transport_error.empty()
+                         ? ("HTTP " + std::to_string(response.status_code))
+                         : last_transport_error,
+                     wait_ms, attempt + 1, attempts);
+            std::this_thread::sleep_for(std::chrono::milliseconds(wait_ms));
+        }
 
         // Create response object
         JSValue result = JS_NewObject(ctx);

+ 20 - 0
tests/nodes/http-retry.json

@@ -0,0 +1,20 @@
+{
+  "name": "verify-http-retry",
+  "nodes": [
+    {"id": "n1", "name": "Trigger", "type": "click-trigger", "position": {"x": 0, "y": 0}, "config": {}},
+    {"id": "once", "name": "Without Retry", "type": "code", "position": {"x": 0, "y": 100},
+     "config": {"timeout": 60,
+       "code": "const t = Date.now(); let err = ''; try { smartbotic.http.request({ method: 'GET', url: 'http://10.255.255.1/nowhere', timeout: 1000 }); } catch (e) { err = String(e.message || e); } const elapsed = Date.now() - t; return { elapsed: elapsed, err: err, tookOneAttempt: elapsed < 2500 && err.indexOf('after 1 attempt') !== -1 };"}},
+    {"id": "thrice", "name": "With Retry", "type": "code", "position": {"x": 0, "y": 200},
+     "config": {"timeout": 60,
+       "code": "const t = Date.now(); let err = ''; try { smartbotic.http.request({ method: 'GET', url: 'http://10.255.255.1/nowhere', timeout: 1000, retries: 2, retryDelayMs: 1000, retryMaxDelayMs: 5000 }); } catch (e) { err = String(e.message || e); } const elapsed = Date.now() - t; return { elapsed: elapsed, err: err, tookThreeAttempts: err.indexOf('after 3 attempt') !== -1, waitedAndDoubled: elapsed >= 5500 && elapsed < 9000 };"}}
+  ],
+  "connections": [
+    {"sourceNodeId": "n1", "sourceOutput": "main", "targetNodeId": "once", "targetInput": "data"},
+    {"sourceNodeId": "once", "sourceOutput": "main", "targetNodeId": "thrice", "targetInput": "data"}
+  ],
+  "expect": {
+    "once": {"status": "completed", "output": {"result": {"tookOneAttempt": true}}},
+    "thrice": {"status": "completed", "output": {"result": {"tookThreeAttempts": true, "waitedAndDoubled": true}}}
+  }
+}