From 826589002da7b22501c463236fe1abb40bfc9349 Mon Sep 17 00:00:00 2001 From: Yashash Sheshagiri Date: Thu, 11 Jun 2026 14:37:36 +0530 Subject: [PATCH 1/2] fix: classify gateway responses and add retry/backoff policy (closes #104) Intermittent 502/504 from the MCP gateway under burst load had no retry guidance and the previous base_request retried every non-4xx in a tight loop with no delay, which could amplify the overload. Add retry_utils.js with pure, table-tested helpers: - classify_response: stable outcome taxonomy (success / redirect / retryable / rate_limited / blocked / client_error / fatal) over HTTP status and network error codes; 502/504/503/500/408 are retryable, 403/451 are a first-class BLOCKED outcome, 3xx are a non-retryable REDIRECT, 4xx are terminal. - parse_retry_after: strict RFC 9110 parsing (integer seconds or a date-shaped HTTP-date); fractional/negative/junk values return null so the caller falls back to computed backoff instead of an immediate retry. - compute_backoff: exponential backoff with full jitter, capped at max_ms, that honors a server Retry-After (seconds or HTTP-date). - should_retry: budget + delay decision for a retry loop. Wire base_request to use them so only transient failures are retried, with jittered backoff so a burst of concurrent calls no longer retries in lockstep. Backoff is configurable via BASE_BACKOFF_MS (default 500) and MAX_BACKOFF_MS (default 30000). Document the new env knobs and the worst-case added latency in the README. Reduce retry logging from one stderr line per attempt to a single concise summary line per request (only on final give-up), so the #104 burst (50-100 calls x up to 3 retries) no longer floods stderr. Clamp BASE_MAX_RETRIES to a sane 0-3 integer so a negative or non-numeric value behaves as 0 instead of skipping the request loop entirely and throwing undefined. --- README.md | 7 + package.json | 1 + retry_utils.js | 245 ++++++++++++++++++++++++ server.js | 41 +++- test/retry-utils.test.js | 397 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 687 insertions(+), 4 deletions(-) create mode 100644 retry_utils.js create mode 100644 test/retry-utils.test.js diff --git a/README.md b/README.md index 76c3576..4eec4e9 100644 --- a/README.md +++ b/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 diff --git a/package.json b/package.json index ec23edd..877d67e 100644 --- a/package.json +++ b/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", diff --git a/retry_utils.js b/retry_utils.js new file mode 100644 index 0000000..9858ba5 --- /dev/null +++ b/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}; +} diff --git a/server.js b/server.js index 19f85ac..42024af 100644 --- a/server.js +++ b/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; } diff --git a/test/retry-utils.test.js b/test/retry-utils.test.js new file mode 100644 index 0000000..f2ad52d --- /dev/null +++ b/test/retry-utils.test.js @@ -0,0 +1,397 @@ +'use strict'; /*jslint node:true es9:true*/ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { + OUTCOME, + classify_response, + compute_backoff, + parse_retry_after, + should_retry, +} from '../retry_utils.js'; + +// Fixed clock so HTTP-date Retry-After cases are deterministic. +const NOW = Date.parse('2026-01-19T20:51:08Z'); + +const classify_cases = [ + { + name: '200 is success, not retryable', + input: {status: 200, headers: {}}, + outcome: OUTCOME.SUCCESS, + retryable: false, + retry_after_ms: null, + }, + { + name: '204 is success', + input: {status: 204, headers: {}}, + outcome: OUTCOME.SUCCESS, + retryable: false, + }, + { + name: '502 from gateway is retryable (issue #104)', + input: {error: {response: {status: 502, headers: {}}}}, + outcome: OUTCOME.RETRYABLE, + retryable: true, + }, + { + name: '504 gateway timeout is retryable (issue #104)', + input: {error: {response: {status: 504, headers: {}}}}, + outcome: OUTCOME.RETRYABLE, + retryable: true, + }, + { + name: '500 is retryable', + input: {error: {response: {status: 500, headers: {}}}}, + outcome: OUTCOME.RETRYABLE, + retryable: true, + }, + { + name: '503 is retryable', + input: {error: {response: {status: 503, headers: {}}}}, + outcome: OUTCOME.RETRYABLE, + retryable: true, + }, + { + name: '408 request timeout is retryable', + input: {error: {response: {status: 408, headers: {}}}}, + outcome: OUTCOME.RETRYABLE, + retryable: true, + }, + { + name: 'unenumerated 5xx (599) falls back to retryable', + input: {error: {response: {status: 599, headers: {}}}}, + outcome: OUTCOME.RETRYABLE, + retryable: true, + }, + { + name: '301 is a redirect, not fatal (self-consistent outcome)', + input: {status: 301, headers: {location: 'https://shop.example/x'}}, + outcome: OUTCOME.REDIRECT, + retryable: false, + retry_after_ms: null, + }, + { + name: '302 is a redirect, never retried', + input: {error: {response: {status: 302, headers: {}}}}, + outcome: OUTCOME.REDIRECT, + retryable: false, + }, + { + name: '307 temporary redirect is a redirect outcome', + input: {status: 307, headers: {}}, + outcome: OUTCOME.REDIRECT, + retryable: false, + }, + { + name: '429 is rate_limited and retryable', + input: {error: {response: {status: 429, headers: {}}}}, + outcome: OUTCOME.RATE_LIMITED, + retryable: true, + }, + { + name: '429 surfaces numeric Retry-After in ms', + input: {error: {response: {status: 429, + headers: {'retry-after': '2'}}}}, + outcome: OUTCOME.RATE_LIMITED, + retryable: true, + retry_after_ms: 2000, + }, + { + name: '429 surfaces uppercase Retry-After header', + input: {error: {response: {status: 429, + headers: {'Retry-After': '5'}}}}, + outcome: OUTCOME.RATE_LIMITED, + retryable: true, + retry_after_ms: 5000, + }, + { + name: '503 surfaces HTTP-date Retry-After relative to now', + input: {error: {response: {status: 503, + headers: {'retry-after': 'Mon, 19 Jan 2026 20:51:18 GMT'}}}}, + outcome: OUTCOME.RETRYABLE, + retryable: true, + retry_after_ms: 10000, + }, + { + name: '403 is a first-class BLOCKED outcome, not a discarded error', + input: {error: {response: {status: 403, headers: {}}}}, + outcome: OUTCOME.BLOCKED, + retryable: false, + }, + { + name: '451 (unavailable for legal reasons) is BLOCKED', + input: {error: {response: {status: 451, headers: {}}}}, + outcome: OUTCOME.BLOCKED, + retryable: false, + }, + { + name: '400 is a terminal client error', + input: {error: {response: {status: 400, headers: {}}}}, + outcome: OUTCOME.CLIENT_ERROR, + retryable: false, + }, + { + name: '401 is a terminal client error', + input: {error: {response: {status: 401, headers: {}}}}, + outcome: OUTCOME.CLIENT_ERROR, + retryable: false, + }, + { + name: '404 is a terminal client error', + input: {error: {response: {status: 404, headers: {}}}}, + outcome: OUTCOME.CLIENT_ERROR, + retryable: false, + }, + { + name: 'ECONNRESET network error is retryable', + input: {error: {code: 'ECONNRESET'}}, + outcome: OUTCOME.RETRYABLE, + retryable: true, + }, + { + name: 'ETIMEDOUT network error is retryable', + input: {error: {code: 'ETIMEDOUT'}}, + outcome: OUTCOME.RETRYABLE, + retryable: true, + }, + { + name: 'undici connect timeout is retryable', + input: {error: {code: 'UND_ERR_CONNECT_TIMEOUT'}}, + outcome: OUTCOME.RETRYABLE, + retryable: true, + }, + { + name: 'unknown network code is fatal, never silently retried', + input: {error: {code: 'ESOMETHINGWEIRD'}}, + outcome: OUTCOME.FATAL, + retryable: false, + }, + { + name: 'no status and no code is fatal', + input: {error: {}}, + outcome: OUTCOME.FATAL, + retryable: false, + }, + { + name: 'non-object input is fatal, not a crash', + input: null, + outcome: OUTCOME.FATAL, + retryable: false, + }, +]; + +test('classify_response taxonomy (table-driven)', ()=>{ + for (const tc of classify_cases) + { + const got = classify_response(tc.input, NOW); + assert.equal(got.outcome, tc.outcome, + `${tc.name}: outcome ${got.outcome} != ${tc.outcome}`); + assert.equal(got.retryable, tc.retryable, + `${tc.name}: retryable ${got.retryable} != ${tc.retryable}`); + if (tc.retry_after_ms!==undefined) + { + assert.equal(got.retry_after_ms, tc.retry_after_ms, + `${tc.name}: retry_after_ms ${got.retry_after_ms} ` + +`!= ${tc.retry_after_ms}`); + } + assert.equal(typeof got.reason, 'string', + `${tc.name}: reason should be a string`); + } +}); + +const parse_retry_after_cases = [ + {name: 'undefined -> null', value: undefined, expected: null}, + {name: 'null -> null', value: null, expected: null}, + {name: 'empty string -> null', value: ' ', expected: null}, + {name: 'integer seconds -> ms', value: '3', expected: 3000}, + {name: 'large integer seconds -> ms', value: '120', expected: 120000}, + {name: 'zero seconds -> 0', value: '0', expected: 0}, + {name: 'garbage -> null', value: 'soon', expected: null}, + // Strictness: fractional/negative/number-like junk must be null (fall back to + // computed backoff), NOT 0 (an immediate retry via permissive Date.parse). + {name: 'fractional seconds -> null (not 0)', value: '1.5', expected: null}, + {name: 'negative seconds -> null (not 0)', value: '-3', expected: null}, + {name: 'leading-plus -> null', value: '+5', expected: null}, + {name: 'trailing junk -> null', value: '5s', expected: null}, + {name: 'numeric-with-space -> null', value: '5 ', expected: 5000}, + {name: 'date-shaped junk -> null', value: 'Mon, not a date', expected: null}, + { + name: 'future HTTP-date -> positive ms', + value: 'Mon, 19 Jan 2026 20:51:18 GMT', + expected: 10000, + }, + { + name: 'past HTTP-date clamps to 0', + value: 'Mon, 19 Jan 2026 20:51:00 GMT', + expected: 0, + }, +]; + +test('parse_retry_after (table-driven)', ()=>{ + for (const tc of parse_retry_after_cases) + { + const got = parse_retry_after(tc.value, NOW); + assert.equal(got, tc.expected, + `${tc.name}: got ${got} expected ${tc.expected}`); + } +}); + +const backoff_cases = [ + { + name: 'no jitter, attempt 0 -> base', + attempt: 0, + opts: {base_ms: 500, jitter: 'none'}, + expected: 500, + }, + { + name: 'no jitter, attempt 1 -> base*factor', + attempt: 1, + opts: {base_ms: 500, factor: 2, jitter: 'none'}, + expected: 1000, + }, + { + name: 'no jitter, attempt 3 -> base*factor^3', + attempt: 3, + opts: {base_ms: 500, factor: 2, jitter: 'none'}, + expected: 4000, + }, + { + name: 'no jitter caps at max_ms', + attempt: 10, + opts: {base_ms: 500, factor: 2, max_ms: 30000, jitter: 'none'}, + expected: 30000, + }, + { + name: 'retry_after_ms overrides exponential', + attempt: 5, + opts: {base_ms: 500, retry_after_ms: 2000, jitter: 'none'}, + expected: 2000, + }, + { + name: 'retry_after_ms is clamped to max_ms', + attempt: 0, + opts: {retry_after_ms: 120000, max_ms: 30000, jitter: 'none'}, + expected: 30000, + }, + { + name: 'full jitter with rng=0 -> 0', + attempt: 2, + opts: {base_ms: 500, jitter: 'full'}, + rng: ()=>0, + expected: 0, + }, + { + name: 'full jitter with rng=1 -> capped value', + attempt: 2, + opts: {base_ms: 500, factor: 2, jitter: 'full'}, + rng: ()=>1, + expected: 2000, + }, + { + name: 'equal jitter with rng=0 -> half capped', + attempt: 2, + opts: {base_ms: 500, factor: 2, jitter: 'equal'}, + rng: ()=>0, + expected: 1000, + }, + { + name: 'equal jitter with rng=1 -> full capped', + attempt: 2, + opts: {base_ms: 500, factor: 2, jitter: 'equal'}, + rng: ()=>1, + expected: 2000, + }, + { + name: 'negative attempt is floored to 0', + attempt: -3, + opts: {base_ms: 500, jitter: 'none'}, + expected: 500, + }, + { + name: 'defaults applied when opts empty', + attempt: 0, + opts: {jitter: 'none'}, + expected: 500, + }, +]; + +test('compute_backoff (table-driven)', ()=>{ + for (const tc of backoff_cases) + { + const rng = tc.rng || (()=>0.5); + const got = compute_backoff(tc.attempt, tc.opts, rng); + assert.equal(got, tc.expected, + `${tc.name}: got ${got} expected ${tc.expected}`); + } +}); + +test('full jitter stays within [0, capped] across the rng range', ()=>{ + const opts = {base_ms: 500, factor: 2, max_ms: 30000, jitter: 'full'}; + for (const r of [0, 0.01, 0.25, 0.5, 0.75, 0.99, 1]) + { + const delay = compute_backoff(3, opts, ()=>r); + assert.ok(delay>=0, `delay ${delay} should be >= 0`); + assert.ok(delay<=4000, `delay ${delay} should be <= capped 4000`); + } +}); + +const should_retry_cases = [ + { + name: 'retryable within budget -> retry', + classification: {retryable: true, retry_after_ms: null}, + attempt: 0, + max_retries: 3, + expect_retry: true, + }, + { + name: 'retryable but budget exhausted -> stop', + classification: {retryable: true, retry_after_ms: null}, + attempt: 3, + max_retries: 3, + expect_retry: false, + }, + { + name: 'non-retryable -> never retry', + classification: {retryable: false, retry_after_ms: null}, + attempt: 0, + max_retries: 3, + expect_retry: false, + }, + { + name: 'redirect classification -> never retried (no 3xx loop)', + classification: classify_response({status: 302, headers: {}}), + attempt: 0, + max_retries: 3, + expect_retry: false, + }, + { + name: 'missing classification -> never retry', + classification: null, + attempt: 0, + max_retries: 3, + expect_retry: false, + }, + { + name: 'retry honors classification retry_after_ms', + classification: {retryable: true, retry_after_ms: 2000}, + attempt: 0, + max_retries: 3, + opts: {jitter: 'none', max_ms: 30000}, + expect_retry: true, + expect_delay: 2000, + }, +]; + +test('should_retry budget + delay (table-driven)', ()=>{ + for (const tc of should_retry_cases) + { + const got = should_retry(tc.classification, tc.attempt, tc.max_retries, + tc.opts || {jitter: 'none'}, ()=>0.5); + assert.equal(got.retry, tc.expect_retry, + `${tc.name}: retry ${got.retry} != ${tc.expect_retry}`); + if (tc.expect_delay!==undefined) + { + assert.equal(got.delay_ms, tc.expect_delay, + `${tc.name}: delay ${got.delay_ms} != ${tc.expect_delay}`); + } + } +}); From cb34f4626620d914873d0a8aa540fe991248b980 Mon Sep 17 00:00:00 2001 From: Yashash Sheshagiri Date: Thu, 11 Jun 2026 23:32:24 +0530 Subject: [PATCH 2/2] fix: geo_fanout reads the target status via Unlocker format:json (not the gateway 200) The geo_fanout executor called the Unlocker /request with format:'raw' and then classified axios response.status, which is always the gateway's 200. A target 403/451/redirect was therefore misclassified as ok, defeating the whole point of the tool (surfacing geo-gating as a first-class classified result). Fix: call /request with format:'json'. The response body is the envelope {status_code, headers, body} where status_code is the TARGET's HTTP status and headers are the TARGET's response headers. A 3xx is surfaced WITHOUT the Unlocker following it, so a redirect keeps its Location header. We classify on that real target status. - geo_utils.js: add parse_unlocker_json(data) mapping {status_code, headers, body} to the {status, headers, body} shape build_geo_entry expects (tolerant of a missing/non-object/JSON-string input). build_geo_entry now carries the rendered body through and documents exit_ip as best-effort null (the json envelope has no gateway x-brd-* headers; we never fabricate an IP). - server.js: executor uses format:'json' + responseType:'json', parses with parse_unlocker_json, passes the target {status, headers} into build_geo_entry, and renders the body to markdown via the existing remark/strip pipeline when data_format is markdown (raw otherwise). Per-geo country targeting preserved. - tests: geo-utils fixtures now use the REAL Unlocker envelope shape and drive build_geo_entry end-to-end through parse_unlocker_json (403 -> blocked, 302 with a cross-host location -> redirected, 200 -> ok, 429 -> rate_limited, thrown transport error -> error); parse_unlocker_json gets its own table-driven test. The stdio registration test is unchanged. Based on PR #155 (retry/backoff for #104); retry_utils.js classification is reused. --- geo_utils.js | 198 ++++++++++++++++++++++++ server.js | 86 ++++++++++- test/geo-fanout-tool.test.js | 40 +++++ test/geo-utils.test.js | 292 +++++++++++++++++++++++++++++++++++ tool_groups.js | 1 + 5 files changed, 615 insertions(+), 2 deletions(-) create mode 100644 geo_utils.js create mode 100644 test/geo-fanout-tool.test.js create mode 100644 test/geo-utils.test.js diff --git a/geo_utils.js b/geo_utils.js new file mode 100644 index 0000000..f169bd8 --- /dev/null +++ b/geo_utils.js @@ -0,0 +1,198 @@ +'use strict'; /*jslint node:true es9:true*/ + +// Pure helpers for the geo_fanout tool: validating the list of country exits +// and turning a set of per-geo results into a single structured report where a +// blocked / redirected / failed geo is a FIRST-CLASS classified result, not a +// discarded error. No network or transport dependencies live here so the fanout +// aggregation logic is unit-testable in isolation. + +import {classify_response, OUTCOME} from './retry_utils.js'; + +// Per-geo status the caller sees in the aggregated report. +export const GEO_STATUS = { + OK: 'ok', // fetched successfully + BLOCKED: 'blocked', // target refused this geo (403/451) + REDIRECTED: 'redirected', // 3xx to a different host/path (geo gating signal) + RATE_LIMITED: 'rate_limited', + ERROR: 'error', // transient/fatal failure after retries +}; + +// Map the Unlocker's JSON envelope to the shape build_geo_entry expects. +// When the /request endpoint is called with format:'json', the gateway always +// answers HTTP 200 and the TARGET's real result lives in the JSON body: +// {status_code, headers, body} +// where status_code is the target's HTTP status and headers are the target's +// response headers (lowercased keys). The previous format:'raw' path only ever +// exposed the gateway's 200, so a target 403/451/3xx was misclassified as ok. +// This helper isolates that mapping so the real envelope shape is unit-testable. +// Tolerant of a missing / non-object / string-but-JSON input: a value with no +// recognizable status yields {status:null} and classify_response then treats it +// as a transport-level failure rather than a silent success. +export function parse_unlocker_json(data){ + let obj = data; + if (typeof obj=='string') + { + try { obj = JSON.parse(obj); } + catch(e){ obj = null; } + } + if (!obj || typeof obj!='object') + return {status: null, headers: undefined, body: undefined}; + const status = typeof obj.status_code=='number' + && Number.isFinite(obj.status_code) ? obj.status_code : null; + const headers = obj.headers && typeof obj.headers=='object' + ? obj.headers : undefined; + return {status, headers, body: obj.body}; +} + +// Validate and normalize a list of 2-letter ISO country codes. Lowercases, +// trims, dedupes (preserving first-seen order), and rejects malformed entries +// loudly so a typo never becomes a silently-dropped exit. Returns the clean +// array; throws Error on any invalid code (callers want fast, explicit failure). +export function normalize_geos(geos){ + if (!Array.isArray(geos) || geos.length===0) + throw new Error('geos must be a non-empty array of 2-letter codes'); + const seen = new Set(); + const out = []; + for (const raw of geos) + { + if (typeof raw!='string') + throw new Error(`invalid geo (not a string): ${JSON.stringify(raw)}`); + const code = raw.trim().toLowerCase(); + if (!/^[a-z]{2}$/.test(code)) + throw new Error(`invalid geo country code: "${raw}" ` + +`(expected 2 letters, e.g. "us", "de")`); + if (seen.has(code)) + continue; + seen.add(code); + out.push(code); + } + return out; +} + +// Detect whether a successful-looking response is actually a geo redirect. +// A 3xx with a Location header pointing at a different host is the classic +// "Belgian visitor bounced to a different storefront" signal we must capture. +function detect_redirect(status, headers, request_url){ + if (!(status>=300 && status<400)) + return null; + const location = read_header(headers, 'location'); + if (!location) + return {redirected: true, location: null, cross_host: false}; + let cross_host = false; + try { + const from = new URL(request_url); + const to = new URL(location, request_url); + cross_host = from.host!==to.host; + } catch(e){ + cross_host = false; + } + return {redirected: true, location, cross_host}; +} + +function read_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; +} + +// Map one classified outcome to the per-geo report status. +function outcome_to_geo_status(outcome){ + switch (outcome) + { + case OUTCOME.SUCCESS: return GEO_STATUS.OK; + case OUTCOME.REDIRECT: return GEO_STATUS.REDIRECTED; + case OUTCOME.BLOCKED: return GEO_STATUS.BLOCKED; + case OUTCOME.RATE_LIMITED: return GEO_STATUS.RATE_LIMITED; + default: return GEO_STATUS.ERROR; + } +} + +// Build the per-geo entry for a single settled attempt. `attempt` is the +// normalized shape produced by the tool's executor: +// {geo, url, response:{status, headers, exit_ip?}, body?} on a completed +// request (status/headers are the TARGET's, parsed from the Unlocker +// format:'json' envelope, NOT the gateway's 200), or +// {geo, url, error:{code?, response?}} on a thrown failure. +// `now_ms` is injectable for deterministic Retry-After math under test. +export function build_geo_entry(attempt, now_ms = Date.now()){ + const obj = attempt && typeof attempt=='object' ? attempt : {}; + const geo = typeof obj.geo=='string' ? obj.geo : 'unknown'; + const url = typeof obj.url=='string' ? obj.url : null; + // Normalize to the shape classify_response understands: a thrown error keeps + // its {error} envelope; a completed request passes its {status, headers}. + const classify_input = obj.error ? {error: obj.error} + : (obj.response || {}); + const classification = classify_response(classify_input, now_ms); + const status = classification.status; + const headers = obj.response ? obj.response.headers + : (obj.error && obj.error.response + ? obj.error.response.headers : undefined); + // exit_ip is best-effort: the Unlocker format:'json' envelope carries the + // target's headers, not the gateway's x-brd-* headers, so the exit IP is not + // observable on this path. We surface whatever the executor explicitly + // supplied (none, today) and otherwise null; we never fabricate an IP. + const exit_ip = obj.response + && typeof obj.response.exit_ip=='string' + ? obj.response.exit_ip : null; + + const redirect = detect_redirect(status, headers, url); + let geo_status = outcome_to_geo_status(classification.outcome); + if (redirect && redirect.redirected) + geo_status = GEO_STATUS.REDIRECTED; + + const entry = { + geo, + url, + status: geo_status, + http_status: status, + exit_ip, + outcome: classification.outcome, + retry_after_ms: classification.retry_after_ms, + reason: classification.reason, + redirect: redirect || null, + }; + // The rendered target body (markdown or raw) when the executor captured one. + // Kept out of the entry entirely when absent so an error/blocked geo is not + // padded with an empty string that reads like real content. + if (obj.body!==undefined) + entry.body = obj.body; + return entry; +} + +// Aggregate all per-geo entries into one report. Every geo appears exactly once +// regardless of success/failure; nothing is dropped. The summary makes the +// "this geo was blocked / redirected" fact queryable at a glance. +export function summarize_fanout(entries){ + const list = Array.isArray(entries) ? entries : []; + const summary = { + total: list.length, + ok: 0, + blocked: 0, + redirected: 0, + rate_limited: 0, + error: 0, + }; + for (const e of list) + { + switch (e.status) + { + case GEO_STATUS.OK: summary.ok++; break; + case GEO_STATUS.BLOCKED: summary.blocked++; break; + case GEO_STATUS.REDIRECTED: summary.redirected++; break; + case GEO_STATUS.RATE_LIMITED: summary.rate_limited++; break; + default: summary.error++; break; + } + } + return { + summary, + any_blocked: summary.blocked>0, + any_redirected: summary.redirected>0, + results: list, + }; +} diff --git a/server.js b/server.js index 42024af..f4e08c1 100644 --- a/server.js +++ b/server.js @@ -10,6 +10,8 @@ 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 {normalize_geos, parse_unlocker_json, build_geo_entry, summarize_fanout} + from './geo_utils.js'; import {createRequire} from 'node:module'; import {remark} from 'remark'; import strip from 'strip-markdown'; @@ -25,7 +27,7 @@ const base_timeout = process.env.BASE_TIMEOUT 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']; + 'search_engine_batch', 'scrape_batch', 'discover', 'geo_fanout']; const tool_groups = process.env.GROUPS ? process.env.GROUPS.split(',').map(g=>g.trim().toLowerCase()) .filter(Boolean) : []; @@ -74,7 +76,7 @@ if (!api_token) const sleep = ms=>new Promise(resolve=>setTimeout(resolve, ms)); -// Backoff knobs (overridable via env) used by base_request. +// Backoff knobs (overridable via env) used by base_request and geo_fanout. // 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. @@ -429,6 +431,86 @@ addTool({ }), }); +addTool({ + name: 'geo_fanout', + description: 'Fetch the SAME url from multiple country exits in parallel and ' + +'return one structured report. A geo that is blocked (403/451), ' + +'redirected (3xx to a different host), rate-limited (429) or fails ' + +'transiently becomes a FIRST-CLASS classified result, not a discarded ' + +'error. Ideal for detecting geo-gating, regional price/availability ' + +'differences, and access denial across countries.', + annotations: { + title: 'Geo Fanout', + readOnlyHint: true, + openWorldHint: true, + }, + parameters: z.object({ + url: z.string().url(), + countries: z.array(z.string().length(2)) + .min(1) + .max(10) + .describe('2-letter ISO country codes to fan the request across ' + +'(e.g., ["de", "be", "fr"]). Deduped; max 10.'), + data_format: z.enum(['raw', 'markdown']) + .optional() + .default('markdown') + .describe('Response body format per geo (default: markdown).'), + }), + execute: tool_fn('geo_fanout', async({url, countries, data_format}, ctx)=>{ + const geos = normalize_geos(countries); + const now = Date.now(); + const want_markdown = data_format=='markdown'; + const attempts = geos.map(geo=>(async()=>{ + try { + // format:'json' makes the Unlocker return a JSON envelope + // {status_code, headers, body} where status_code is the TARGET's + // HTTP status and headers are the TARGET's response headers. With + // the previous format:'raw' we only ever saw the gateway's 200, + // so a target 403/451/3xx was misclassified as ok. We classify on + // the real target status here. data_format still controls how the + // target body is rendered (markdown vs the raw body) inside the + // envelope; the 3xx is surfaced in the envelope WITHOUT the + // Unlocker following it, so a redirect keeps its Location header. + const response = await base_request({ + url: 'https://api.brightdata.com/request', + method: 'POST', + data: { + url, + zone: unlocker_zone, + format: 'json', + ...want_markdown ? {data_format: 'markdown'} : {}, + country: geo, + }, + headers: api_headers(ctx.clientName, 'geo_fanout'), + responseType: 'json', + }); + const parsed = parse_unlocker_json(response.data); + let body = parsed.body; + if (want_markdown && typeof body=='string' && body) + { + body = (await remark() + .use(strip, {keep: ['link', 'linkReference', 'code', + 'inlineCode']}) + .process(body)).value; + } + return build_geo_entry({ + geo, + url, + body, + response: { + status: parsed.status, + headers: parsed.headers, + }, + }, now); + } catch(e){ + return build_geo_entry({geo, url, error: e}, now); + } + })()); + const entries = await Promise.all(attempts); + return JSON.stringify(summarize_fanout(entries), null, 2); + }), +}); + addTool({ name: 'scrape_as_html', description: 'Scrape a single webpage URL with advanced options for ' diff --git a/test/geo-fanout-tool.test.js b/test/geo-fanout-tool.test.js new file mode 100644 index 0000000..5fe98b1 --- /dev/null +++ b/test/geo-fanout-tool.test.js @@ -0,0 +1,40 @@ +'use strict'; /*jslint node:true es9:true*/ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import {fileURLToPath} from 'node:url'; +import {dirname, resolve} from 'node:path'; +import {Client} from '@modelcontextprotocol/sdk/client/index.js'; +import {StdioClientTransport} from '@modelcontextprotocol/sdk/client/stdio.js'; + +const test_dir = dirname(fileURLToPath(import.meta.url)); +const repo_root = resolve(test_dir, '..'); + +test('geo_fanout tool is registered and well-formed over stdio', async()=>{ + const env = { + ...process.env, + API_TOKEN: 'dummy-token', + PRO_MODE: 'true', + }; + const client = new Client( + {name: 'geo-fanout-test', version: '0.0.1'}, + {capabilities: {tools: {}}}); + const transport = new StdioClientTransport({ + command: process.execPath, + args: ['server.js'], + cwd: repo_root, + env, + }); + try { + await client.connect(transport); + const {tools} = await client.listTools(); + const geo_fanout = tools.find(tool=>tool.name=='geo_fanout'); + assert.ok(geo_fanout, 'geo_fanout tool is exposed'); + assert.match(geo_fanout.description, /first-class/i, + 'description documents first-class classified results'); + const props = geo_fanout.inputSchema?.properties || {}; + assert.ok(props.url, 'geo_fanout exposes a url parameter'); + assert.ok(props.countries, 'geo_fanout exposes a countries parameter'); + } finally { + await client.close(); + } +}); diff --git a/test/geo-utils.test.js b/test/geo-utils.test.js new file mode 100644 index 0000000..76c9df1 --- /dev/null +++ b/test/geo-utils.test.js @@ -0,0 +1,292 @@ +'use strict'; /*jslint node:true es9:true*/ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { + GEO_STATUS, + normalize_geos, + parse_unlocker_json, + build_geo_entry, + summarize_fanout, +} from '../geo_utils.js'; +import {OUTCOME} from '../retry_utils.js'; + +const NOW = Date.parse('2026-01-19T20:51:08Z'); + +// Helper mirroring the geo_fanout executor's settled-attempt flow exactly: the +// Unlocker /request endpoint is called with format:'json' and answers a JSON +// envelope {status_code, headers, body}; the executor maps it with +// parse_unlocker_json and feeds the TARGET's {status, headers} into +// build_geo_entry. Driving the tests through this same path is the whole point: +// the OLD tests handed build_geo_entry a synthetic axios-status fixture (the +// gateway's 200), which misrepresented the real shape and hid the bug where a +// target 403/451/3xx was classified as ok. +function entry_from_envelope(geo, url, envelope, now = NOW){ + const parsed = parse_unlocker_json(envelope); + return build_geo_entry({ + geo, + url, + body: parsed.body, + response: {status: parsed.status, headers: parsed.headers}, + }, now); +} + +// A thrown transport error never produces an envelope; the executor passes the +// raw error straight into build_geo_entry as {error}. +function entry_from_error(geo, url, error, now = NOW){ + return build_geo_entry({geo, url, error}, now); +} + +const normalize_ok_cases = [ + { + name: 'lowercases and trims', + input: [' US ', 'De'], + expected: ['us', 'de'], + }, + { + name: 'dedupes preserving first-seen order', + input: ['de', 'be', 'DE', 'be', 'fr'], + expected: ['de', 'be', 'fr'], + }, + { + name: 'single code', + input: ['gb'], + expected: ['gb'], + }, +]; + +test('normalize_geos valid cases (table-driven)', ()=>{ + for (const tc of normalize_ok_cases) + { + assert.deepEqual(normalize_geos(tc.input), tc.expected, tc.name); + } +}); + +const normalize_throw_cases = [ + {name: 'empty array throws', input: [], match: /non-empty array/}, + {name: 'not an array throws', input: 'us', match: /non-empty array/}, + {name: 'three-letter code throws', input: ['usa'], + match: /invalid geo country code/}, + {name: 'one-letter code throws', input: ['u'], + match: /invalid geo country code/}, + {name: 'digits throw', input: ['12'], match: /invalid geo country code/}, + {name: 'non-string entry throws', input: [42], match: /not a string/}, +]; + +test('normalize_geos invalid cases throw loudly (table-driven)', ()=>{ + for (const tc of normalize_throw_cases) + { + assert.throws(()=>normalize_geos(tc.input), tc.match, tc.name); + } +}); + +// parse_unlocker_json maps the REAL Unlocker envelope {status_code, headers, +// body} to the {status, headers, body} shape build_geo_entry expects. These +// fixtures use the verified live shape: a normal page is status_code 200, a +// blocked page is 403, a redirect is a 302 carrying headers.location WITHOUT the +// Unlocker following it. +const parse_cases = [ + { + name: 'maps status_code -> status and passes headers/body through', + input: {status_code: 200, headers: {'content-type': 'text/html'}, + body: 'ok'}, + expected: {status: 200, headers: {'content-type': 'text/html'}, + body: 'ok'}, + }, + { + name: 'maps a 403 target status', + input: {status_code: 403, headers: {}, body: 'forbidden'}, + expected: {status: 403, headers: {}, body: 'forbidden'}, + }, + { + name: 'maps a 302 keeping the target location header', + input: {status_code: 302, + headers: {location: 'https://be.shop.example/blocked'}, body: ''}, + expected: {status: 302, + headers: {location: 'https://be.shop.example/blocked'}, body: ''}, + }, + { + name: 'parses a JSON string envelope (responseType fallback)', + input: JSON.stringify({status_code: 200, headers: {}, body: 'hi'}), + expected: {status: 200, headers: {}, body: 'hi'}, + }, + { + name: 'tolerates a non-object input -> null status', + input: null, + expected: {status: null, headers: undefined, body: undefined}, + }, + { + name: 'tolerates a non-JSON string -> null status', + input: 'not json at all', + expected: {status: null, headers: undefined, body: undefined}, + }, + { + name: 'tolerates a missing status_code -> null status', + input: {headers: {x: '1'}, body: 'b'}, + expected: {status: null, headers: {x: '1'}, body: 'b'}, + }, +]; + +test('parse_unlocker_json maps the real Unlocker envelope (table-driven)', ()=>{ + for (const tc of parse_cases) + { + assert.deepEqual(parse_unlocker_json(tc.input), tc.expected, tc.name); + } +}); + +// End-to-end from a realistic parsed attempt: each case starts from the REAL +// Unlocker envelope (or a thrown error) and goes through the same mapping the +// executor uses, so the assertion is on the bug's actual surface (target status, +// not gateway 200). +const envelope_cases = [ + { + name: '200 envelope -> ok', + url: 'https://shop.example/p/1', + envelope: {status_code: 200, headers: {'content-type': 'text/html'}, + body: 'price 19.99'}, + status: GEO_STATUS.OK, + http_status: 200, + outcome: OUTCOME.SUCCESS, + body: 'price 19.99', + }, + { + name: '403 envelope -> blocked as first-class result (not ok)', + url: 'https://shop.example/p/1', + envelope: {status_code: 403, headers: {}, body: 'forbidden'}, + status: GEO_STATUS.BLOCKED, + http_status: 403, + outcome: OUTCOME.BLOCKED, + }, + { + name: '451 envelope -> blocked', + url: 'https://shop.example/p/1', + envelope: {status_code: 451, headers: {}, body: ''}, + status: GEO_STATUS.BLOCKED, + http_status: 451, + outcome: OUTCOME.BLOCKED, + }, + { + name: '302 cross-host envelope -> redirected with location captured', + url: 'https://de.shop.example/p/1', + envelope: {status_code: 302, + headers: {location: 'https://be.shop.example/blocked'}, body: ''}, + status: GEO_STATUS.REDIRECTED, + http_status: 302, + outcome: OUTCOME.REDIRECT, + location: 'https://be.shop.example/blocked', + cross_host: true, + }, + { + name: '301 same-host envelope -> redirected, cross_host false', + url: 'https://shop.example/p/1', + envelope: {status_code: 301, + headers: {location: 'https://shop.example/p/1/'}, body: ''}, + status: GEO_STATUS.REDIRECTED, + http_status: 301, + outcome: OUTCOME.REDIRECT, + location: 'https://shop.example/p/1/', + cross_host: false, + }, + { + name: '429 envelope -> rate_limited honoring retry-after', + url: 'https://shop.example/p/1', + envelope: {status_code: 429, headers: {'retry-after': '3'}, body: ''}, + status: GEO_STATUS.RATE_LIMITED, + http_status: 429, + outcome: OUTCOME.RATE_LIMITED, + retry_after_ms: 3000, + }, + { + name: '502 envelope -> error (transient, surfaced not dropped)', + url: 'https://shop.example/p/1', + envelope: {status_code: 502, headers: {}, body: ''}, + status: GEO_STATUS.ERROR, + http_status: 502, + outcome: OUTCOME.RETRYABLE, + }, +]; + +test('build_geo_entry from real Unlocker envelopes (table-driven)', ()=>{ + for (const tc of envelope_cases) + { + const e = entry_from_envelope(tc.geo || 'xx', tc.url, tc.envelope); + assert.equal(e.status, tc.status, `${tc.name}: status`); + assert.equal(e.http_status, tc.http_status, + `${tc.name}: http_status is the TARGET status`); + if (tc.outcome!==undefined) + assert.equal(e.outcome, tc.outcome, `${tc.name}: outcome`); + if (tc.location!==undefined) + { + assert.ok(e.redirect, `${tc.name}: redirect present`); + assert.equal(e.redirect.location, tc.location, + `${tc.name}: location captured`); + } + if (tc.cross_host!==undefined) + { + assert.equal(e.redirect.cross_host, tc.cross_host, + `${tc.name}: cross_host`); + } + if (tc.retry_after_ms!==undefined) + { + assert.equal(e.retry_after_ms, tc.retry_after_ms, + `${tc.name}: retry_after_ms`); + } + if (tc.body!==undefined) + assert.equal(e.body, tc.body, `${tc.name}: body carried through`); + // exit_ip is never fabricated on the json path. + assert.equal(e.exit_ip, null, `${tc.name}: exit_ip best-effort null`); + assert.equal(typeof e.reason, 'string', `${tc.name}: reason string`); + } +}); + +test('build_geo_entry surfaces a thrown transport error as error', ()=>{ + const e = entry_from_error('es', 'https://shop.example/p/1', + {code: 'ECONNRESET'}); + assert.equal(e.status, GEO_STATUS.ERROR); + assert.equal(e.http_status, null); + assert.equal(e.outcome, OUTCOME.RETRYABLE); + assert.equal(typeof e.reason, 'string'); +}); + +test('build_geo_entry surfaces an error carrying an http response', ()=>{ + // A thrown axios error that still has response.status (e.g. a 429 the + // transport raised) is classified on that real status, not dropped. + const e = entry_from_error('fr', 'https://shop.example/p/1', + {response: {status: 429, headers: {'retry-after': '2'}}}); + assert.equal(e.status, GEO_STATUS.RATE_LIMITED); + assert.equal(e.http_status, 429); + assert.equal(e.retry_after_ms, 2000); +}); + +test('summarize_fanout counts every geo with none dropped', ()=>{ + const entries = [ + entry_from_envelope('de', 'https://shop.example/p', + {status_code: 200, headers: {}, body: 'ok'}), + entry_from_envelope('be', 'https://shop.example/p', + {status_code: 403, headers: {}, body: ''}), + entry_from_envelope('fr', 'https://de.shop.example/p', + {status_code: 302, + headers: {location: 'https://fr.shop.example/x'}, body: ''}), + entry_from_envelope('it', 'https://shop.example/p', + {status_code: 429, headers: {'retry-after': '1'}, body: ''}), + entry_from_error('es', 'https://shop.example/p', {code: 'ETIMEDOUT'}), + ]; + const report = summarize_fanout(entries); + assert.equal(report.summary.total, 5); + assert.equal(report.summary.ok, 1); + assert.equal(report.summary.blocked, 1); + assert.equal(report.summary.redirected, 1); + assert.equal(report.summary.rate_limited, 1); + assert.equal(report.summary.error, 1); + assert.equal(report.any_blocked, true); + assert.equal(report.any_redirected, true); + assert.equal(report.results.length, 5, + 'every geo is present in results, nothing discarded'); +}); + +test('summarize_fanout handles empty input without crashing', ()=>{ + const report = summarize_fanout([]); + assert.equal(report.summary.total, 0); + assert.equal(report.any_blocked, false); + assert.equal(report.any_redirected, false); + assert.deepEqual(report.results, []); +}); diff --git a/tool_groups.js b/tool_groups.js index 2c48d31..c15523a 100644 --- a/tool_groups.js +++ b/tool_groups.js @@ -139,6 +139,7 @@ export const GROUPS = { ...base_tools, 'search_engine_batch', 'scrape_batch', + 'geo_fanout', 'scrape_as_html', 'extract', 'session_stats',