fix: classify gateway responses and add retry/backoff policy (closes #104)

README.md

@@ -484,6 +484,8 @@ Try the Web MCP without any setup:
 | `POLLING_TIMEOUT` | Timeout for web_data_* tools polling (seconds) | `600` | `300`, `1200` |
 | `BASE_TIMEOUT` | Request timeout for base tools in seconds (search & scrape) | No limit | `60`, `120` |
 | `BASE_MAX_RETRIES` | Max retries for base tools on transient errors (0-3) | `0` | `1`, `3` |
+| `BASE_BACKOFF_MS` | Starting backoff (ms) between retries (doubles each attempt, with jitter) | `500` | `250`, `1000` |
+| `MAX_BACKOFF_MS` | Upper cap (ms) on a single backoff wait | `30000` | `5000`, `10000` |
 | `GROUPS` | Comma-separated tool group IDs | - | `ecommerce,browser` |
 | `TOOLS` | Comma-separated individual tool names | - | `extract,scrape_as_html` |
 
@@ -492,6 +494,11 @@ Try the Web MCP without any setup:
 - Lower values (e.g., 300) will fail faster on slow data collections.
 - Higher values (e.g., 1200) allow more time for complex scraping tasks.
 
+**Retry and backoff (base tools):**
+- When `BASE_MAX_RETRIES` is above `0`, transient gateway failures (502/504/503/500/408, network resets, and 429 rate limits) are retried. Permanent outcomes (4xx client errors, 403/451 blocks, 3xx redirects) are never retried.
+- Each retry waits an exponential, jittered backoff that starts at `BASE_BACKOFF_MS` and doubles per attempt, capped at `MAX_BACKOFF_MS`. A server `Retry-After` header (when present) takes precedence, also capped at `MAX_BACKOFF_MS`.
+- This means a single call can now block for tens of seconds before it ultimately fails. With the defaults (`BASE_MAX_RETRIES` up to `3`, `MAX_BACKOFF_MS` of `30000`), the worst case adds up to roughly 90 seconds of backoff (three waits capped at 30s each) on top of the request timeouts. Lower `MAX_BACKOFF_MS` (and/or `BASE_MAX_RETRIES`) if you need calls to fail faster.
+
 ---
 
 ## 📚 Documentation

package.json

@@ -40,6 +40,7 @@
         "server.js",
         "search_utils.js",
         "search_dataset_schema.js",
+        "retry_utils.js",
         "browser_tools.js",
         "browser_session.js",
         "aria_snapshot_filter.js",

retry_utils.js

@@ -0,0 +1,245 @@
+'use strict'; /*jslint node:true es9:true*/
+
+// Pure, side-effect-free helpers for classifying Bright Data responses and
+// computing retry/backoff delays. Extracted so the retry policy is testable in
+// isolation and reusable across tools (search, scrape, batch, web_data).
+// See issue #104 (intermittent 502/504 from the gateway, no retry guidance).
+
+// Outcome taxonomy returned by classify_response. Each value tells the caller
+// what to do next, independent of any particular transport library.
+export const OUTCOME = {
+    SUCCESS: 'success',         // 2xx - use the body
+    REDIRECT: 'redirect',       // 3xx - follow the Location (not an error)
+    RETRYABLE: 'retryable',     // transient - safe to retry after a backoff
+    RATE_LIMITED: 'rate_limited', // 429 - retry, but honor Retry-After if given
+    BLOCKED: 'blocked',         // target actively blocked us (403/451) - terminal
+    CLIENT_ERROR: 'client_error', // 4xx caller mistake - terminal, do not retry
+    FATAL: 'fatal',             // unexpected/unclassifiable - terminal
+};
+
+// Gateway/transport statuses that are safe to retry. 502/504 are the exact
+// symptoms reported in issue #104; 408/425 are slow/early-data conditions and
+// 500/503 are transient server states.
+const RETRYABLE_STATUS = new Set([408, 425, 500, 502, 503, 504]);
+
+// Statuses that mean "the target refused us"; retrying the same request will
+// not help, so we surface them as a first-class BLOCKED outcome rather than
+// burning retries or discarding the signal.
+const BLOCKED_STATUS = new Set([403, 451]);
+
+// Node/undici/axios network error codes with no HTTP status attached. These are
+// transient connectivity failures and are safe to retry.
+const RETRYABLE_NETWORK_CODES = new Set([
+    'ECONNRESET',
+    'ECONNREFUSED',
+    'ECONNABORTED',
+    'ETIMEDOUT',
+    'EAI_AGAIN',
+    'EPIPE',
+    'ENETUNREACH',
+    'ENETRESET',
+    'EHOSTUNREACH',
+    'UND_ERR_CONNECT_TIMEOUT',
+    'UND_ERR_HEADERS_TIMEOUT',
+    'UND_ERR_SOCKET',
+]);
+
+function is_finite_number(value){
+    return typeof value=='number' && Number.isFinite(value);
+}
+
+// An HTTP-date per RFC 9110: IMF-fixdate ("Sun, 06 Nov 1994 08:49:37 GMT"),
+// rfc850-date ("Sunday, 06-Nov-94 08:49:37 GMT"), or asctime
+// ("Sun Nov  6 08:49:37 1994"). We require a recognizable day-name prefix so a
+// bare number-like string ('1.5', '-3') is NEVER fed to the permissive
+// Date.parse (which would read it as a past date and clamp to an immediate retry).
+const HTTP_DATE_RE =
+    /^(mon|tue|wed|thu|fri|sat|sun)[a-z]*[,\s]/i;
+
+// Parse a Retry-After header value (RFC 9110). It is either a non-negative
+// integer number of seconds or an HTTP-date. Returns milliseconds, or null if
+// absent/malformed/in the past. A malformed value (fractional '1.5', negative
+// '-3', junk) returns null so the caller falls back to its computed backoff
+// rather than retrying immediately. `now_ms` is injectable for deterministic tests.
+export function parse_retry_after(value, now_ms = Date.now()){
+    if (value===undefined || value===null)
+        return null;
+    const raw = String(value).trim();
+    if (!raw)
+        return null;
+    // (a) a non-negative integer number of seconds.
+    if (/^\d+$/.test(raw))
+    {
+        const seconds = parseInt(raw, 10);
+        return seconds * 1000;
+    }
+    // (b) a valid HTTP-date. Reject anything that is not date-shaped up front so
+    // permissive Date.parse never silently accepts numeric junk as a past date.
+    if (!HTTP_DATE_RE.test(raw))
+        return null;
+    const when = Date.parse(raw);
+    if (Number.isNaN(when))
+        return null;
+    const delta = when - now_ms;
+    return delta > 0 ? delta : 0;
+}
+
+// Read a header case-insensitively from a plain object. Bright Data / undici may
+// return header names in any case, so we never assume a fixed casing.
+function get_header(headers, name){
+    if (!headers || typeof headers!='object')
+        return undefined;
+    const target = name.toLowerCase();
+    for (const key of Object.keys(headers))
+    {
+        if (key.toLowerCase()===target)
+            return headers[key];
+    }
+    return undefined;
+}
+
+// Classify a response or thrown error into a stable OUTCOME plus the metadata a
+// retry loop needs. Accepts a normalized shape so it never depends on axios:
+//   {status, headers}            - a completed HTTP response, or
+//   {error: {code, response}}    - a thrown transport error (axios-style).
+// Returns {outcome, status|null, retry_after_ms|null, retryable, reason}.
+export function classify_response(input, now_ms = Date.now()){
+    const obj = input && typeof input=='object' ? input : {};
+    const err = obj.error && typeof obj.error=='object' ? obj.error : null;
+
+    // A thrown error may carry an HTTP response (server replied with a status)
+    // or only a network code (connection never completed).
+    const response = err ? err.response : obj;
+    const status = response && is_finite_number(response.status)
+        ? response.status : null;
+    const headers = response ? response.headers : undefined;
+    const retry_after_ms = parse_retry_after(get_header(headers, 'retry-after'),
+        now_ms);
+
+    if (status===null)
+    {
+        const code = err && typeof err.code=='string' ? err.code : null;
+        if (code && RETRYABLE_NETWORK_CODES.has(code))
+        {
+            return {outcome: OUTCOME.RETRYABLE, status: null,
+                retry_after_ms: null, retryable: true,
+                reason: `network error ${code}`};
+        }
+        return {outcome: OUTCOME.FATAL, status: null, retry_after_ms: null,
+            retryable: false,
+            reason: code ? `unhandled network error ${code}`
+                : 'no status and no network code'};
+    }
+
+    if (status>=200 && status<300)
+    {
+        return {outcome: OUTCOME.SUCCESS, status, retry_after_ms: null,
+            retryable: false, reason: `http ${status}`};
+    }
+
+    // 3xx: a redirect, not an error. axios follows these transparently, so one
+    // surfacing here is a terminal-for-this-call signal to follow/report. It is
+    // neither a retryable gateway error nor a hard fatal, hence its own outcome.
+    if (status>=300 && status<400)
+    {
+        return {outcome: OUTCOME.REDIRECT, status, retry_after_ms: null,
+            retryable: false, reason: `http ${status} redirect`};
+    }
+
+    if (status===429)
+    {
+        return {outcome: OUTCOME.RATE_LIMITED, status, retry_after_ms,
+            retryable: true, reason: 'http 429 rate limited'};
+    }
+
+    if (BLOCKED_STATUS.has(status))
+    {
+        return {outcome: OUTCOME.BLOCKED, status, retry_after_ms: null,
+            retryable: false, reason: `http ${status} target blocked request`};
+    }
+
+    if (RETRYABLE_STATUS.has(status))
+    {
+        return {outcome: OUTCOME.RETRYABLE, status, retry_after_ms,
+            retryable: true, reason: `http ${status} transient gateway error`};
+    }
+
+    if (status>=400 && status<500)
+    {
+        return {outcome: OUTCOME.CLIENT_ERROR, status, retry_after_ms: null,
+            retryable: false, reason: `http ${status} client error`};
+    }
+
+    // Any other 5xx we did not enumerate: treat as retryable (transient by
+    // nature) rather than fatal, but cap via the caller's max_retries.
+    if (status>=500)
+    {
+        return {outcome: OUTCOME.RETRYABLE, status, retry_after_ms,
+            retryable: true, reason: `http ${status} server error`};
+    }
+
+    return {outcome: OUTCOME.FATAL, status, retry_after_ms: null,
+        retryable: false, reason: `http ${status} unclassified`};
+}
+
+// Default exponential-backoff parameters. base_ms doubles each attempt up to
+// max_ms, then full jitter is applied so concurrent callers (issue #104's burst
+// of 50+ calls) do not retry in lockstep and re-overload the gateway.
+export const DEFAULT_BACKOFF = {
+    base_ms: 500,
+    max_ms: 30000,
+    factor: 2,
+    jitter: 'full',
+};
+
+// Compute the delay (ms) before retry `attempt` (0-indexed: attempt 0 is the
+// wait before the 2nd try). A server-supplied retry_after_ms always wins and is
+// clamped to max_ms. `rng` is injectable (defaults to Math.random) so jittered
+// delays are deterministic under test.
+export function compute_backoff(attempt, opts = {}, rng = Math.random){
+    const base_ms = is_finite_number(opts.base_ms) && opts.base_ms>=0
+        ? opts.base_ms : DEFAULT_BACKOFF.base_ms;
+    const max_ms = is_finite_number(opts.max_ms) && opts.max_ms>=0
+        ? opts.max_ms : DEFAULT_BACKOFF.max_ms;
+    const factor = is_finite_number(opts.factor) && opts.factor>=1
+        ? opts.factor : DEFAULT_BACKOFF.factor;
+    const jitter = opts.jitter===undefined ? DEFAULT_BACKOFF.jitter
+        : opts.jitter;
+
+    if (is_finite_number(opts.retry_after_ms) && opts.retry_after_ms>=0)
+        return Math.min(opts.retry_after_ms, max_ms);
+
+    const safe_attempt = is_finite_number(attempt) && attempt>0
+        ? Math.floor(attempt) : 0;
+    const exponential = base_ms * Math.pow(factor, safe_attempt);
+    const capped = Math.min(exponential, max_ms);
+
+    if (jitter==='none')
+        return capped;
+    if (jitter==='equal')
+    {
+        // AWS "equal jitter": half fixed, half random.
+        const half = capped / 2;
+        return Math.round(half + rng() * half);
+    }
+    // "full jitter" (default): uniform random in [0, capped].
+    return Math.round(rng() * capped);
+}
+
+// Decide whether to retry given a classification and how many attempts remain.
+// `attempt` is 0-indexed (0 = the first try just failed). Returns
+// {retry, delay_ms, classification} so a loop has everything it needs.
+export function should_retry(classification, attempt, max_retries,
+    opts = {}, rng = Math.random){
+    const safe_max = is_finite_number(max_retries) && max_retries>=0
+        ? Math.floor(max_retries) : 0;
+    if (!classification || !classification.retryable)
+        return {retry: false, delay_ms: 0, classification};
+    if (attempt>=safe_max)
+        return {retry: false, delay_ms: 0, classification};
+    const delay_ms = compute_backoff(attempt, {
+        ...opts,
+        retry_after_ms: classification.retry_after_ms ?? undefined,
+    }, rng);
+    return {retry: true, delay_ms, classification};
+}

server.js

@@ -9,6 +9,7 @@ import {GROUPS} from './tool_groups.js';
 import {parse_google_search_response} from './search_utils.js';
 import {dataset_id_schema, filter_schema, metadata_to_fields, FILTER_OPERATORS}
     from './search_dataset_schema.js';
+import {classify_response, should_retry} from './retry_utils.js';
 import {createRequire} from 'node:module';
 import {remark} from 'remark';
 import strip from 'strip-markdown';
@@ -21,8 +22,8 @@ const pro_mode = process.env.PRO_MODE === 'true';
 const polling_timeout = parseInt(process.env.POLLING_TIMEOUT || '600', 10);
 const base_timeout = process.env.BASE_TIMEOUT
     ? parseInt(process.env.BASE_TIMEOUT, 10) * 1000 : 0;
-const base_max_retries = Math.min(
-    parseInt(process.env.BASE_MAX_RETRIES || '0', 10), 3);
+const base_max_retries = Math.max(0,
+    Math.min(parseInt(process.env.BASE_MAX_RETRIES || '0', 10) || 0, 3));
 const pro_mode_tools = ['search_engine', 'scrape_as_markdown',
     'search_engine_batch', 'scrape_batch', 'discover'];
 const tool_groups = process.env.GROUPS ?
@@ -71,21 +72,53 @@ const rate_limit_config = parse_rate_limit(process.env.RATE_LIMIT);
 if (!api_token)
     throw new Error('Cannot run MCP server without API_TOKEN env');
 
+const sleep = ms=>new Promise(resolve=>setTimeout(resolve, ms));
+
+// Backoff knobs (overridable via env) used by base_request.
+// Issue #104: bursts of MCP calls hit intermittent 502/504 from the gateway and
+// there was no backoff guidance. We now classify each failure and retry only the
+// transient ones with exponential backoff + full jitter, honoring Retry-After.
+const backoff_opts = {
+    base_ms: parseInt(process.env.BASE_BACKOFF_MS || '500', 10),
+    max_ms: parseInt(process.env.MAX_BACKOFF_MS || '30000', 10),
+    factor: 2,
+    jitter: 'full',
+};
+
 async function base_request(config){
     let last_err;
+    let retries = 0;
+    let total_delay_ms = 0;
     for (let attempt = 0; attempt <= base_max_retries; attempt++)
     {
         try {
             return await axios({...config, timeout: base_timeout});
         } catch(e){
             last_err = e;
-            if (e.response?.status && e.response.status >= 400
-                && e.response.status < 500)
+            const classification = classify_response({error: e});
+            const decision = should_retry(classification, attempt,
+                base_max_retries, backoff_opts);
+            if (!decision.retry)
             {
+                // Give up. Emit one concise summary line per request (not one per
+                // attempt: under issue #104's burst of 50-100 calls x up to 3
+                // retries, per-attempt stderr logging floods the transport) and
+                // only when we actually spent at least one retry, so a first-try
+                // non-retryable failure (e.g. a 4xx) stays silent.
+                if (retries)
+                    console.error(`[base_request] gave up after ${retries} retr`
+                        +`${retries==1 ? 'y' : 'ies'} (${total_delay_ms}ms `
+                        +`backoff total, last: ${classification.reason})`);
                 throw e;
             }
+            retries++;
+            total_delay_ms += decision.delay_ms;
+            await sleep(decision.delay_ms);
         }
     }
+    // Unreachable: base_max_retries is clamped to [0,3], so the loop always runs
+    // at least once and either returns on success or throws on give-up. Kept as a
+    // final safeguard rather than falling off the end with an implicit undefined.
     throw last_err;
 }