From cb3ddfd6f99ad5a4f8f0a29a1133691f4aaa6d9a Mon Sep 17 00:00:00 2001 From: gonzalo-zenrows Date: Fri, 16 Oct 2026 19:02:27 +0200 Subject: [PATCH 1/5] feat(fetch): retry uncharged busy answers so a brief capacity wait does not fail the command Fetch and Extract sent each request once. A 429, 502, 503 or 504 (none of which the gateway charges) or a network error failed the command, and 503 RESP009 fell into the generic branch that advised the most expensive manual configuration. - Retry those answers up to --retries times (default 2; 0 sends once). Each wait honours Retry-After in whole seconds, capped at 30 s, or is full jitter: uniform(0.25 s, min(30 s, 1 s x 2^attempt)). No retry starts once the time spent plus the next wait would pass 90 s. - Never retry the client timeout: the API keeps working and may still charge the request. Never retry other 4xx, or a 429 that says the account is out of credits. The Extract-not-enabled fallback to autoparse is unchanged. - 503 RESP009 is now SERVICE_BUSY: not charged, retry shortly, no escalation. - The gateway budget is 180 s for the whole request, not 90 s. The client timeout stays at 120 s; the timeout error now says the API may still finish and charge the request, and suggests a value above the budget. - Retry-After parsing and the delay schedules move to src/core/backoff.ts; Crawl keeps its own schedule unchanged. --- README.md | 8 + skills/protected-fetch/SKILL.md | 3 + skills/trace-debug/SKILL.md | 9 +- src/adapters/protected-fetch.ts | 7 +- src/cli/commands/extract.ts | 4 + src/cli/commands/fetch.ts | 24 ++- src/core/backoff.ts | 37 ++++ src/core/crawl-api.ts | 15 +- src/core/errors.ts | 1 + src/core/http.ts | 152 ++++++++++++++-- tests/fetch-retry.test.ts | 314 ++++++++++++++++++++++++++++++++ tests/fetch-timeout.test.ts | 20 +- 12 files changed, 551 insertions(+), 43 deletions(-) create mode 100644 src/core/backoff.ts create mode 100644 tests/fetch-retry.test.ts diff --git a/README.md b/README.md index a884c0d..4cb586e 100644 --- a/README.md +++ b/README.md @@ -112,8 +112,16 @@ zenrows fetch # Adaptive Stealth Mode (mode=auto) zenrows fetch --output markdown zenrows fetch --manual --js-render --premium-proxy zenrows fetch --proxy-country us --wait-for ".price" +zenrows fetch --retries 0 # send once; the default is 2 retries ``` +`fetch` and `extract` retry a 429, 502, 503 or 504 answer and a network error +up to `--retries` times (default 2). Each wait honours `Retry-After` (at most +30 s) or is a random delay that grows with each retry, and no retry starts +after 90 s in total. None of these statuses is charged. A client timeout +(`--timeout`) is never retried, because the API may still finish and charge +that request. + ## 8. Extract ```bash diff --git a/skills/protected-fetch/SKILL.md b/skills/protected-fetch/SKILL.md index 0448f46..5b9148f 100644 --- a/skills/protected-fetch/SKILL.md +++ b/skills/protected-fetch/SKILL.md @@ -47,6 +47,9 @@ zenrows fetch --manual --js-render --premium-proxy # the most expensive needs `--premium-proxy` (else `PARAM_PROXY_COUNTRY_REQUIRES_PREMIUM`). - You are billed only for the configuration that succeeds. - On failure, read `zenrows trace explain ` before escalating. +- The CLI already retries 429, 502, 503 and 504 answers and network errors + (`--retries`, default 2; `--retries 0` sends once). Do not wrap it in your own + retry loop. Every run is saved under `.zenrows/runs//` (no secrets). See [[extract]] for structured output and [[cost-control]] for the cost model. diff --git a/skills/trace-debug/SKILL.md b/skills/trace-debug/SKILL.md index 54f243e..b93165f 100644 --- a/skills/trace-debug/SKILL.md +++ b/skills/trace-debug/SKILL.md @@ -27,9 +27,14 @@ zenrows trace export # JSON for sharing multiplies the cost of the request, and both together are the most expensive configuration available ([[cost-control]]). - `REQUEST_TIMEOUT` → the CLI stopped waiting; the API was reached. Raise - `--timeout` (default 120000ms, above the API's own 90s budget), or drop + `--timeout` (default 120000ms; the API's own budget is 180s), or drop `--wait-for` so the request finishes inside that budget and the API returns - its own error. Not a connectivity problem — do not chase the network. + its own error. Not a connectivity problem — do not chase the network. The CLI + never retries a timeout on its own: the API may still finish and charge it. +- `SERVICE_BUSY` → HTTP 503 `RESP009`: Zenrows had no capacity for the request + yet, or Extract is still preparing this layout. Not charged. The CLI already + retried it (`--retries`, default 2); run the same command again in about a + minute. Do not escalate to `--js-render` / `--premium-proxy`. - `BACKEND_UNAVAILABLE` → a genuine transport failure (DNS/TCP/TLS). Check connectivity and `zenrows config show`. - `AUTH_INVALID` → re-check the key, `zenrows login --api-key …`. diff --git a/src/adapters/protected-fetch.ts b/src/adapters/protected-fetch.ts index 7b68c25..ad403c1 100644 --- a/src/adapters/protected-fetch.ts +++ b/src/adapters/protected-fetch.ts @@ -51,6 +51,11 @@ export interface FetchOptions { * gateway's own request budget on purpose. */ timeoutMs?: number; + /** + * Retries for a retryable failure (429, 502, 503, 504, or a network error). + * Not an API parameter. Unset means `http.ts`'s default; 0 sends the request once. + */ + retries?: number; } const RESPONSE_TYPE: Partial> = { @@ -152,6 +157,6 @@ export async function runFetch( validateAutoManual(opts, config); const params = buildParams(opts, config); const mode: "auto" | "manual" = params.mode === "auto" ? "auto" : "manual"; - const result = await scrape(config.apiBase, apiKey, params, { timeoutMs: opts.timeoutMs, projectRoot }); + const result = await scrape(config.apiBase, apiKey, params, { timeoutMs: opts.timeoutMs, retries: opts.retries, projectRoot }); return { result, params, mode }; } diff --git a/src/cli/commands/extract.ts b/src/cli/commands/extract.ts index 50445db..ebca1b7 100644 --- a/src/cli/commands/extract.ts +++ b/src/cli/commands/extract.ts @@ -18,6 +18,7 @@ import { formatRequestCost } from "../../core/http.ts"; import { parse, asString, asNumber, type Command, type RunContext } from "../command.ts"; import { printError, writeOut } from "../output.ts"; import { refreshClaimAdvice } from "../../core/claim-link.ts"; +import { normalizeRetries } from "./fetch.ts"; export const extract: Command = { name: "extract", @@ -35,6 +36,7 @@ export const extract: Command = { "Shared:", " --manual --js-render --premium-proxy manual fetch controls", " --validate fail if the result is not valid JSON", + " --retries retries on 429/502/503/504 or a network error (default 2; 0 disables)", " --out write output to a file", " --no-signup do not auto-create a Free plan account if no key exists", " --json structured result", @@ -55,6 +57,7 @@ export const extract: Command = { wait: { type: "string" }, "wait-for": { type: "string" }, validate: { type: "boolean" }, + retries: { type: "string" }, out: { type: "string" }, "no-signup": { type: "boolean" }, json: { type: "boolean" }, @@ -105,6 +108,7 @@ export const extract: Command = { wait: asNumber(values.wait), waitFor: asString(values["wait-for"]), validate: values.validate === true, + retries: normalizeRetries(values.retries), }; assertUsable("extract"); diff --git a/src/cli/commands/fetch.ts b/src/cli/commands/fetch.ts index f5437a3..f5e6d16 100644 --- a/src/cli/commands/fetch.ts +++ b/src/cli/commands/fetch.ts @@ -36,7 +36,8 @@ export const fetch_: Command = { " --output html (default) | markdown | text | pdf", " --screenshot capture an above-the-fold screenshot", " --out write the response body to a file", - " --timeout client-side timeout (default 120000; above the API's own 90s budget)", + " --timeout client-side timeout per attempt (default 120000; the API's own budget is 180s)", + " --retries retries on 429/502/503/504 or a network error (default 2; 0 disables)", " --no-signup do not auto-create a Free plan account if no key exists", " --json print a structured result", "", @@ -59,6 +60,7 @@ export const fetch_: Command = { screenshot: { type: "boolean" }, out: { type: "string" }, timeout: { type: "string" }, + retries: { type: "string" }, "no-signup": { type: "boolean" }, json: { type: "boolean" }, }); @@ -101,6 +103,7 @@ export const fetch_: Command = { output: normalizeOutput(asString(values.output)), screenshot: values.screenshot === true, timeoutMs: normalizeTimeout(values.timeout), + retries: normalizeRetries(values.retries), }; const runId = newRunId(); @@ -209,6 +212,25 @@ export function normalizeTimeout(v: unknown): number | undefined { return ms; } +/** + * `--retries `: a whole number, 0 or more. Like `--timeout`, a bad value is + * an error, never a silent fallback to the default. + */ +export function normalizeRetries(v: unknown): number | undefined { + if (v === undefined) return undefined; + const n = asNumber(v); + if (n === undefined || !Number.isInteger(n) || n < 0) { + throw new ToolkitError({ + code: "INVALID_USAGE", + message: `Invalid --retries value '${String(v)}'.`, + likely_cause: "--retries takes a whole number, 0 or more.", + next_action: "Pass a count, e.g. --retries 2, or --retries 0 to send the request once.", + suggested_commands: ["zenrows fetch --retries 0"], + }); + } + return n; +} + export function normalizeOutput(v?: string): ResponseFormat | undefined { if (!v) return undefined; const map: Record = { diff --git a/src/core/backoff.ts b/src/core/backoff.ts new file mode 100644 index 0000000..d8ff54a --- /dev/null +++ b/src/core/backoff.ts @@ -0,0 +1,37 @@ +/** + * Retry delays shared by the HTTP clients. + * + * Crawl keeps its own schedule (`crawlBackoffMs`). Fetch and Extract use full + * jitter (`fullJitterMs`), the policy every Zenrows Fetch/Extract client shares. + * Both honour a `Retry-After` in whole seconds through `retryAfterMs`. + */ + +/** The longest a Retry-After may make one retry wait, so a large server value cannot hang a command. */ +export const MAX_RETRY_AFTER_SECONDS = 30; + +/** Crawl's schedule: 250 ms x 2^attempt, capped at 10 s, +/-20% jitter. */ +export function crawlBackoffMs(attempt: number): number { + return Math.min(250 * 2 ** attempt, 10_000) * (0.8 + Math.random() * 0.4); +} + +/** Shortest full-jitter wait: never retry in a tight loop. */ +export const JITTER_FLOOR_MS = 250; +/** Longest full-jitter wait. */ +export const JITTER_CAP_MS = 30_000; + +/** + * Full jitter: uniform(0.25 s, min(30 s, 1 s x 2^attempt)). `attempt` is the + * 0-based retry index, so the first retry waits at most 1 s and the second 2 s. + * `random` is injectable for tests. + */ +export function fullJitterMs(attempt: number, random: () => number = Math.random): number { + const ceiling = Math.max(JITTER_FLOOR_MS, Math.min(JITTER_CAP_MS, 1000 * 2 ** attempt)); + return JITTER_FLOOR_MS + random() * (ceiling - JITTER_FLOOR_MS); +} + +/** `Retry-After` in whole seconds, as milliseconds, at most `MAX_RETRY_AFTER_SECONDS`. */ +export function retryAfterMs(value: string | null): number | undefined { + return value && /^\d+$/.test(value.trim()) ? Math.min(Number(value), MAX_RETRY_AFTER_SECONDS) * 1000 : undefined; +} + +export const defaultSleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); diff --git a/src/core/crawl-api.ts b/src/core/crawl-api.ts index 91d6e43..3318b97 100644 --- a/src/core/crawl-api.ts +++ b/src/core/crawl-api.ts @@ -20,6 +20,7 @@ import { readAccount } from "./agent-account.ts"; import { registerSecret } from "./logger.ts"; import { CLI_USER_AGENT, loadConfig } from "./config.ts"; import { agentClientHeader } from "./agent-client.ts"; +import { crawlBackoffMs as backoffMs, defaultSleep, retryAfterMs } from "./backoff.ts"; /** Production Crawl API base (no trailing slash): the Fetch/Extract host. */ export const DEFAULT_CRAWL_API_BASE = "https://api.zenrows.com/v1"; @@ -176,8 +177,6 @@ interface RawResponse { } const MAX_RETRIES = 3; -/** The longest a Retry-After may make one retry wait, so a large server value cannot hang a command. */ -const MAX_RETRY_AFTER_SECONDS = 30; const READ_RETRY_STATUSES: ReadonlySet = new Set([429, 502, 503, 504]); // 429 on create is the active-job limit: a retry only waits for a slot that may never free. const CREATE_RETRY_STATUSES: ReadonlySet = new Set([502, 503, 504]); @@ -194,18 +193,6 @@ function retryStatuses(method: string, headers: Record): Readonl return headers["Idempotency-Key"] ? CREATE_RETRY_STATUSES : undefined; } -/** 250 ms x 2^attempt, capped at 10 s, +/-20% jitter. */ -function backoffMs(attempt: number): number { - return Math.min(250 * 2 ** attempt, 10_000) * (0.8 + Math.random() * 0.4); -} - -/** `Retry-After` in whole seconds, as milliseconds, at most `MAX_RETRY_AFTER_SECONDS`. */ -function retryAfterMs(value: string | null): number | undefined { - return value && /^\d+$/.test(value.trim()) ? Math.min(Number(value), MAX_RETRY_AFTER_SECONDS) * 1000 : undefined; -} - -const defaultSleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); - /** One HTTP attempt with its own timeout; the body is read inside it. */ async function attempt(url: string, init: RequestInit, opts: RequestOpts): Promise { const controller = new AbortController(); diff --git a/src/core/errors.ts b/src/core/errors.ts index ceb80a4..e993f74 100644 --- a/src/core/errors.ts +++ b/src/core/errors.ts @@ -12,6 +12,7 @@ export type ErrorCode = | "AUTH_INVALID" | "BACKEND_UNAVAILABLE" | "REQUEST_TIMEOUT" + | "SERVICE_BUSY" | "CAPABILITY_UNAVAILABLE" | "PARAM_CONFLICT_AUTO_MANUAL" | "PARAM_PROXY_COUNTRY_REQUIRES_PREMIUM" diff --git a/src/core/http.ts b/src/core/http.ts index 0e3e4af..2f530cd 100644 --- a/src/core/http.ts +++ b/src/core/http.ts @@ -9,9 +9,10 @@ import { ToolkitError, isKeyCapReached, keyCapReached, quotaExhausted } from "./ import { storedClaimHint, usableClaimUrl } from "./claim-link.ts"; import { readAccount } from "./agent-account.ts"; import { ENV_KEY, resolveApiKey } from "./auth.ts"; -import { registerSecret } from "./logger.ts"; +import { log, registerSecret } from "./logger.ts"; import { CLI_USER_AGENT } from "./config.ts"; import { agentClientHeader } from "./agent-client.ts"; +import { defaultSleep, fullJitterMs, retryAfterMs } from "./backoff.ts"; export interface ScraperResult { status: number; @@ -52,20 +53,44 @@ export interface ScraperParams { } /** - * Client-side timeout for a Fetch and Extract request, deliberately ABOVE the - * gateway's own 90s request budget. + * Client-side timeout for one Fetch and Extract HTTP attempt. * - * The two must not be equal. When they were both 90s, any request that used - * the full server budget became a race between our own abort and the API's - * real error envelope — and the abort usually won, so a specific, actionable - * server error (a 422, a 499) was reported as "could not reach the API". The - * client must outlive the server budget so the API always gets the chance to - * answer for itself. + * It was raised from 90s when the gateway's budget was believed to be 90s, so + * that our abort would not race the API's own error envelope. The gateway's + * real budget is `SERVER_BUDGET_MS` (180s), so a request that runs longer than + * this can still be aborted while the API is working on it. That is why a + * timeout is never retried: the API keeps running and may still charge a 200. */ export const DEFAULT_TIMEOUT_MS = 120_000; -/** The gateway's own request budget. Kept here only to justify the default above. */ -export const SERVER_BUDGET_MS = 90_000; +/** The gateway's budget for the whole request: auth, fetch and parse. */ +export const SERVER_BUDGET_MS = 180_000; + +/** Retries for a retryable failure (`--retries`). 0 sends the request once. */ +export const DEFAULT_RETRIES = 2; + +/** No retry starts when the time already spent plus its delay would pass this. */ +export const RETRY_BUDGET_MS = 90_000; + +/** + * Statuses that can succeed on the next attempt. The gateway charges only + * 2xx, 304, 404 and 410, so none of these is charged. + */ +const RETRY_STATUSES: ReadonlySet = new Set([429, 502, 503, 504]); + +export interface ScrapeOptions { + /** Per HTTP attempt. */ + timeoutMs?: number; + projectRoot?: string; + /** Retries after the first attempt; defaults to `DEFAULT_RETRIES`. */ + retries?: number; + fetchImpl?: typeof fetch; + sleepImpl?: (ms: number) => Promise; + /** Clock for the retry budget. */ + nowImpl?: () => number; + /** Source of the jitter. */ + randomImpl?: () => number; +} function buildUrl(apiBase: string, apiKey: string, params: ScraperParams): { full: string; redacted: string } { const u = new URL(apiBase); @@ -80,15 +105,69 @@ function buildUrl(apiBase: string, apiKey: string, params: ScraperParams): { ful return { full, redacted: redactedUrl.toString() }; } -/** Perform a Fetch and Extract API request. Throws ToolkitError on failure. */ +/** + * Perform a Fetch and Extract API request. Throws ToolkitError on failure. + * + * Retries 429, 502, 503 and 504 and network errors that came before any + * response, up to `retries` times, with full jitter or the server's + * Retry-After. A retry never starts past `RETRY_BUDGET_MS`. Our own timeout is + * never retried, and neither is any other status. + */ export async function scrape( apiBase: string, apiKey: string, params: ScraperParams, - opts: { timeoutMs?: number; projectRoot?: string } = {}, + opts: ScrapeOptions = {}, ): Promise { registerSecret(apiKey); const { full, redacted } = buildUrl(apiBase, apiKey, params); + const retries = opts.retries ?? DEFAULT_RETRIES; + const sleep = opts.sleepImpl ?? defaultSleep; + const now = opts.nowImpl ?? Date.now; + const startedAt = now(); + + for (let n = 0; ; n++) { + let sent: { res: Response; raw: Buffer }; + try { + sent = await sendOnce(full, params.url, opts); + } catch (err) { + // REQUEST_TIMEOUT is not retried: the API keeps working and may still charge. + if (n >= retries || !(err instanceof ToolkitError) || err.code !== "BACKEND_UNAVAILABLE") throw err; + const delayMs = fullJitterMs(n, opts.randomImpl); + if (now() - startedAt + delayMs > RETRY_BUDGET_MS) throw err; + log.dim(`Network error; retry ${n + 1}/${retries} in ${formatMs(delayMs)}`); + await sleep(delayMs); + continue; + } + const body = sent.raw.toString("utf8"); + if (n < retries && isRetryable(sent.res.status, body)) { + const delayMs = retryAfterMs(sent.res.headers.get("retry-after")) ?? fullJitterMs(n, opts.randomImpl); + if (now() - startedAt + delayMs <= RETRY_BUDGET_MS) { + const code = zrErrorCode(body); + log.dim(`HTTP ${sent.res.status}${code ? ` ${code}` : ""}; retry ${n + 1}/${retries} in ${formatMs(delayMs)}`); + await sleep(delayMs); + continue; + } + } + return interpret(sent.res, sent.raw, params, redacted, n); + } +} + +/** + * True for a status worth another attempt. A 429 that says the account is out + * of credits is excluded: it answers the same until the account changes. + */ +function isRetryable(status: number, body: string): boolean { + if (!RETRY_STATUSES.has(status)) return false; + return !(status === 429 && isQuotaError(body)); +} + +/** One HTTP attempt with its own timeout; the body is read inside it. */ +async function sendOnce( + full: string, + targetUrl: string, + opts: ScrapeOptions, +): Promise<{ res: Response; raw: Buffer }> { const timeoutMs = opts.timeoutMs ?? DEFAULT_TIMEOUT_MS; const controller = new AbortController(); // Our own timer is the only thing that aborts this controller, so this flag — @@ -103,7 +182,7 @@ export async function scrape( let res: Response; try { - res = await fetch(full, { + res = await (opts.fetchImpl ?? fetch)(full, { method: "GET", headers: { "User-Agent": CLI_USER_AGENT, "Accept-Encoding": "gzip, deflate", ...agentClientHeader({ projectRoot: opts.projectRoot }) }, signal: controller.signal, @@ -111,7 +190,7 @@ export async function scrape( } catch (err) { clearTimeout(timeout); const elapsedMs = Date.now() - startedAt; - if (timedOut) throw requestTimeout(params.url, timeoutMs, elapsedMs); + if (timedOut) throw requestTimeout(targetUrl, timeoutMs, elapsedMs); const cause = err instanceof Error ? err.message : String(err); throw new ToolkitError({ code: "BACKEND_UNAVAILABLE", @@ -127,6 +206,17 @@ export async function scrape( // output; `raw` preserves the exact bytes so binary responses (screenshot / // PDF) can be written without the UTF-8 round-trip that corrupts them. const raw = Buffer.from(await res.arrayBuffer()); + return { res, raw }; +} + +/** Turn one final response into a result, or a normalized error. */ +function interpret( + res: Response, + raw: Buffer, + params: ScraperParams, + redacted: string, + retriesUsed: number, +): ScraperResult { const body = raw.toString("utf8"); // A binary body is only ever produced by a successful screenshot/PDF request. // Zenrows error responses are always a JSON envelope (text), so exclude those. @@ -295,6 +385,7 @@ export async function scrape( : "Verify the URL. Rendering or premium proxies will not turn a 404 into content — do not retry; it will fail identically and still cost credits.", }); } + if (res.status === 503 && zrErrorCode(body) === "RESP009") throw serviceBusy(params, body, res.headers.get("retry-after"), retriesUsed); if (res.status === 422 || res.status >= 500 || (res.status >= 400 && !looksLikeContent(result))) { throw new ToolkitError({ code: "FETCH_FAILED", @@ -317,11 +408,38 @@ export async function scrape( return result; } +/** + * 503 RESP009: Zenrows had no capacity for this request yet. It is temporary + * and not charged, so the fix is to wait and send the same request, not to + * escalate to --js-render / --premium-proxy (which costs more and helps nothing). + */ +function serviceBusy(params: ScraperParams, body: string, retryAfter: string | null, retriesUsed: number): ToolkitError { + const isExtract = params.extract !== undefined; + const command = `zenrows ${isExtract ? "extract" : "fetch"} ${params.url}`; + const detail = zrErrorDetail(body); + const tried = retriesUsed > 0 ? ` after ${retriesUsed} ${retriesUsed === 1 ? "retry" : "retries"}` : ""; + const waitSeconds = retryAfterMs(retryAfter); + return new ToolkitError({ + code: "SERVICE_BUSY", + message: `Zenrows could not take this request yet (HTTP 503 RESP009${tried}). It was not charged.`, + likely_cause: + (isExtract + ? "The extraction service is still preparing this layout; the request was not charged." + : "Zenrows scraping capacity is busy right now; the request was not charged.") + + (detail ? ` Server said: ${detail}.` : ""), + next_action: + "Retry the same command shortly (in about a minute). Do not add --js-render or --premium-proxy: they cost more and do not help here.", + suggested_commands: [command], + server_code: "RESP009", + retry_after: waitSeconds !== undefined ? waitSeconds / 1000 : undefined, + }); +} + /** * The CLI gave up waiting before the API answered. This is NOT unreachability: * the connection was established and the gateway was still working, so telling * the operator to check connectivity would send them at the wrong problem. The - * elapsed time is included so the 90s server budget is visible in the output. + * elapsed time is included so the server budget is visible in the output. */ export function requestTimeout(url: string, timeoutMs: number, elapsedMs: number): ToolkitError { const suggestedMs = Math.max(timeoutMs + 60_000, SERVER_BUDGET_MS + 60_000); @@ -333,7 +451,7 @@ export function requestTimeout(url: string, timeoutMs: number, elapsedMs: number `The request was aborted client-side after ${formatMs(elapsedMs)}. The API was reached — this is not a ` + (atBudget ? `connectivity problem. The request also passed the API's own ${formatMs(SERVER_BUDGET_MS)} budget, so the target is very likely rendering slowly or a wait condition never matched.` - : `connectivity problem, and the API's own ${formatMs(SERVER_BUDGET_MS)} budget had not run out yet.`), + : `connectivity problem, and the API's own ${formatMs(SERVER_BUDGET_MS)} budget had not run out yet, so the API may still finish this request and charge it.`), next_action: `Retry with a longer client timeout (\`--timeout ${suggestedMs}\`). If the target needs a long render, drop ` + "`--wait-for` so the request finishes inside the API's budget and the API can return its own error instead.", diff --git a/tests/fetch-retry.test.ts b/tests/fetch-retry.test.ts new file mode 100644 index 0000000..85693c7 --- /dev/null +++ b/tests/fetch-retry.test.ts @@ -0,0 +1,314 @@ +/** + * Fetch and Extract retry policy: which failures are retried, how long each + * wait is, and when the CLI stops. Every test stubs fetch and injects the + * sleep (and the clock, where the budget matters), so nothing here waits. + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { scrape, DEFAULT_RETRIES, RETRY_BUDGET_MS, type ScrapeOptions } from "../src/core/http.ts"; +import { fullJitterMs, retryAfterMs, crawlBackoffMs } from "../src/core/backoff.ts"; +import { runFetch } from "../src/adapters/protected-fetch.ts"; +import { runExtract } from "../src/adapters/extract.ts"; +import { normalizeRetries } from "../src/cli/commands/fetch.ts"; +import { defaultConfig } from "../src/core/config.ts"; +import { defaultPolicy } from "../src/core/policy.ts"; +import { ToolkitError } from "../src/core/errors.ts"; + +const API = "https://api.zenrows.com/v1/"; + +const RESP009 = JSON.stringify({ + code: "RESP009", + detail: "All capacity is busy right now. Retry in a minute.", + status: 503, + title: "Service temporarily at capacity (RESP009)", + type: "https://docs.zenrows.com/api-error-codes#RESP009", +}); + +const AUTH010 = JSON.stringify({ + code: "AUTH010", + detail: "Extract is not enabled for this domain.", + status: 402, + title: "Feature is not included in plan (AUTH010)", + type: "https://docs.zenrows.com/api-error-codes#AUTH010", +}); + +type Step = { status: number; body?: string; headers?: Record } | "network"; + +/** A fetch that answers `steps` in order (repeating the last) and counts calls. */ +function seqFetch(steps: Step[], onCall?: () => void): { impl: typeof fetch; calls: () => number } { + let n = 0; + const impl = (async () => { + onCall?.(); + const step = steps[Math.min(n, steps.length - 1)]!; + n++; + if (step === "network") throw new TypeError("fetch failed"); + return new Response(step.body ?? "", { + status: step.status, + headers: { "content-type": "application/json", ...step.headers }, + }); + }) as unknown as typeof fetch; + return { impl, calls: () => n }; +} + +function recordSleep(): { sleepImpl: (ms: number) => Promise; waits: number[] } { + const waits: number[] = []; + return { waits, sleepImpl: async (ms: number) => void waits.push(ms) }; +} + +function withGlobalFetch(impl: typeof fetch, fn: () => Promise): Promise { + const orig = globalThis.fetch; + globalThis.fetch = impl; + return fn().finally(() => { + globalThis.fetch = orig; + }); +} + +const OK: Step = { status: 200, body: "ok", headers: { "content-type": "text/html" } }; + +// --- delays ------------------------------------------------------------------- + +test("full jitter stays inside uniform(0.25 s, min(30 s, 1 s x 2^attempt))", () => { + for (let attempt = 0; attempt < 12; attempt++) { + const ceiling = Math.min(30_000, 1000 * 2 ** attempt); + assert.equal(fullJitterMs(attempt, () => 0), 250); + assert.ok(fullJitterMs(attempt, () => 0.999999) <= ceiling); + assert.ok(fullJitterMs(attempt, () => 0.999999) > ceiling - 1); + for (let i = 0; i < 200; i++) { + const ms = fullJitterMs(attempt); + assert.ok(ms >= 250 && ms <= ceiling, `attempt ${attempt}: ${ms}ms outside [250, ${ceiling}]`); + } + } +}); + +test("Retry-After counts whole seconds only and is capped at 30 s", () => { + assert.equal(retryAfterMs("7"), 7000); + assert.equal(retryAfterMs(" 2 "), 2000); + assert.equal(retryAfterMs("3600"), 30_000); + assert.equal(retryAfterMs("1.5"), undefined); + assert.equal(retryAfterMs("Wed, 21 Oct 2015 07:28:00 GMT"), undefined); + assert.equal(retryAfterMs(null), undefined); +}); + +test("crawl keeps its own schedule: 250 ms x 2^attempt, capped at 10 s, +/-20%", () => { + for (let attempt = 0; attempt < 8; attempt++) { + const base = Math.min(250 * 2 ** attempt, 10_000); + for (let i = 0; i < 100; i++) { + const ms = crawlBackoffMs(attempt); + assert.ok(ms >= base * 0.8 && ms <= base * 1.2, `attempt ${attempt}: ${ms}`); + } + } +}); + +// --- which failures are retried ------------------------------------------------- + +test("a 503 is retried with full jitter, and the first and second waits keep their bounds", async () => { + const { impl, calls } = seqFetch([{ status: 503 }, { status: 503 }, OK]); + const { sleepImpl, waits } = recordSleep(); + const r = await scrape(API, "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl }); + assert.equal(r.status, 200); + assert.equal(calls(), 3); + assert.equal(waits.length, 2); + const [first = -1, second = -1] = waits; + assert.ok(first >= 250 && first <= 1000, `first wait ${first}`); + assert.ok(second >= 250 && second <= 2000, `second wait ${second}`); +}); + +for (const status of [429, 502, 504]) { + test(`HTTP ${status} is retried`, async () => { + const { impl, calls } = seqFetch([{ status, body: "{}" }, OK]); + const { sleepImpl, waits } = recordSleep(); + const r = await scrape(API, "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl }); + assert.equal(r.status, 200); + assert.equal(calls(), 2); + assert.equal(waits.length, 1); + }); +} + +test("a network error before any response is retried", async () => { + const { impl, calls } = seqFetch(["network", "network", OK]); + const { sleepImpl, waits } = recordSleep(); + const r = await scrape(API, "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl }); + assert.equal(r.status, 200); + assert.equal(calls(), 3); + assert.equal(waits.length, 2); +}); + +test("Retry-After is honoured", async () => { + const { impl } = seqFetch([{ status: 503, headers: { "retry-after": "7" } }, OK]); + const { sleepImpl, waits } = recordSleep(); + await scrape(API, "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl }); + assert.deepEqual(waits, [7000]); +}); + +test("a large Retry-After is capped at 30 s", async () => { + const { impl } = seqFetch([{ status: 429, body: "{}", headers: { "retry-after": "3600" } }, OK]); + const { sleepImpl, waits } = recordSleep(); + await scrape(API, "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl }); + assert.deepEqual(waits, [30_000]); +}); + +for (const status of [400, 401, 404, 422]) { + test(`HTTP ${status} is not retried`, async () => { + const body = JSON.stringify({ code: "REQS002", title: "Bad request (REQS002)", status }); + const { impl, calls } = seqFetch([{ status, body }, OK]); + const { sleepImpl, waits } = recordSleep(); + await assert.rejects(() => scrape(API, "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl })); + assert.equal(calls(), 1); + assert.deepEqual(waits, []); + }); +} + +test("a 429 that says the account is out of credits is not retried", async () => { + const body = JSON.stringify({ code: "AUTH004", title: "Usage exceeded", detail: "No credit available" }); + const { impl, calls } = seqFetch([{ status: 429, body }, OK]); + const { sleepImpl } = recordSleep(); + await assert.rejects( + () => scrape(API, "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl }), + (err: unknown) => (err as ToolkitError).code === "POLICY_MAX_CREDITS_EXCEEDED", + ); + assert.equal(calls(), 1); +}); + +test("the client timeout is never retried: the API may still finish and charge it", async () => { + let calls = 0; + const hanging = ((_input: unknown, init?: { signal?: AbortSignal }) => { + calls++; + return new Promise((_resolve, reject) => { + init?.signal?.addEventListener("abort", () => reject(new DOMException("This operation was aborted", "AbortError"))); + }); + }) as unknown as typeof fetch; + const { sleepImpl, waits } = recordSleep(); + await assert.rejects( + () => scrape(API, "k", { url: "https://x" }, { fetchImpl: hanging, sleepImpl, timeoutMs: 20, retries: 5 }), + (err: unknown) => (err as ToolkitError).code === "REQUEST_TIMEOUT", + ); + assert.equal(calls, 1); + assert.deepEqual(waits, []); +}); + +// --- how many, and for how long --------------------------------------------------- + +test("the default is 2 retries: three attempts in all", async () => { + assert.equal(DEFAULT_RETRIES, 2); + const { impl, calls } = seqFetch([{ status: 503 }]); + const { sleepImpl, waits } = recordSleep(); + await assert.rejects(() => scrape(API, "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl })); + assert.equal(calls(), 3); + assert.equal(waits.length, 2); +}); + +test("retries: 0 sends the request once", async () => { + const { impl, calls } = seqFetch([{ status: 503 }, OK]); + const { sleepImpl, waits } = recordSleep(); + await assert.rejects(() => scrape(API, "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl, retries: 0 })); + assert.equal(calls(), 1); + assert.deepEqual(waits, []); +}); + +test("no retry starts when it would pass the 90 s budget", async () => { + assert.equal(RETRY_BUDGET_MS, 90_000); + let clock = 0; + // Each attempt takes 40 s; the server asks for 5 s between attempts. + const { impl, calls } = seqFetch([{ status: 503, headers: { "retry-after": "5" } }], () => (clock += 40_000)); + const opts: ScrapeOptions = { + fetchImpl: impl, + sleepImpl: async (ms) => void (clock += ms), + nowImpl: () => clock, + retries: 10, + }; + await assert.rejects(() => scrape(API, "k", { url: "https://x" }, opts)); + // 40 s + 5 s fits (45 s); then 85 s + 5 s = 90 s still fits; then 130 s does not. + assert.equal(calls(), 3); + assert.ok(clock <= RETRY_BUDGET_MS + 40_000); +}); + +test("a retry whose delay alone would pass the budget is skipped", async () => { + let clock = 0; + const { impl, calls } = seqFetch([{ status: 503, headers: { "retry-after": "30" } }], () => (clock += 70_000)); + await assert.rejects(() => + scrape(API, "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl: async (ms) => void (clock += ms), nowImpl: () => clock }), + ); + assert.equal(calls(), 1); +}); + +// --- the flag --------------------------------------------------------------------- + +test("--retries accepts whole numbers from 0, and rejects anything else", () => { + assert.equal(normalizeRetries(undefined), undefined); + assert.equal(normalizeRetries("0"), 0); + assert.equal(normalizeRetries("3"), 3); + for (const bad of ["-1", "1.5", "many", ""]) { + assert.throws(() => normalizeRetries(bad), (err: unknown) => (err as ToolkitError).code === "INVALID_USAGE", bad); + } +}); + +test("runFetch threads --retries 0 through to the HTTP client", async () => { + const { impl, calls } = seqFetch([{ status: 503 }, OK]); + await withGlobalFetch(impl, async () => { + await assert.rejects(() => runFetch({ url: "https://x.com", retries: 0 }, defaultConfig(), defaultPolicy(), "k")); + }); + assert.equal(calls(), 1); +}); + +// --- the outcome after retries ------------------------------------------------------ + +test("503 RESP009 on Extract is SERVICE_BUSY: not charged, retry shortly, no escalation", async () => { + const { impl, calls } = seqFetch([{ status: 503, body: RESP009, headers: { "retry-after": "1" } }]); + const { sleepImpl } = recordSleep(); + await assert.rejects( + () => scrape(API, "k", { url: "https://x.com/p", extract: "auto" }, { fetchImpl: impl, sleepImpl }), + (err: unknown) => { + assert.ok(err instanceof ToolkitError); + assert.equal(err.code, "SERVICE_BUSY"); + assert.equal(err.server_code, "RESP009"); + assert.equal(err.retry_after, 1); + assert.match(err.message, /RESP009 after 2 retries/); + assert.match(err.message, /not charged/i); + assert.match(err.likely_cause, /still preparing this layout; the request was not charged/); + assert.match(err.next_action, /retry/i); + assert.deepEqual(err.suggested_commands, ["zenrows extract https://x.com/p"]); + assert.ok(!err.suggested_commands.some((c) => /--js-render|--premium-proxy|--manual/.test(c))); + return true; + }, + ); + assert.equal(calls(), 3); +}); + +test("503 RESP009 on Fetch is SERVICE_BUSY with a fetch retry command", async () => { + const { impl } = seqFetch([{ status: 503, body: RESP009 }]); + await assert.rejects( + () => scrape(API, "k", { url: "https://x.com/p" }, { fetchImpl: impl, retries: 0 }), + (err: unknown) => { + const e = err as ToolkitError; + assert.equal(e.code, "SERVICE_BUSY"); + assert.match(e.likely_cause, /not charged/); + assert.doesNotMatch(e.message, /after \d+ retr/); + assert.deepEqual(e.suggested_commands, ["zenrows fetch https://x.com/p"]); + return true; + }, + ); +}); + +test("a 503 RESP009 that clears on retry returns the page", async () => { + const { impl } = seqFetch([{ status: 503, body: RESP009 }, OK]); + const { sleepImpl } = recordSleep(); + const r = await scrape(API, "k", { url: "https://x", extract: "auto" }, { fetchImpl: impl, sleepImpl }); + assert.equal(r.status, 200); +}); + +test("the Extract-not-enabled fallback to autoparse is unchanged and spends no retries", async () => { + const seen: string[] = []; + const impl = (async (input: string | URL | Request) => { + const u = new URL(String(input)); + seen.push(u.searchParams.get("extract") ? "extract" : u.searchParams.get("autoparse") ? "autoparse" : "other"); + if (u.searchParams.get("extract")) { + return new Response(AUTH010, { status: 402, headers: { "content-type": "application/json" } }); + } + return new Response(JSON.stringify({ title: "ok" }), { status: 200, headers: { "content-type": "application/json" } }); + }) as unknown as typeof fetch; + await withGlobalFetch(impl, async () => { + const outcome = await runExtract({ url: "https://x.com/p" }, defaultConfig(), defaultPolicy(), "k"); + assert.equal(outcome.fellBackToAutoparse, true); + }); + assert.deepEqual(seen, ["extract", "autoparse"]); +}); diff --git a/tests/fetch-timeout.test.ts b/tests/fetch-timeout.test.ts index df04ba0..cb553fa 100644 --- a/tests/fetch-timeout.test.ts +++ b/tests/fetch-timeout.test.ts @@ -10,6 +10,7 @@ import { test } from "node:test"; import assert from "node:assert/strict"; import { scrape, + requestTimeout, DEFAULT_TIMEOUT_MS, SERVER_BUDGET_MS, } from "../src/core/http.ts"; @@ -41,13 +42,16 @@ function withFetchImpl(impl: typeof fetch, fn: () => Promise): Promise { - assert.ok( - DEFAULT_TIMEOUT_MS > SERVER_BUDGET_MS, - `client default (${DEFAULT_TIMEOUT_MS}ms) must outlive the server budget (${SERVER_BUDGET_MS}ms), ` + - "otherwise the abort races the API's own error envelope", - ); +// The gateway's budget covers the whole request (auth, fetch, parse) and is +// 180s. The client default stays at 120s, so a timeout can abort a request the +// API is still working on; the timeout must suggest a value above the budget. +test("REQUEST_TIMEOUT suggests a client timeout above the API's own 180s budget", () => { + assert.equal(SERVER_BUDGET_MS, 180_000); + assert.equal(DEFAULT_TIMEOUT_MS, 120_000); + const err = requestTimeout("https://x", DEFAULT_TIMEOUT_MS, DEFAULT_TIMEOUT_MS); + const suggested = Number(/--timeout (\d+)/.exec(err.next_action)?.[1]); + assert.ok(suggested > SERVER_BUDGET_MS, `suggested ${suggested}ms must outlive the ${SERVER_BUDGET_MS}ms budget`); + assert.match(err.likely_cause, /may still finish this request and charge it/); }); // Encodes: "Test covering a simulated abort asserting it is not BACKEND_UNAVAILABLE." @@ -92,7 +96,7 @@ test("a genuine transport failure is still BACKEND_UNAVAILABLE, with the elapsed const failing = (() => Promise.reject(new TypeError("fetch failed"))) as unknown as typeof fetch; await withFetchImpl(failing, async () => { await assert.rejects( - () => scrape("https://api.zenrows.com/v1/", "k", { url: "https://x" }), + () => scrape("https://api.zenrows.com/v1/", "k", { url: "https://x" }, { retries: 0 }), (err: unknown) => { const e = err as ToolkitError; assert.equal(e.code, "BACKEND_UNAVAILABLE"); From 4104b09eeeab2e7f961bc26d108b5cb0eba4b537 Mon Sep 17 00:00:00 2001 From: gonzalo-zenrows Date: Mon, 19 Oct 2026 22:49:20 +0200 Subject: [PATCH 2/5] fix(fetch): retry only what cannot be charged twice, and send agents back to the command they ran The retry change resent requests the API may already be working on, retried pages the target site sent, and told agents to run a different command than the one that failed. - Retry a network error only when the request never reached the API (refused connection, DNS failure, connect timeout, unreachable network). A connection that closes after the request was sent is not retried: the API may still finish and charge it, as with a timeout. - Do not retry a 429, 502, 503 or 504 under original_status, or when allowed_status_codes covers it: that status may be the target site's page. - Decide a 429 by its error code: AUTH004 and AUTH014 are not retried, any other 429 is, including a non-JSON page. - Read Retry-After as whole digits clamped to 0.25-30 s, so 0 cannot drive a tight loop. Crawl keeps its own reading. --retries takes whole decimal digits only, so 1e1, 0x2 and padded values are usage errors. - Extract's Autoparse fallback shares the first request's 90 s budget. - Raise the default client timeout to 200 s, above the API's 180 s budget, as the public docs advise, and restore the test that guards it. - SERVICE_BUSY and the final error after a retried 429, 502, 503 or 504 suggest the command that ran, with its flags and extract method, and no longer advise the most expensive configuration. On Extract, SERVICE_BUSY quotes the API's own title, which says which of RESP009's two meanings applies. - --json results report attempts, the number of requests sent. --- README.md | 29 ++- skills/protected-fetch/SKILL.md | 10 +- skills/trace-debug/SKILL.md | 23 +- src/adapters/extract.ts | 7 +- src/adapters/protected-fetch.ts | 21 +- src/cli/commands/extract.ts | 6 +- src/cli/commands/fetch.ts | 28 ++- src/core/backoff.ts | 35 ++- src/core/http.ts | 284 ++++++++++++++++----- tests/fetch-retry-command.test.ts | 206 +++++++++++++++ tests/fetch-retry.test.ts | 405 ++++++++++++++++++++++++++++-- tests/fetch-timeout.test.ts | 21 +- 12 files changed, 948 insertions(+), 127 deletions(-) create mode 100644 tests/fetch-retry-command.test.ts diff --git a/README.md b/README.md index 4cb586e..d6c1aa8 100644 --- a/README.md +++ b/README.md @@ -115,12 +115,29 @@ zenrows fetch --proxy-country us --wait-for ".price" zenrows fetch --retries 0 # send once; the default is 2 retries ``` -`fetch` and `extract` retry a 429, 502, 503 or 504 answer and a network error -up to `--retries` times (default 2). Each wait honours `Retry-After` (at most -30 s) or is a random delay that grows with each retry, and no retry starts -after 90 s in total. None of these statuses is charged. A client timeout -(`--timeout`) is never retried, because the API may still finish and charge -that request. +`fetch` and `extract` retry a 429, 502, 503 or 504 answer up to `--retries` +times (default 2; `--retries 0` sends once). They also retry a network error +that happened before the request reached the API: a refused connection, a name +that did not resolve, a connect timeout or an unreachable network. Each wait +honours `Retry-After` in whole seconds (at least 0.25 s, at most 30 s) or is a +random delay that grows with each retry. No retry starts after 90 s, counted +from the command's first request; Extract's fallback to Autoparse shares that +clock. A running request is not cut short, so a command can take longer than +90 s. None of the retried statuses is charged. The CLI has no cancel option of +its own; Ctrl-C ends the command, including a wait between retries. + +The CLI does not retry: + +1. A client timeout (`--timeout`, default 200000 ms, above the API's own 180 s + budget). The API may still finish and charge that request. +2. A connection that closed after the request was sent, for the same reason. +3. A 429 whose code says the account or the API key is out of credits + (`AUTH004`, `AUTH014`). +4. A status the target site sent, under `--original-status`. + +With `--json`, `attempts` in the result says how many HTTP requests the command +sent (1 means no retry). When the retries run out, the error suggests the same +command again rather than a costlier configuration. ## 8. Extract diff --git a/skills/protected-fetch/SKILL.md b/skills/protected-fetch/SKILL.md index 5b9148f..608fe17 100644 --- a/skills/protected-fetch/SKILL.md +++ b/skills/protected-fetch/SKILL.md @@ -47,9 +47,13 @@ zenrows fetch --manual --js-render --premium-proxy # the most expensive needs `--premium-proxy` (else `PARAM_PROXY_COUNTRY_REQUIRES_PREMIUM`). - You are billed only for the configuration that succeeds. - On failure, read `zenrows trace explain ` before escalating. -- The CLI already retries 429, 502, 503 and 504 answers and network errors - (`--retries`, default 2; `--retries 0` sends once). Do not wrap it in your own - retry loop. +- The CLI already retries 429, 502, 503 and 504 answers, and network errors + that happened before the request reached the API (`--retries`, default 2; + `--retries 0` sends once). No retry starts after 90 s. Do not wrap it in your + own retry loop. `--json` reports `attempts`, the number of requests sent. +- The CLI does not retry a timeout, a connection that closed after the request + was sent, an out-of-credits 429 (`AUTH004`, `AUTH014`), or a status the target + sent under `--original-status`. Every run is saved under `.zenrows/runs//` (no secrets). See [[extract]] for structured output and [[cost-control]] for the cost model. diff --git a/skills/trace-debug/SKILL.md b/skills/trace-debug/SKILL.md index b93165f..bfa88f8 100644 --- a/skills/trace-debug/SKILL.md +++ b/skills/trace-debug/SKILL.md @@ -26,17 +26,28 @@ zenrows trace export # JSON for sharing `--manual --js-render` run; auto mode may ignore it. Each step multiplies the cost of the request, and both together are the most expensive configuration available ([[cost-control]]). +- `FETCH_FAILED` after HTTP 429, 502, 503 or 504 → a temporary answer the CLI + already retried (`--retries`, default 2). Wait a minute and run the suggested + command, which is the same command. Do not escalate: a costlier fetch + configuration does not fix it. Under `--original-status` the status may be the + target site's own, and the CLI sends it once. - `REQUEST_TIMEOUT` → the CLI stopped waiting; the API was reached. Raise - `--timeout` (default 120000ms; the API's own budget is 180s), or drop + `--timeout` (default 200000ms, above the API's own 180s budget), or drop `--wait-for` so the request finishes inside that budget and the API returns its own error. Not a connectivity problem — do not chase the network. The CLI never retries a timeout on its own: the API may still finish and charge it. -- `SERVICE_BUSY` → HTTP 503 `RESP009`: Zenrows had no capacity for the request - yet, or Extract is still preparing this layout. Not charged. The CLI already - retried it (`--retries`, default 2); run the same command again in about a - minute. Do not escalate to `--js-render` / `--premium-proxy`. +- `SERVICE_BUSY` → HTTP 503 `RESP009`: Zenrows could not take the request now. + Not charged. On Extract, `likely_cause` quotes the API's title, which says + either that Extract could not finish this page right now (retry in about a + minute) or that it is at capacity for pages it has not seen before (retry + later). The CLI already retried it (`--retries`, default 2); wait, then run + the suggested command, which is the command that ran. Do not escalate to + `--js-render` / `--premium-proxy`. - `BACKEND_UNAVAILABLE` → a genuine transport failure (DNS/TCP/TLS). Check - connectivity and `zenrows config show`. + connectivity and `zenrows config show`. The CLI retries it only when the + request never reached the API (refused connection, DNS, connect timeout). A + connection that closed after the request was sent is not retried, because the + API may still finish and charge that request. - `AUTH_INVALID` → re-check the key, `zenrows login --api-key …`. - `KEY_CREDIT_CAP_REACHED` → this API key hit one of its own credit caps (HTTP 402, gateway code `AUTH014`, Batch `api_key_cap_reached`). The account still diff --git a/src/adapters/extract.ts b/src/adapters/extract.ts index 0876e96..ef4aff4 100644 --- a/src/adapters/extract.ts +++ b/src/adapters/extract.ts @@ -16,6 +16,7 @@ import type { Policy, ToolkitConfig } from "../types/index.ts"; import { ToolkitError } from "../core/errors.ts"; import { runFetch, type FetchOptions, type FetchOutcome } from "./protected-fetch.ts"; +import { newRetryBudget } from "../core/http.ts"; export type ExtractMethod = "extract" | "autoparse" | "css" | "outputs" | "markdown" | "plaintext"; @@ -91,11 +92,13 @@ export async function runExtract( } if (method === "extract" && opts.fallbackAutoparse !== false) { + // One budget for the call: the fallback does not restart the 90 s clock. + const budget = opts.budget ?? newRetryBudget(); try { - return await runExtractOnce({ ...opts, method: "extract" }, config, policy, apiKey); + return await runExtractOnce({ ...opts, budget, method: "extract" }, config, policy, apiKey); } catch (err) { if (err instanceof ToolkitError && err.code === "EXTRACT_DOMAIN_NOT_ENABLED") { - const outcome = await runExtractOnce({ ...opts, method: "autoparse" }, config, policy, apiKey); + const outcome = await runExtractOnce({ ...opts, budget, method: "autoparse" }, config, policy, apiKey); return { ...outcome, fellBackToAutoparse: true }; } throw err; diff --git a/src/adapters/protected-fetch.ts b/src/adapters/protected-fetch.ts index ad403c1..edd9f54 100644 --- a/src/adapters/protected-fetch.ts +++ b/src/adapters/protected-fetch.ts @@ -10,7 +10,7 @@ import type { Policy, ToolkitConfig } from "../types/index.ts"; import { ToolkitError } from "../core/errors.ts"; import { assertDomainAllowed } from "../core/policy.ts"; -import { scrape, type ScraperParams, type ScraperResult } from "../core/http.ts"; +import { scrape, type RetryBudget, type ScraperParams, type ScraperResult } from "../core/http.ts"; export type ResponseFormat = "html" | "markdown" | "plaintext" | "pdf"; @@ -56,6 +56,17 @@ export interface FetchOptions { * Not an API parameter. Unset means `http.ts`'s default; 0 sends the request once. */ retries?: number; + /** + * The retry budget of the logical call this request belongs to. Not an API + * parameter. Extract's Autoparse fallback passes the first request's budget, + * so no retry starts more than 90 s after the call's first request. + */ + budget?: RetryBudget; + /** + * The command the user ran, quoted for a shell. Not an API parameter. + * Errors that advise running the same command again suggest it verbatim. + */ + rerunCommand?: string; } const RESPONSE_TYPE: Partial> = { @@ -157,6 +168,12 @@ export async function runFetch( validateAutoManual(opts, config); const params = buildParams(opts, config); const mode: "auto" | "manual" = params.mode === "auto" ? "auto" : "manual"; - const result = await scrape(config.apiBase, apiKey, params, { timeoutMs: opts.timeoutMs, retries: opts.retries, projectRoot }); + const result = await scrape(config.apiBase, apiKey, params, { + timeoutMs: opts.timeoutMs, + retries: opts.retries, + projectRoot, + budget: opts.budget, + rerunCommand: opts.rerunCommand, + }); return { result, params, mode }; } diff --git a/src/cli/commands/extract.ts b/src/cli/commands/extract.ts index ebca1b7..15ae10f 100644 --- a/src/cli/commands/extract.ts +++ b/src/cli/commands/extract.ts @@ -18,7 +18,7 @@ import { formatRequestCost } from "../../core/http.ts"; import { parse, asString, asNumber, type Command, type RunContext } from "../command.ts"; import { printError, writeOut } from "../output.ts"; import { refreshClaimAdvice } from "../../core/claim-link.ts"; -import { normalizeRetries } from "./fetch.ts"; +import { normalizeRetries, rerunCommand } from "./fetch.ts"; export const extract: Command = { name: "extract", @@ -36,7 +36,7 @@ export const extract: Command = { "Shared:", " --manual --js-render --premium-proxy manual fetch controls", " --validate fail if the result is not valid JSON", - " --retries retries on 429/502/503/504 or a network error (default 2; 0 disables)", + " --retries retries on 429/502/503/504 or a connect/DNS error (default 2; 0 disables)", " --out write output to a file", " --no-signup do not auto-create a Free plan account if no key exists", " --json structured result", @@ -109,6 +109,7 @@ export const extract: Command = { waitFor: asString(values["wait-for"]), validate: values.validate === true, retries: normalizeRetries(values.retries), + rerunCommand: rerunCommand("extract", argv, ctx), }; assertUsable("extract"); @@ -195,6 +196,7 @@ export const extract: Command = { bytes: outcome.result.body.length, costUsd: outcome.result.costUsd, costCredits: outcome.result.costCredits, + attempts: outcome.result.attempts ?? 1, }, null, 2, diff --git a/src/cli/commands/fetch.ts b/src/cli/commands/fetch.ts index f5e6d16..265edd2 100644 --- a/src/cli/commands/fetch.ts +++ b/src/cli/commands/fetch.ts @@ -13,7 +13,7 @@ import { log } from "../../core/logger.ts"; import { newRunId, writeRun } from "../../core/artifacts.ts"; import { ToolkitError } from "../../core/errors.ts"; import { runFetch, type FetchOptions, type ResponseFormat } from "../../adapters/protected-fetch.ts"; -import { formatRequestCost } from "../../core/http.ts"; +import { formatRequestCost, shellQuote } from "../../core/http.ts"; import { parse, asString, asNumber, type Command, type RunContext } from "../command.ts"; import { printError, writeOut } from "../output.ts"; import { refreshClaimAdvice } from "../../core/claim-link.ts"; @@ -36,8 +36,8 @@ export const fetch_: Command = { " --output html (default) | markdown | text | pdf", " --screenshot capture an above-the-fold screenshot", " --out write the response body to a file", - " --timeout client-side timeout per attempt (default 120000; the API's own budget is 180s)", - " --retries retries on 429/502/503/504 or a network error (default 2; 0 disables)", + " --timeout client-side timeout per attempt (default 200000; the API's own budget is 180s)", + " --retries retries on 429/502/503/504 or a connect/DNS error (default 2; 0 disables)", " --no-signup do not auto-create a Free plan account if no key exists", " --json print a structured result", "", @@ -104,6 +104,7 @@ export const fetch_: Command = { screenshot: values.screenshot === true, timeoutMs: normalizeTimeout(values.timeout), retries: normalizeRetries(values.retries), + rerunCommand: rerunCommand("fetch", argv, ctx), }; const runId = newRunId(); @@ -158,7 +159,7 @@ export const fetch_: Command = { if (runDir) log.dim(` artifact: ${runDir}`); if (ctx.json || values.json) { - log.out(JSON.stringify({ ok: true, runId, mode, httpStatus: result.status, bytes, costUsd: result.costUsd, costCredits: result.costCredits, requestId: result.requestId, finalUrl: result.finalUrl }, null, 2)); + log.out(JSON.stringify({ ok: true, runId, mode, httpStatus: result.status, bytes, costUsd: result.costUsd, costCredits: result.costCredits, requestId: result.requestId, finalUrl: result.finalUrl, attempts: result.attempts ?? 1 }, null, 2)); } else if (!values.out) { if (result.isBinary) { // Never dump raw binary to the terminal — it corrupts the session and @@ -213,13 +214,14 @@ export function normalizeTimeout(v: unknown): number | undefined { } /** - * `--retries `: a whole number, 0 or more. Like `--timeout`, a bad value is - * an error, never a silent fallback to the default. + * `--retries `: whole decimal digits, 0 or more. Like `--timeout`, a bad + * value is an error, never a silent fallback to the default. `1e1`, `0x2`, + * `+1` and values with spaces are rejected, although `Number()` would read them. */ export function normalizeRetries(v: unknown): number | undefined { if (v === undefined) return undefined; - const n = asNumber(v); - if (n === undefined || !Number.isInteger(n) || n < 0) { + const n = typeof v === "string" && /^\d+$/.test(v) ? Number(v) : undefined; + if (n === undefined || !Number.isSafeInteger(n)) { throw new ToolkitError({ code: "INVALID_USAGE", message: `Invalid --retries value '${String(v)}'.`, @@ -231,6 +233,16 @@ export function normalizeRetries(v: unknown): number | undefined { return n; } +/** + * The command the user ran, quoted for a shell, for errors whose advice is + * "run the same command again". The router strips a global `--json`, so it is + * added back when it was set. + */ +export function rerunCommand(name: string, argv: string[], ctx: RunContext): string { + const args = ctx.json && !argv.includes("--json") ? [...argv, "--json"] : argv; + return ["zenrows", name, ...args.map(shellQuote)].join(" "); +} + export function normalizeOutput(v?: string): ResponseFormat | undefined { if (!v) return undefined; const map: Record = { diff --git a/src/core/backoff.ts b/src/core/backoff.ts index d8ff54a..b58c71f 100644 --- a/src/core/backoff.ts +++ b/src/core/backoff.ts @@ -1,9 +1,10 @@ /** * Retry delays shared by the HTTP clients. * - * Crawl keeps its own schedule (`crawlBackoffMs`). Fetch and Extract use full - * jitter (`fullJitterMs`), the policy every Zenrows Fetch/Extract client shares. - * Both honour a `Retry-After` in whole seconds through `retryAfterMs`. + * Crawl keeps its own schedule (`crawlBackoffMs`) and reads `Retry-After` + * through `retryAfterMs`. Fetch and Extract use full jitter (`fullJitterMs`) + * and read `Retry-After` through `retryAfterDelayMs`, which also applies the + * 250 ms floor: the policy every Zenrows Fetch/Extract client shares. */ /** The longest a Retry-After may make one retry wait, so a large server value cannot hang a command. */ @@ -14,24 +15,38 @@ export function crawlBackoffMs(attempt: number): number { return Math.min(250 * 2 ** attempt, 10_000) * (0.8 + Math.random() * 0.4); } -/** Shortest full-jitter wait: never retry in a tight loop. */ +/** Shortest Fetch/Extract wait, from jitter or from Retry-After: never retry in a tight loop. */ export const JITTER_FLOOR_MS = 250; /** Longest full-jitter wait. */ export const JITTER_CAP_MS = 30_000; /** - * Full jitter: uniform(0.25 s, min(30 s, 1 s x 2^attempt)). `attempt` is the - * 0-based retry index, so the first retry waits at most 1 s and the second 2 s. - * `random` is injectable for tests. + * Full jitter: uniform(0.25 s, max(0.25 s, min(capMs, 1 s x 2^attempt))). + * `attempt` is the 0-based retry index, so the first retry waits at most 1 s + * and the second 2 s. `random` is injectable for tests. `capMs` is 30 s; the + * CLI has no option to change it, and the parameter exists for the shared + * conformance vectors. */ -export function fullJitterMs(attempt: number, random: () => number = Math.random): number { - const ceiling = Math.max(JITTER_FLOOR_MS, Math.min(JITTER_CAP_MS, 1000 * 2 ** attempt)); +export function fullJitterMs(attempt: number, random: () => number = Math.random, capMs: number = JITTER_CAP_MS): number { + const ceiling = Math.max(JITTER_FLOOR_MS, Math.min(capMs, 1000 * 2 ** attempt)); return JITTER_FLOOR_MS + random() * (ceiling - JITTER_FLOOR_MS); } -/** `Retry-After` in whole seconds, as milliseconds, at most `MAX_RETRY_AFTER_SECONDS`. */ +/** + * `Retry-After` in whole seconds, as milliseconds, at most `MAX_RETRY_AFTER_SECONDS`. + * Crawl's reading: `0` means no wait. Anything but whole decimal digits is undefined. + */ export function retryAfterMs(value: string | null): number | undefined { return value && /^\d+$/.test(value.trim()) ? Math.min(Number(value), MAX_RETRY_AFTER_SECONDS) * 1000 : undefined; } +/** + * Fetch and Extract's reading of `Retry-After`: whole decimal digits only, + * clamped to [0.25 s, 30 s], so `0` waits 250 ms. Undefined means "use jitter". + */ +export function retryAfterDelayMs(value: string | null): number | undefined { + const ms = retryAfterMs(value); + return ms === undefined ? undefined : Math.max(JITTER_FLOOR_MS, ms); +} + export const defaultSleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); diff --git a/src/core/http.ts b/src/core/http.ts index 2f530cd..3754a5e 100644 --- a/src/core/http.ts +++ b/src/core/http.ts @@ -12,7 +12,7 @@ import { ENV_KEY, resolveApiKey } from "./auth.ts"; import { log, registerSecret } from "./logger.ts"; import { CLI_USER_AGENT } from "./config.ts"; import { agentClientHeader } from "./agent-client.ts"; -import { defaultSleep, fullJitterMs, retryAfterMs } from "./backoff.ts"; +import { defaultSleep, fullJitterMs, retryAfterDelayMs, retryAfterMs } from "./backoff.ts"; export interface ScraperResult { status: number; @@ -45,6 +45,12 @@ export interface ScraperResult { * The request is not charged, and the same request returns the same answer. */ noDataExtracted?: boolean; + /** + * HTTP requests sent for this logical call, the first one included (1 means + * no retry). With a shared `RetryBudget` it counts every request in it, such + * as Extract's request plus its Autoparse fallback. + */ + attempts?: number; } export interface ScraperParams { @@ -55,15 +61,15 @@ export interface ScraperParams { /** * Client-side timeout for one Fetch and Extract HTTP attempt. * - * It was raised from 90s when the gateway's budget was believed to be 90s, so - * that our abort would not race the API's own error envelope. The gateway's - * real budget is `SERVER_BUDGET_MS` (180s), so a request that runs longer than - * this can still be aborted while the API is working on it. That is why a - * timeout is never retried: the API keeps running and may still charge a 200. + * It sits above the API's own budget (`SERVER_BUDGET_MS`, 180 s), as the public + * docs advise, so the API normally answers with its own error before the CLI + * gives up. Giving up earlier would abandon a request the API keeps working on + * and may still charge, while it holds a concurrency slot. A timeout is still + * never retried, for the same reason. */ -export const DEFAULT_TIMEOUT_MS = 120_000; +export const DEFAULT_TIMEOUT_MS = 200_000; -/** The gateway's budget for the whole request: auth, fetch and parse. */ +/** The API's budget for the whole request: auth, fetch and parse. */ export const SERVER_BUDGET_MS = 180_000; /** Retries for a retryable failure (`--retries`). 0 sends the request once. */ @@ -73,11 +79,48 @@ export const DEFAULT_RETRIES = 2; export const RETRY_BUDGET_MS = 90_000; /** - * Statuses that can succeed on the next attempt. The gateway charges only - * 2xx, 304, 404 and 410, so none of these is charged. + * Statuses that can succeed on the next attempt. Zenrows charges only + * successful answers, so none of these is charged. */ const RETRY_STATUSES: ReadonlySet = new Set([429, 502, 503, 504]); +/** + * Error codes that say the account or key is out of credits (402/429 `AUTH004`, + * `AUTH014`, per docs.zenrows.com/api-error-codes). A 429 with one of these + * answers the same until the account changes, so it is not retried. + */ +const QUOTA_CODES: ReadonlySet = new Set(["AUTH004", "AUTH014"]); + +/** + * `error.cause.code` values that mean the request was never written to the + * API: the connection was refused, the name did not resolve, the connect timed + * out or the network was unreachable. Only these network errors are retried. + * A reset or close after the request was sent (`UND_ERR_SOCKET`, `ECONNRESET`) + * is not: the API may still be working on it and may charge it. + */ +const NEVER_SENT_CODES: ReadonlySet = new Set([ + "ECONNREFUSED", + "ENOTFOUND", + "EAI_AGAIN", + "ENETUNREACH", + "EHOSTUNREACH", + "UND_ERR_CONNECT_TIMEOUT", +]); + +/** + * The start time and request count of one logical call. Extract's Autoparse + * fallback shares the first request's budget, so its retries cannot restart + * the 90 s clock. + */ +export interface RetryBudget { + startedAt: number; + attempts: number; +} + +export function newRetryBudget(now: () => number = Date.now): RetryBudget { + return { startedAt: now(), attempts: 0 }; +} + export interface ScrapeOptions { /** Per HTTP attempt. */ timeoutMs?: number; @@ -90,6 +133,13 @@ export interface ScrapeOptions { nowImpl?: () => number; /** Source of the jitter. */ randomImpl?: () => number; + /** A budget shared with an earlier request of the same call; unset starts a new one. */ + budget?: RetryBudget; + /** + * The command the user ran, ready to paste. Errors that say "run the same + * command again" suggest it. Unset means `zenrows fetch|extract `. + */ + rerunCommand?: string; } function buildUrl(apiBase: string, apiKey: string, params: ScraperParams): { full: string; redacted: string } { @@ -108,10 +158,12 @@ function buildUrl(apiBase: string, apiKey: string, params: ScraperParams): { ful /** * Perform a Fetch and Extract API request. Throws ToolkitError on failure. * - * Retries 429, 502, 503 and 504 and network errors that came before any - * response, up to `retries` times, with full jitter or the server's - * Retry-After. A retry never starts past `RETRY_BUDGET_MS`. Our own timeout is - * never retried, and neither is any other status. + * Retries 429, 502, 503 and 504, and network errors that happened before the + * request was written, up to `retries` times, with full jitter or the server's + * Retry-After. No retry starts after `RETRY_BUDGET_MS` from the first request + * of the call. Our own timeout is never retried, and neither is any other + * status, a 429 out-of-credits code, or a status that belongs to the target + * site (`original_status`, `allowed_status_codes`). */ export async function scrape( apiBase: string, @@ -124,50 +176,99 @@ export async function scrape( const retries = opts.retries ?? DEFAULT_RETRIES; const sleep = opts.sleepImpl ?? defaultSleep; const now = opts.nowImpl ?? Date.now; - const startedAt = now(); + const budget = opts.budget ?? newRetryBudget(now); for (let n = 0; ; n++) { - let sent: { res: Response; raw: Buffer }; - try { - sent = await sendOnce(full, params.url, opts); - } catch (err) { - // REQUEST_TIMEOUT is not retried: the API keeps working and may still charge. - if (n >= retries || !(err instanceof ToolkitError) || err.code !== "BACKEND_UNAVAILABLE") throw err; + budget.attempts++; + const sent = await sendOnce(full, params.url, opts); + if (!sent.ok) { + // A timeout, or a failure after the request was written, is not retried: + // the API may still be working on it and may charge it. + if (n >= retries || !sent.neverSent) throw sent.error; const delayMs = fullJitterMs(n, opts.randomImpl); - if (now() - startedAt + delayMs > RETRY_BUDGET_MS) throw err; - log.dim(`Network error; retry ${n + 1}/${retries} in ${formatMs(delayMs)}`); + if (now() - budget.startedAt + delayMs > RETRY_BUDGET_MS) throw sent.error; + log.dim(`Network error before the request was sent; retry ${n + 1}/${retries} in ${formatMs(delayMs)}`); await sleep(delayMs); continue; } const body = sent.raw.toString("utf8"); - if (n < retries && isRetryable(sent.res.status, body)) { - const delayMs = retryAfterMs(sent.res.headers.get("retry-after")) ?? fullJitterMs(n, opts.randomImpl); - if (now() - startedAt + delayMs <= RETRY_BUDGET_MS) { + if (n < retries && isRetryable(sent.res.status, body, params)) { + const delayMs = retryAfterDelayMs(sent.res.headers.get("retry-after")) ?? fullJitterMs(n, opts.randomImpl); + if (now() - budget.startedAt + delayMs <= RETRY_BUDGET_MS) { const code = zrErrorCode(body); log.dim(`HTTP ${sent.res.status}${code ? ` ${code}` : ""}; retry ${n + 1}/${retries} in ${formatMs(delayMs)}`); await sleep(delayMs); continue; } } - return interpret(sent.res, sent.raw, params, redacted, n); + return interpret(sent.res, sent.raw, params, redacted, { + retriesUsed: n, + attempts: budget.attempts, + rerunCommand: opts.rerunCommand ?? defaultRerunCommand(params), + }); } } /** - * True for a status worth another attempt. A 429 that says the account is out - * of credits is excluded: it answers the same until the account changes. + * True for a status worth another attempt. Not retried: a 429 whose code says + * the account or key is out of credits, and any status the target site sent. */ -function isRetryable(status: number, body: string): boolean { +function isRetryable(status: number, body: string, params: ScraperParams): boolean { if (!RETRY_STATUSES.has(status)) return false; - return !(status === 429 && isQuotaError(body)); + if (isTargetStatus(status, params)) return false; + return !(status === 429 && QUOTA_CODES.has(zrErrorCode(body) ?? "")); +} + +/** + * True when `status` may be the target site's own answer, passed through by + * `original_status` or `allowed_status_codes`. Retrying it would hit the + * target again, and the API may charge a request that reached the target. + */ +function isTargetStatus(status: number, params: ScraperParams): boolean { + if (params.original_status === true || params.original_status === "true") return true; + const allowed = params.allowed_status_codes; + if (allowed === undefined || allowed === false) return false; + return String(allowed) + .split(",") + .some((code) => code.trim() === String(status)); +} + +/** + * True when a fetch() rejection means the request never reached the API. + * undici wraps every failure as `TypeError("fetch failed")` with the real + * error in `cause`; a refused connection to a host with several addresses + * arrives as an AggregateError whose `errors` all carry a code. + */ +export function failedBeforeSend(err: unknown): boolean { + return neverSentCause(err instanceof Error ? err.cause : undefined); +} + +function neverSentCause(cause: unknown): boolean { + if (!cause || typeof cause !== "object") return false; + const code = (cause as { code?: unknown }).code; + if (typeof code === "string") return NEVER_SENT_CODES.has(code); + const errors = (cause as { errors?: unknown }).errors; + return Array.isArray(errors) && errors.length > 0 && errors.every(neverSentCause); +} + +/** Shell-safe form of one argument for a suggested command. */ +export function shellQuote(arg: string): string { + return /^[A-Za-z0-9_@%+=:,./-]+$/.test(arg) ? arg : `'${arg.replace(/'/g, `'\\''`)}'`; } +/** `zenrows fetch|extract ` for a caller that did not pass the command it ran. */ +function defaultRerunCommand(params: ScraperParams): string { + const isExtract = + params.extract !== undefined || params.autoparse !== undefined || params.css_extractor !== undefined || params.outputs !== undefined; + return `zenrows ${isExtract ? "extract" : "fetch"} ${shellQuote(params.url)}`; +} + +type SendResult = + | { ok: true; res: Response; raw: Buffer } + | { ok: false; error: ToolkitError; neverSent: boolean }; + /** One HTTP attempt with its own timeout; the body is read inside it. */ -async function sendOnce( - full: string, - targetUrl: string, - opts: ScrapeOptions, -): Promise<{ res: Response; raw: Buffer }> { +async function sendOnce(full: string, targetUrl: string, opts: ScrapeOptions): Promise { const timeoutMs = opts.timeoutMs ?? DEFAULT_TIMEOUT_MS; const controller = new AbortController(); // Our own timer is the only thing that aborts this controller, so this flag — @@ -190,15 +291,20 @@ async function sendOnce( } catch (err) { clearTimeout(timeout); const elapsedMs = Date.now() - startedAt; - if (timedOut) throw requestTimeout(targetUrl, timeoutMs, elapsedMs); + if (timedOut) return { ok: false, error: requestTimeout(targetUrl, timeoutMs, elapsedMs), neverSent: false }; const cause = err instanceof Error ? err.message : String(err); - throw new ToolkitError({ - code: "BACKEND_UNAVAILABLE", - message: `Could not reach the Zenrows API.`, - likely_cause: `Network error after ${formatMs(elapsedMs)}: ${cause}`, - next_action: "Check connectivity and retry. Verify api base in `zenrows config show`.", - suggested_commands: ["zenrows status", "zenrows config show"], - }); + const causeCode = err instanceof Error ? (err.cause as { code?: unknown } | undefined)?.code : undefined; + return { + ok: false, + neverSent: failedBeforeSend(err), + error: new ToolkitError({ + code: "BACKEND_UNAVAILABLE", + message: `Could not reach the Zenrows API.`, + likely_cause: `Network error after ${formatMs(elapsedMs)}: ${cause}${typeof causeCode === "string" ? ` (${causeCode})` : ""}`, + next_action: "Check connectivity and retry. Verify api base in `zenrows config show`.", + suggested_commands: ["zenrows status", "zenrows config show"], + }), + }; } clearTimeout(timeout); @@ -206,7 +312,17 @@ async function sendOnce( // output; `raw` preserves the exact bytes so binary responses (screenshot / // PDF) can be written without the UTF-8 round-trip that corrupts them. const raw = Buffer.from(await res.arrayBuffer()); - return { res, raw }; + return { ok: true, res, raw }; +} + +/** What `interpret` needs to know about the attempts behind a final response. */ +interface AttemptContext { + /** Retries this request made before the final response. */ + retriesUsed: number; + /** Requests sent for the whole call (see `ScraperResult.attempts`). */ + attempts: number; + /** The command to suggest when the advice is "run the same command again". */ + rerunCommand: string; } /** Turn one final response into a result, or a normalized error. */ @@ -215,7 +331,7 @@ function interpret( raw: Buffer, params: ScraperParams, redacted: string, - retriesUsed: number, + attempt: AttemptContext, ): ScraperResult { const body = raw.toString("utf8"); // A binary body is only ever produced by a successful screenshot/PDF request. @@ -234,6 +350,7 @@ function interpret( finalUrl: res.headers.get("zr-final-url"), concurrencyRemaining: numOrNull(res.headers.get("concurrency-remaining")), redactedUrl: redacted, + attempts: attempt.attempts, }; // REQS007 on extract=auto: the domain hasn't been prepared for Extract yet. @@ -345,12 +462,12 @@ function interpret( const detail = zrErrorDetail(body) ?? snippet(body); throw new ToolkitError({ code: "FETCH_FAILED", - message: "Rate limited (HTTP 429).", + message: `Rate limited (HTTP 429${triedText(attempt.retriesUsed)}).`, likely_cause: detail ? `${detail}. This looks like a concurrency cap or a target-site rate limit, not Free-plan credit exhaustion.` : "A concurrency cap or a target-site rate limit was hit, not Free-plan credit exhaustion.", next_action: "Wait a few seconds and retry. If it persists, lower concurrency / parallel requests.", - suggested_commands: [`zenrows fetch ${params.url}`], + suggested_commands: [attempt.rerunCommand], }); } if (isForbiddenDomain(body)) { @@ -385,7 +502,19 @@ function interpret( : "Verify the URL. Rendering or premium proxies will not turn a 404 into content — do not retry; it will fail identically and still cost credits.", }); } - if (res.status === 503 && zrErrorCode(body) === "RESP009") throw serviceBusy(params, body, res.headers.get("retry-after"), retriesUsed); + if (res.status === 503 && zrErrorCode(body) === "RESP009") throw serviceBusy(params, body, res.headers.get("retry-after"), attempt); + if (RETRY_STATUSES.has(res.status) && !isTargetStatus(res.status, params)) { + // A temporary answer from the API side that outlasted the retries. A + // stronger (and up to 25x dearer) fetch configuration does not fix it. + throw new ToolkitError({ + code: "FETCH_FAILED", + message: `The Zenrows API answered HTTP ${res.status}${triedText(attempt.retriesUsed)}. It was not charged.`, + likely_cause: `${snippet(body) || `HTTP ${res.status}`}. The Zenrows API gave a temporary answer, and the CLI already retried it.`, + next_action: + "Wait a minute, then run the same command again. Do not add --js-render or --premium-proxy: they cost more and do not help here.", + suggested_commands: [attempt.rerunCommand], + }); + } if (res.status === 422 || res.status >= 500 || (res.status >= 400 && !looksLikeContent(result))) { throw new ToolkitError({ code: "FETCH_FAILED", @@ -408,28 +537,42 @@ function interpret( return result; } +/** ` after 2 retries`, or nothing when the request was sent once. */ +function triedText(retriesUsed: number): string { + return retriesUsed > 0 ? ` after ${retriesUsed} ${retriesUsed === 1 ? "retry" : "retries"}` : ""; +} + /** - * 503 RESP009: Zenrows had no capacity for this request yet. It is temporary - * and not charged, so the fix is to wait and send the same request, not to + * 503 RESP009: Zenrows could not take this request now. It is temporary and + * not charged, so the fix is to wait and send the same command, not to * escalate to --js-render / --premium-proxy (which costs more and helps nothing). + * + * On Extract the docs give RESP009 two meanings, and the answer's `title` says + * which: Extract could not finish this page right now (retry in a minute), or + * Extract is at capacity for pages it has not seen before (retry later). The + * cause therefore quotes the API's own words instead of guessing. */ -function serviceBusy(params: ScraperParams, body: string, retryAfter: string | null, retriesUsed: number): ToolkitError { +function serviceBusy(params: ScraperParams, body: string, retryAfter: string | null, attempt: AttemptContext): ToolkitError { const isExtract = params.extract !== undefined; - const command = `zenrows ${isExtract ? "extract" : "fetch"} ${params.url}`; - const detail = zrErrorDetail(body); - const tried = retriesUsed > 0 ? ` after ${retriesUsed} ${retriesUsed === 1 ? "retry" : "retries"}` : ""; + const said = zrErrorTitleAndDetail(body); const waitSeconds = retryAfterMs(retryAfter); + const cause = said + ? `The API said: ${said}` + : isExtract + ? "Extract could not take this request now: either it could not finish this page yet, or it is at capacity for pages it has not seen before." + : "Zenrows scraping capacity is busy right now."; + const when = + waitSeconds !== undefined + ? `Wait at least ${waitSeconds / 1000} s (the API's Retry-After)` + : isExtract + ? "Wait: about a minute if the API says it could not finish this page, longer if it says it is at capacity for new pages" + : "Wait a short time"; return new ToolkitError({ code: "SERVICE_BUSY", - message: `Zenrows could not take this request yet (HTTP 503 RESP009${tried}). It was not charged.`, - likely_cause: - (isExtract - ? "The extraction service is still preparing this layout; the request was not charged." - : "Zenrows scraping capacity is busy right now; the request was not charged.") + - (detail ? ` Server said: ${detail}.` : ""), - next_action: - "Retry the same command shortly (in about a minute). Do not add --js-render or --premium-proxy: they cost more and do not help here.", - suggested_commands: [command], + message: `Zenrows could not take this request yet (HTTP 503 RESP009${triedText(attempt.retriesUsed)}). It was not charged.`, + likely_cause: `${cause.replace(/\.\s*$/, "")}. The request was not charged.`, + next_action: `${when}, then run the same command again. Do not add --js-render or --premium-proxy: they cost more and do not help here.`, + suggested_commands: [attempt.rerunCommand], server_code: "RESP009", retry_after: waitSeconds !== undefined ? waitSeconds / 1000 : undefined, }); @@ -572,6 +715,19 @@ export function zrErrorCode(body: string): string | null { } } +/** `(CODE) title: detail` from a problem body, or null when it has neither. */ +function zrErrorTitleAndDetail(body: string): string | null { + try { + const j = JSON.parse(body) as { code?: unknown; title?: unknown; detail?: unknown }; + const parts = [j.title, j.detail].filter((v): v is string => typeof v === "string" && v.trim() !== ""); + if (parts.length === 0) return null; + const text = parts.map((v) => v.trim().replace(/\.$/, "")).join(": "); + return typeof j.code === "string" && !text.includes(j.code) ? `(${j.code}) ${text}` : text; + } catch { + return null; + } +} + /** The problem body's `detail` field alone, prefixed with its code. */ function zrErrorProblemDetail(body: string): string | null { try { diff --git a/tests/fetch-retry-command.test.ts b/tests/fetch-retry-command.test.ts new file mode 100644 index 0000000..01f9cec --- /dev/null +++ b/tests/fetch-retry-command.test.ts @@ -0,0 +1,206 @@ +/** + * Retry behaviour seen from the `fetch` and `extract` commands: what an agent + * reads on stdout under --json, and which command it is told to run again. + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { fetch_ } from "../src/cli/commands/fetch.ts"; +import { extract } from "../src/cli/commands/extract.ts"; +import { createWorkspace } from "../src/core/workspace.ts"; +import { savePolicy, defaultPolicy } from "../src/core/policy.ts"; +import { saveApiKey } from "../src/core/auth.ts"; +import type { Command } from "../src/cli/command.ts"; +import { tempRoot } from "./helpers.ts"; + +const URL_ = "https://x.com/p"; + +const RESP009 = JSON.stringify({ + code: "RESP009", + status: 503, + title: "Service temporarily at capacity (RESP009)", + type: "https://docs.zenrows.com/api-error-codes#RESP009", +}); + +const AUTH010 = JSON.stringify({ + code: "AUTH010", + status: 402, + title: "Feature is not included in plan (AUTH010)", + type: "https://docs.zenrows.com/api-error-codes#AUTH010", +}); + +type Handler = (url: URL) => Response; + +/** Run `fn` in a fresh workspace with a saved key; every request goes to `handler`. */ +async function inWorkspace(handler: Handler, fn: (calls: URL[]) => Promise): Promise { + const { root, cleanup } = tempRoot(); + const cwd = process.cwd(); + createWorkspace(root); + savePolicy(defaultPolicy(), root); + saveApiKey("0".repeat(41), root); + process.chdir(root); + const calls: URL[] = []; + const orig = globalThis.fetch; + globalThis.fetch = (async (u: string) => { + const url = new URL(u); + calls.push(url); + return handler(url); + }) as unknown as typeof fetch; + try { + await fn(calls); + } finally { + globalThis.fetch = orig; + process.chdir(cwd); + cleanup(); + } +} + +/** Run a command with the global --json flag; returns its exit code and parsed stdout. */ +async function run(cmd: Command, args: string[]): Promise<{ code: number; out: Record }> { + const orig = process.stdout.write.bind(process.stdout); + let buf = ""; + process.stdout.write = ((s: string | Uint8Array, ...rest: unknown[]) => { + if (typeof s !== "string") return (orig as (...a: unknown[]) => boolean)(s, ...rest); + buf += s; + return true; + }) as typeof process.stdout.write; + let code = -1; + try { + code = await cmd.run(args, { json: true, yes: false }); + } finally { + process.stdout.write = orig; + } + return { code, out: JSON.parse(buf) }; +} + +const json = (body: string, status: number) => + new Response(body, { status, headers: { "content-type": "application/json" } }); + +// SERVICE_BUSY says "run the same command again": the suggested command must +// be that command, with its extract method and flags, not `zenrows fetch `. +const BUSY_CASES: Array<{ name: string; cmd: Command; args: string[]; expected: string; param?: [string, string] }> = [ + { + name: "extract (extract=auto)", + cmd: extract, + args: [URL_, "--retries", "0"], + expected: `zenrows extract ${URL_} --retries 0 --json`, + param: ["extract", "auto"], + }, + { + name: "extract --css", + cmd: extract, + args: [URL_, "--css", '{"t":"h1"}', "--retries", "0"], + expected: `zenrows extract ${URL_} --css '{"t":"h1"}' --retries 0 --json`, + param: ["css_extractor", '{"t":"h1"}'], + }, + { + name: "extract --autoparse", + cmd: extract, + args: [URL_, "--autoparse", "--retries", "0"], + expected: `zenrows extract ${URL_} --autoparse --retries 0 --json`, + param: ["autoparse", "true"], + }, + { + name: "extract --outputs", + cmd: extract, + args: [URL_, "--outputs", "emails,links", "--retries", "0"], + expected: `zenrows extract ${URL_} --outputs emails,links --retries 0 --json`, + param: ["outputs", "emails,links"], + }, + { + name: "extract --output md", + cmd: extract, + args: [URL_, "--output", "md", "--retries", "0"], + expected: `zenrows extract ${URL_} --output md --retries 0 --json`, + param: ["response_type", "markdown"], + }, + { + name: "fetch --manual --js-render --premium-proxy", + cmd: fetch_, + args: [URL_, "--manual", "--js-render", "--premium-proxy", "--retries", "0"], + expected: `zenrows fetch ${URL_} --manual --js-render --premium-proxy --retries 0 --json`, + param: ["premium_proxy", "true"], + }, + { + name: "fetch with a URL that needs shell quoting", + cmd: fetch_, + args: ["https://x.com/p?a=1&b=it's", "--retries", "0"], + expected: `zenrows fetch 'https://x.com/p?a=1&b=it'\\''s' --retries 0 --json`, + }, +]; + +for (const c of BUSY_CASES) { + test(`SERVICE_BUSY suggests the command that ran: ${c.name}`, async () => { + await inWorkspace( + () => json(RESP009, 503), + async (calls) => { + const { code, out } = await run(c.cmd, c.args); + assert.equal(code, 1); + assert.equal(out.error.code, "SERVICE_BUSY"); + assert.deepEqual(out.error.suggested_commands, [c.expected]); + assert.equal(calls.length, 1); + if (c.param) assert.equal(calls[0]!.searchParams.get(c.param[0]), c.param[1]); + }, + ); + }); +} + +test("SERVICE_BUSY after the Autoparse fallback suggests the extract command that ran", async () => { + await inWorkspace( + (url) => (url.searchParams.get("extract") ? json(AUTH010, 402) : json(RESP009, 503)), + async (calls) => { + const { code, out } = await run(extract, [URL_, "--retries", "0"]); + assert.equal(code, 1); + assert.equal(out.error.code, "SERVICE_BUSY"); + assert.deepEqual(out.error.suggested_commands, [`zenrows extract ${URL_} --retries 0 --json`]); + assert.deepEqual( + calls.map((u) => (u.searchParams.get("extract") ? "extract" : u.searchParams.get("autoparse") ? "autoparse" : "?")), + ["extract", "autoparse"], + ); + }, + ); +}); + +test("extract --retries 0 sends the request once", async () => { + await inWorkspace( + () => new Response("busy", { status: 503 }), + async (calls) => { + const { code } = await run(extract, [URL_, "--css", '{"t":"h1"}', "--retries", "0"]); + assert.equal(code, 1); + assert.equal(calls.length, 1); + }, + ); +}); + +test("fetch --retries 0 sends the request once", async () => { + await inWorkspace( + () => new Response("busy", { status: 503 }), + async (calls) => { + const { code } = await run(fetch_, [URL_, "--retries", "0"]); + assert.equal(code, 1); + assert.equal(calls.length, 1); + }, + ); +}); + +test("--json reports how many HTTP requests the command sent", async () => { + let n = 0; + await inWorkspace( + () => (++n === 1 ? new Response("", { status: 503, headers: { "retry-after": "0" } }) : new Response("ok", { status: 200 })), + async (calls) => { + const { code, out } = await run(fetch_, [URL_]); + assert.equal(code, 0); + assert.equal(out.ok, true); + assert.equal(out.attempts, 2); + assert.equal(calls.length, 2); + }, + ); + await inWorkspace( + (url) => (url.searchParams.get("extract") ? json(AUTH010, 402) : json(JSON.stringify({ title: "ok" }), 200)), + async () => { + const { code, out } = await run(extract, [URL_]); + assert.equal(code, 0); + assert.equal(out.fellBackToAutoparse, true); + assert.equal(out.attempts, 2); + }, + ); +}); diff --git a/tests/fetch-retry.test.ts b/tests/fetch-retry.test.ts index 85693c7..b294268 100644 --- a/tests/fetch-retry.test.ts +++ b/tests/fetch-retry.test.ts @@ -5,8 +5,10 @@ */ import { test } from "node:test"; import assert from "node:assert/strict"; -import { scrape, DEFAULT_RETRIES, RETRY_BUDGET_MS, type ScrapeOptions } from "../src/core/http.ts"; -import { fullJitterMs, retryAfterMs, crawlBackoffMs } from "../src/core/backoff.ts"; +import { createServer, type Server } from "node:http"; +import type { AddressInfo } from "node:net"; +import { scrape, failedBeforeSend, DEFAULT_RETRIES, RETRY_BUDGET_MS, type ScrapeOptions } from "../src/core/http.ts"; +import { fullJitterMs, retryAfterMs, retryAfterDelayMs, crawlBackoffMs } from "../src/core/backoff.ts"; import { runFetch } from "../src/adapters/protected-fetch.ts"; import { runExtract } from "../src/adapters/extract.ts"; import { normalizeRetries } from "../src/cli/commands/fetch.ts"; @@ -32,7 +34,21 @@ const AUTH010 = JSON.stringify({ type: "https://docs.zenrows.com/api-error-codes#AUTH010", }); -type Step = { status: number; body?: string; headers?: Record } | "network"; +/** + * A response to answer, or a fetch() rejection shaped like undici's: + * `TypeError("fetch failed")` whose `cause.code` says what went wrong + * (`cause: null` means no cause at all). + */ +type Step = { status: number; body?: string; headers?: Record } | { network: string | null }; + +/** undici's rejection for a transport failure with the given `cause.code`. */ +function fetchFailed(code: string | null): TypeError { + if (code === null) return new TypeError("fetch failed"); + return new TypeError("fetch failed", { cause: Object.assign(new Error(`connect ${code}`), { code }) }); +} + +/** The request was never written: connection refused. */ +const REFUSED: Step = { network: "ECONNREFUSED" }; /** A fetch that answers `steps` in order (repeating the last) and counts calls. */ function seqFetch(steps: Step[], onCall?: () => void): { impl: typeof fetch; calls: () => number } { @@ -41,7 +57,7 @@ function seqFetch(steps: Step[], onCall?: () => void): { impl: typeof fetch; cal onCall?.(); const step = steps[Math.min(n, steps.length - 1)]!; n++; - if (step === "network") throw new TypeError("fetch failed"); + if ("network" in step) throw fetchFailed(step.network); return new Response(step.body ?? "", { status: step.status, headers: { "content-type": "application/json", ...step.headers }, @@ -80,15 +96,73 @@ test("full jitter stays inside uniform(0.25 s, min(30 s, 1 s x 2^attempt))", () } }); -test("Retry-After counts whole seconds only and is capped at 30 s", () => { +// Shared retry policy, conformance vectors: Retry-After header value -> delay. +const RETRY_AFTER_VECTORS: Array<[string, number | undefined]> = [ + ["5", 5000], + ["0", 250], + [" 7 ", 7000], + ["99999", 30_000], + ["1.5", undefined], + ["-1", undefined], + ["1e3", undefined], + ["0x10", undefined], + ["", undefined], + ["abc", undefined], + ["Wed, 21 Oct 2026 07:28:00 GMT", undefined], +]; + +for (const [header, expected] of RETRY_AFTER_VECTORS) { + test(`conformance: Retry-After ${JSON.stringify(header)} -> ${expected ?? "jitter"}`, () => { + assert.equal(retryAfterDelayMs(header), expected); + }); +} + +test("conformance: a Retry-After the policy ignores falls back to jitter on the wire", async () => { + for (const [header, expected] of RETRY_AFTER_VECTORS) { + const { impl } = seqFetch([{ status: 503, headers: { "retry-after": header } }, OK]); + const { sleepImpl, waits } = recordSleep(); + // random() = 1 makes the first jitter wait exactly 1000 ms, which no vector produces. + await scrape(API, "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl, randomImpl: () => 1 }); + assert.deepEqual(waits, [expected ?? 1000], `Retry-After ${JSON.stringify(header)}`); + } +}); + +test("Retry-After: 0 waits 250 ms on Fetch, so a server cannot drive a tight loop", async () => { + const { impl, calls } = seqFetch([{ status: 503, headers: { "retry-after": "0" } }]); + const { sleepImpl, waits } = recordSleep(); + await assert.rejects(() => scrape(API, "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl })); + assert.equal(calls(), 3); + assert.deepEqual(waits, [250, 250]); +}); + +test("crawl keeps its own Retry-After reading: whole seconds, capped at 30 s, 0 is no wait", () => { + assert.equal(retryAfterMs("0"), 0); assert.equal(retryAfterMs("7"), 7000); - assert.equal(retryAfterMs(" 2 "), 2000); assert.equal(retryAfterMs("3600"), 30_000); assert.equal(retryAfterMs("1.5"), undefined); - assert.equal(retryAfterMs("Wed, 21 Oct 2015 07:28:00 GMT"), undefined); assert.equal(retryAfterMs(null), undefined); }); +// Shared retry policy, conformance vectors: jitter bounds, with the random +// source at 0 and at its maximum. The CLI has no option for the 30 s cap; the +// last two rows exercise the internal parameter the vectors name. +const JITTER_VECTORS: Array<{ attempt: number; capMs?: number; low: number; high: number }> = [ + { attempt: 0, low: 250, high: 1000 }, + { attempt: 1, low: 250, high: 2000 }, + { attempt: 2, low: 250, high: 4000 }, + { attempt: 5, low: 250, high: 30_000 }, + { attempt: 9, low: 250, high: 30_000 }, + { attempt: 4, capMs: 5000, low: 250, high: 5000 }, + { attempt: 3, capMs: 100, low: 250, high: 250 }, +]; + +for (const v of JITTER_VECTORS) { + test(`conformance: jitter attempt ${v.attempt}${v.capMs ? `, cap ${v.capMs} ms` : ""} -> [${v.low}, ${v.high}] ms`, () => { + assert.equal(fullJitterMs(v.attempt, () => 0, v.capMs), v.low); + assert.equal(fullJitterMs(v.attempt, () => 1, v.capMs), v.high); + }); +} + test("crawl keeps its own schedule: 250 ms x 2^attempt, capped at 10 s, +/-20%", () => { for (let attempt = 0; attempt < 8; attempt++) { const base = Math.min(250 * 2 ** attempt, 10_000); @@ -113,6 +187,30 @@ test("a 503 is retried with full jitter, and the first and second waits keep the assert.ok(second >= 250 && second <= 2000, `second wait ${second}`); }); +test("the response path waits fullJitter(retry index): exactly 1 s, then 2 s, at the random maximum", async () => { + for (const [random, expected] of [ + [() => 1, [1000, 2000]], + [() => 0, [250, 250]], + ] as const) { + const { impl } = seqFetch([{ status: 503 }, { status: 503 }, OK]); + const { sleepImpl, waits } = recordSleep(); + await scrape(API, "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl, randomImpl: random }); + assert.deepEqual(waits, expected); + } +}); + +test("the network-error path waits fullJitter(retry index) too, never 0", async () => { + for (const [random, expected] of [ + [() => 1, [1000, 2000]], + [() => 0, [250, 250]], + ] as const) { + const { impl } = seqFetch([REFUSED, REFUSED, OK]); + const { sleepImpl, waits } = recordSleep(); + await scrape(API, "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl, randomImpl: random }); + assert.deepEqual(waits, expected); + } +}); + for (const status of [429, 502, 504]) { test(`HTTP ${status} is retried`, async () => { const { impl, calls } = seqFetch([{ status, body: "{}" }, OK]); @@ -124,15 +222,98 @@ for (const status of [429, 502, 504]) { }); } -test("a network error before any response is retried", async () => { - const { impl, calls } = seqFetch(["network", "network", OK]); +// Shared retry policy: only failures before the request was written are retried. +for (const code of ["ECONNREFUSED", "ENOTFOUND", "EAI_AGAIN", "ENETUNREACH", "EHOSTUNREACH", "UND_ERR_CONNECT_TIMEOUT"]) { + test(`a network error before the request was sent (${code}) is retried`, async () => { + const { impl, calls } = seqFetch([{ network: code }, { network: code }, OK]); + const { sleepImpl, waits } = recordSleep(); + const r = await scrape(API, "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl }); + assert.equal(r.status, 200); + assert.equal(calls(), 3); + assert.equal(waits.length, 2); + assert.equal(r.attempts, 3); + }); +} + +for (const code of ["UND_ERR_SOCKET", "ECONNRESET", "UND_ERR_HEADERS_TIMEOUT", "UND_ERR_BODY_TIMEOUT", "EPIPE", null]) { + test(`a network error that may follow the request (${code ?? "no cause"}) is not retried`, async () => { + const { impl, calls } = seqFetch([{ network: code }, OK]); + const { sleepImpl, waits } = recordSleep(); + await assert.rejects( + () => scrape(API, "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl }), + (err: unknown) => (err as ToolkitError).code === "BACKEND_UNAVAILABLE", + ); + assert.equal(calls(), 1); + assert.deepEqual(waits, []); + }); +} + +test("a refused connection to every address of a host (AggregateError) counts as never sent", () => { + const refused = (addr: string) => Object.assign(new Error(`connect ECONNREFUSED ${addr}`), { code: "ECONNREFUSED" }); + const all = new AggregateError([refused("::1"), refused("127.0.0.1")]); + assert.equal(failedBeforeSend(new TypeError("fetch failed", { cause: all })), true); + const mixed = new AggregateError([refused("::1"), Object.assign(new Error("reset"), { code: "ECONNRESET" })]); + assert.equal(failedBeforeSend(new TypeError("fetch failed", { cause: mixed })), false); + assert.equal(failedBeforeSend(new DOMException("aborted", "AbortError")), false); + assert.equal(failedBeforeSend(new TypeError("fetch failed")), false); +}); + +/** Start a real HTTP server on a free port; resolves with its base URL. */ +async function listen(handler: Parameters[1]): Promise<{ server: Server; base: string }> { + const server = createServer(handler); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const { port } = server.address() as AddressInfo; + return { server, base: `http://127.0.0.1:${port}/v1/` }; +} + +/** The real global fetch, counting calls. */ +function countingFetch(): { impl: typeof fetch; calls: () => number } { + let n = 0; + const real = globalThis.fetch; + return { impl: ((...args: Parameters) => (n++, real(...args))) as typeof fetch, calls: () => n }; +} + +test("real socket: a refused port is retried, because the request was never sent", async () => { + const { server, base } = await listen(() => {}); + await new Promise((resolve) => server.close(() => resolve())); + const { impl, calls } = countingFetch(); const { sleepImpl, waits } = recordSleep(); - const r = await scrape(API, "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl }); - assert.equal(r.status, 200); + await assert.rejects( + () => scrape(base, "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl }), + (err: unknown) => { + const e = err as ToolkitError; + assert.equal(e.code, "BACKEND_UNAVAILABLE"); + assert.match(e.likely_cause, /ECONNREFUSED/); + return true; + }, + ); assert.equal(calls(), 3); assert.equal(waits.length, 2); }); +test("real socket: a connection closed after the API read the request is not retried", async () => { + let hits = 0; + const { server, base } = await listen((req) => { + hits++; + req.resume(); + req.on("end", () => req.socket.destroy()); + }); + try { + const { impl, calls } = countingFetch(); + const { sleepImpl, waits } = recordSleep(); + await assert.rejects( + () => scrape(base, "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl, retries: 5 }), + (err: unknown) => (err as ToolkitError).code === "BACKEND_UNAVAILABLE", + ); + assert.equal(calls(), 1); + assert.equal(hits, 1); + assert.deepEqual(waits, []); + } finally { + server.closeAllConnections(); + await new Promise((resolve) => server.close(() => resolve())); + } +}); + test("Retry-After is honoured", async () => { const { impl } = seqFetch([{ status: 503, headers: { "retry-after": "7" } }, OK]); const { sleepImpl, waits } = recordSleep(); @@ -147,17 +328,29 @@ test("a large Retry-After is capped at 30 s", async () => { assert.deepEqual(waits, [30_000]); }); -for (const status of [400, 401, 404, 422]) { - test(`HTTP ${status} is not retried`, async () => { +// Shared retry policy, conformance vectors: retry decision (GET). The CLI +// never retried 422 or 500 and keeps it that way; it only sends GET, so the +// POST rows do not apply. +for (const status of [400, 401, 402, 403, 404, 422, 500]) { + test(`conformance: HTTP ${status} is not retried`, async () => { const body = JSON.stringify({ code: "REQS002", title: "Bad request (REQS002)", status }); const { impl, calls } = seqFetch([{ status, body }, OK]); const { sleepImpl, waits } = recordSleep(); - await assert.rejects(() => scrape(API, "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl })); + await scrape(API, "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl }).catch(() => undefined); assert.equal(calls(), 1); assert.deepEqual(waits, []); }); } +test("conformance: 422 no_data_extracted and any other 422 are not retried by the CLI", async () => { + for (const body of [JSON.stringify({ code: "no_data_extracted" }), JSON.stringify({ code: "RESP001" })]) { + const { impl, calls } = seqFetch([{ status: 422, body }, OK]); + const { sleepImpl } = recordSleep(); + await scrape(API, "k", { url: "https://x", extract: "auto" }, { fetchImpl: impl, sleepImpl }).catch(() => undefined); + assert.equal(calls(), 1, body); + } +}); + test("a 429 that says the account is out of credits is not retried", async () => { const body = JSON.stringify({ code: "AUTH004", title: "Usage exceeded", detail: "No credit available" }); const { impl, calls } = seqFetch([{ status: 429, body }, OK]); @@ -169,6 +362,65 @@ test("a 429 that says the account is out of credits is not retried", async () => assert.equal(calls(), 1); }); +test("a 429 whose code says the key reached its credit cap (AUTH014) is not retried", async () => { + const body = JSON.stringify({ code: "AUTH014", title: "API key credit cap reached (AUTH014)" }); + const { impl, calls } = seqFetch([{ status: 429, body }, OK]); + const { sleepImpl } = recordSleep(); + await assert.rejects(() => scrape(API, "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl })); + assert.equal(calls(), 1); +}); + +test("a 429 is retried by its code, not by keywords: concurrency, rate limit and non-JSON pages all retry", async () => { + const bodies = [ + JSON.stringify({ code: "AUTH006", title: "Concurrency limit reached (AUTH006)" }), + JSON.stringify({ code: "AUTH008", title: "Rate limit reached (AUTH008)", detail: "Upgrade your plan for a higher limit." }), + "Too many requests. Upgrade to premium for more.", + "Too Many Requests", + ]; + for (const body of bodies) { + const { impl, calls } = seqFetch([{ status: 429, body, headers: { "content-type": "text/html" } }, OK]); + const { sleepImpl, waits } = recordSleep(); + const r = await scrape(API, "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl }); + assert.equal(r.status, 200, body); + assert.equal(calls(), 2, body); + assert.equal(waits.length, 1, body); + } +}); + +// With original_status or allowed_status_codes, a 429/5xx may be the target +// site's own answer: retrying would hit the target again and may be charged. +for (const status of [429, 502, 503, 504]) { + test(`--original-status: a ${status} belongs to the target and is not retried`, async () => { + const { impl, calls } = seqFetch([{ status, body: "target page", headers: { "content-type": "text/html" } }, OK]); + const { sleepImpl, waits } = recordSleep(); + await scrape(API, "k", { url: "https://x", original_status: true }, { fetchImpl: impl, sleepImpl }).catch(() => undefined); + assert.equal(calls(), 1); + assert.deepEqual(waits, []); + }); +} + +test("allowed_status_codes that covers the status: not retried; one that does not: retried", async () => { + for (const [allowed, expectedCalls] of [ + ["404, 503", 1], + ["503", 1], + ["404", 3], + ["5030", 3], + ] as const) { + const { impl, calls } = seqFetch([{ status: 503, body: "target", headers: { "content-type": "text/html" } }]); + const { sleepImpl } = recordSleep(); + await scrape(API, "k", { url: "https://x", allowed_status_codes: allowed }, { fetchImpl: impl, sleepImpl }).catch(() => undefined); + assert.equal(calls(), expectedCalls, `allowed_status_codes=${allowed}`); + } +}); + +test("runFetch --original-status sends a target 503 once", async () => { + const { impl, calls } = seqFetch([{ status: 503, body: "maintenance", headers: { "content-type": "text/html" } }, OK]); + await withGlobalFetch(impl, async () => { + await runFetch({ url: "https://x.com", originalStatus: true }, defaultConfig(), defaultPolicy(), "k").catch(() => undefined); + }); + assert.equal(calls(), 1); +}); + test("the client timeout is never retried: the API may still finish and charge it", async () => { let calls = 0; const hanging = ((_input: unknown, init?: { signal?: AbortSignal }) => { @@ -222,6 +474,21 @@ test("no retry starts when it would pass the 90 s budget", async () => { assert.ok(clock <= RETRY_BUDGET_MS + 40_000); }); +test("no network-error retry starts when it would pass the 90 s budget", async () => { + let clock = 0; + const { impl, calls } = seqFetch([REFUSED], () => (clock += 40_000)); + const opts: ScrapeOptions = { + fetchImpl: impl, + sleepImpl: async (ms) => void (clock += ms), + nowImpl: () => clock, + randomImpl: () => 1, + retries: 10, + }; + await assert.rejects(() => scrape(API, "k", { url: "https://x" }, opts)); + // 40 s + 1 s fits; 81 s + 2 s fits; 123 s + 4 s does not. + assert.equal(calls(), 3); +}); + test("a retry whose delay alone would pass the budget is skipped", async () => { let clock = 0; const { impl, calls } = seqFetch([{ status: 503, headers: { "retry-after": "30" } }], () => (clock += 70_000)); @@ -233,11 +500,12 @@ test("a retry whose delay alone would pass the budget is skipped", async () => { // --- the flag --------------------------------------------------------------------- -test("--retries accepts whole numbers from 0, and rejects anything else", () => { +test("--retries accepts whole decimal digits from 0, and rejects anything else", () => { assert.equal(normalizeRetries(undefined), undefined); assert.equal(normalizeRetries("0"), 0); assert.equal(normalizeRetries("3"), 3); - for (const bad of ["-1", "1.5", "many", ""]) { + assert.equal(normalizeRetries("010"), 10); + for (const bad of ["-1", "1.5", "many", "", "1e1", "0x2", " 3", "3 ", "+1", "1_0", "Infinity", "99999999999999999999"]) { assert.throws(() => normalizeRetries(bad), (err: unknown) => (err as ToolkitError).code === "INVALID_USAGE", bad); } }); @@ -264,8 +532,12 @@ test("503 RESP009 on Extract is SERVICE_BUSY: not charged, retry shortly, no esc assert.equal(err.retry_after, 1); assert.match(err.message, /RESP009 after 2 retries/); assert.match(err.message, /not charged/i); - assert.match(err.likely_cause, /still preparing this layout; the request was not charged/); - assert.match(err.next_action, /retry/i); + // The cause quotes the API's own words; it does not claim a reason of its own. + assert.match(err.likely_cause, /Service temporarily at capacity \(RESP009\): All capacity is busy right now/); + assert.match(err.likely_cause, /not charged/); + assert.doesNotMatch(err.likely_cause, /preparing this layout/); + assert.match(err.next_action, /Wait at least 1 s.*run the same command again/); + assert.doesNotMatch(err.next_action, /support|escalat/i); assert.deepEqual(err.suggested_commands, ["zenrows extract https://x.com/p"]); assert.ok(!err.suggested_commands.some((c) => /--js-render|--premium-proxy|--manual/.test(c))); return true; @@ -289,6 +561,72 @@ test("503 RESP009 on Fetch is SERVICE_BUSY with a fetch retry command", async () ); }); +test("503 RESP009 on Extract quotes the API's title for each of its two documented meanings", async () => { + for (const title of ["Extract could not finish this page right now", "Extract is at capacity for new pages right now"]) { + const body = JSON.stringify({ code: "RESP009", status: 503, title, type: "https://docs.zenrows.com/api-error-codes#RESP009" }); + const { impl } = seqFetch([{ status: 503, body }]); + await assert.rejects( + () => scrape(API, "k", { url: "https://x.com/p", extract: "auto" }, { fetchImpl: impl, retries: 0 }), + (err: unknown) => { + const e = err as ToolkitError; + assert.equal(e.code, "SERVICE_BUSY"); + assert.ok(e.likely_cause.includes(title), e.likely_cause); + assert.match(e.next_action, /run the same command again/); + return true; + }, + ); + } +}); + +test("503 RESP009 on Extract without a title names both meanings and asserts neither", async () => { + const { impl } = seqFetch([{ status: 503, body: JSON.stringify({ code: "RESP009" }) }]); + await assert.rejects( + () => scrape(API, "k", { url: "https://x.com/p", extract: "auto" }, { fetchImpl: impl, retries: 0 }), + (err: unknown) => { + const e = err as ToolkitError; + assert.match(e.likely_cause, /either it could not finish this page yet, or it is at capacity/); + assert.match(e.next_action, /about a minute .* longer if it says it is at capacity/); + return true; + }, + ); +}); + +for (const status of [429, 502, 503, 504]) { + test(`after the retries, a ${status} does not advise the most expensive configuration`, async () => { + const { impl } = seqFetch([{ status, body: "upstream trouble", headers: { "content-type": "text/plain" } }]); + const { sleepImpl } = recordSleep(); + await assert.rejects( + () => scrape(API, "k", { url: "https://x.com/p" }, { fetchImpl: impl, sleepImpl, rerunCommand: "zenrows fetch https://x.com/p --json" }), + (err: unknown) => { + const e = err as ToolkitError; + assert.equal(e.code, "FETCH_FAILED"); + assert.match(e.message, /after 2 retries/); + assert.deepEqual(e.suggested_commands, ["zenrows fetch https://x.com/p --json"]); + assert.doesNotMatch(`${e.next_action} ${e.suggested_commands.join(" ")}`, /Retry with --manual/); + assert.ok(!e.suggested_commands.some((c) => /--js-render|--premium-proxy/.test(c))); + return true; + }, + ); + }); +} + +test("a target 503 under --original-status keeps the stronger-fetch advice: the target may be blocking", async () => { + const { impl } = seqFetch([{ status: 503, body: "blocked", headers: { "content-type": "text/html" } }]); + await assert.rejects( + () => scrape(API, "k", { url: "https://x.com/p", original_status: true }, { fetchImpl: impl }), + (err: unknown) => /--js-render --premium-proxy/.test((err as ToolkitError).suggested_commands.join(" ")), + ); +}); + +test("the result counts the HTTP requests sent", async () => { + const { impl } = seqFetch([{ status: 503 }, OK]); + const { sleepImpl } = recordSleep(); + const r = await scrape(API, "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl }); + assert.equal(r.attempts, 2); + const once = await scrape(API, "k", { url: "https://x" }, { fetchImpl: seqFetch([OK]).impl }); + assert.equal(once.attempts, 1); +}); + test("a 503 RESP009 that clears on retry returns the page", async () => { const { impl } = seqFetch([{ status: 503, body: RESP009 }, OK]); const { sleepImpl } = recordSleep(); @@ -309,6 +647,37 @@ test("the Extract-not-enabled fallback to autoparse is unchanged and spends no r await withGlobalFetch(impl, async () => { const outcome = await runExtract({ url: "https://x.com/p" }, defaultConfig(), defaultPolicy(), "k"); assert.equal(outcome.fellBackToAutoparse, true); + // Both requests of the call are counted. + assert.equal(outcome.result.attempts, 2); }); assert.deepEqual(seen, ["extract", "autoparse"]); }); + +test("the Autoparse fallback shares the first request's 90 s budget instead of starting a new one", async () => { + const realNow = Date.now; + let clock = realNow(); + Date.now = () => clock; + const seen: string[] = []; + const impl = (async (input: string | URL | Request) => { + const u = new URL(String(input)); + if (u.searchParams.get("extract")) { + seen.push("extract"); + clock += 89_900; // the Extract request took 89.9 s + return new Response(AUTH010, { status: 402, headers: { "content-type": "application/json" } }); + } + seen.push("autoparse"); + return new Response(RESP009, { status: 503, headers: { "content-type": "application/json", "retry-after": "1" } }); + }) as unknown as typeof fetch; + try { + await withGlobalFetch(impl, async () => { + await assert.rejects( + () => runExtract({ url: "https://x.com/p", retries: 1 }, defaultConfig(), defaultPolicy(), "k"), + (err: unknown) => (err as ToolkitError).code === "SERVICE_BUSY", + ); + }); + } finally { + Date.now = realNow; + } + // 89.9 s spent + a 1 s wait would pass 90 s, so the fallback is sent once. + assert.deepEqual(seen, ["extract", "autoparse"]); +}); diff --git a/tests/fetch-timeout.test.ts b/tests/fetch-timeout.test.ts index cb553fa..417d11e 100644 --- a/tests/fetch-timeout.test.ts +++ b/tests/fetch-timeout.test.ts @@ -42,13 +42,22 @@ function withFetchImpl(impl: typeof fetch, fn: () => Promise): Promise { +// Encodes: "Client timeout raised above the server budget." The public docs +// tell clients to keep the timeout at 180 s or more: a client that gives up +// first abandons a request the API keeps working on and may still charge. +test("the default client timeout is above the API's own request budget", () => { assert.equal(SERVER_BUDGET_MS, 180_000); - assert.equal(DEFAULT_TIMEOUT_MS, 120_000); - const err = requestTimeout("https://x", DEFAULT_TIMEOUT_MS, DEFAULT_TIMEOUT_MS); + assert.ok( + DEFAULT_TIMEOUT_MS > SERVER_BUDGET_MS, + `client default (${DEFAULT_TIMEOUT_MS}ms) must outlive the server budget (${SERVER_BUDGET_MS}ms), ` + + "otherwise the abort races the API's own error envelope", + ); +}); + +// A shorter --timeout can still abort a request the API is working on; the +// error must say so and suggest a value above the budget. +test("REQUEST_TIMEOUT under a short --timeout suggests a client timeout above the API's own 180s budget", () => { + const err = requestTimeout("https://x", 120_000, 120_000); const suggested = Number(/--timeout (\d+)/.exec(err.next_action)?.[1]); assert.ok(suggested > SERVER_BUDGET_MS, `suggested ${suggested}ms must outlive the ${SERVER_BUDGET_MS}ms budget`); assert.match(err.likely_cause, /may still finish this request and charge it/); From 96673b860cd0406ad3aa228cc8e091cf0c48f459 Mon Sep 17 00:00:00 2001 From: gonzalo-zenrows Date: Tue, 20 Oct 2026 18:06:12 +0200 Subject: [PATCH 3/5] fix(fetch): drop a dead retry rule, since out-of-credits codes arrive as a 402 The retry decision skipped a 429 whose code was AUTH004 or AUTH014. The API sends both codes as a 402, never as a 429. A 402 is not a retried status, so the rule never ran and only made the policy harder to read. A 429 is now retried like any other 429. A status the target site sent, under original_status or allowed_status_codes, is still sent once. The out-of-credits error that a 402 produces is unchanged. The tests now prove that a 429 with one of these codes is retried, and that a 402 AUTH004 is sent once. The README and the protected-fetch skill no longer describe the rule. --- README.md | 6 ++---- skills/protected-fetch/SKILL.md | 3 +-- src/core/http.ts | 24 ++++++++---------------- tests/fetch-retry.test.ts | 31 +++++++++++++++++++------------ 4 files changed, 30 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index 07f8b1b..a5c3322 100644 --- a/README.md +++ b/README.md @@ -131,10 +131,8 @@ The CLI does not retry: 1. A client timeout (`--timeout`, default 200000 ms, above the API's own 180 s budget). The API may still finish and charge that request. 2. A connection that closed after the request was sent, for the same reason. -3. A 429 whose code says the account or the API key is out of credits - (`AUTH004`, `AUTH014`). -4. A status the target site sent, under `--original-status`. -5. Any request of `zenrows extract prepare` (section 8). A 429 there means the +3. A status the target site sent, under `--original-status`. +4. Any request of `zenrows extract prepare` (section 8). A 429 there means the monthly allowance is used up, and a 503 means preparation is unavailable. Both are answers, and the error names the next step. diff --git a/skills/protected-fetch/SKILL.md b/skills/protected-fetch/SKILL.md index 608fe17..0469190 100644 --- a/skills/protected-fetch/SKILL.md +++ b/skills/protected-fetch/SKILL.md @@ -52,8 +52,7 @@ zenrows fetch --manual --js-render --premium-proxy # the most expensive `--retries 0` sends once). No retry starts after 90 s. Do not wrap it in your own retry loop. `--json` reports `attempts`, the number of requests sent. - The CLI does not retry a timeout, a connection that closed after the request - was sent, an out-of-credits 429 (`AUTH004`, `AUTH014`), or a status the target - sent under `--original-status`. + was sent, or a status the target sent under `--original-status`. Every run is saved under `.zenrows/runs//` (no secrets). See [[extract]] for structured output and [[cost-control]] for the cost model. diff --git a/src/core/http.ts b/src/core/http.ts index 02696e9..25bcd8e 100644 --- a/src/core/http.ts +++ b/src/core/http.ts @@ -81,13 +81,6 @@ export const RETRY_BUDGET_MS = 90_000; */ const RETRY_STATUSES: ReadonlySet = new Set([429, 502, 503, 504]); -/** - * Error codes that say the account or key is out of credits (402/429 `AUTH004`, - * `AUTH014`, per docs.zenrows.com/api-error-codes). A 429 with one of these - * answers the same until the account changes, so it is not retried. - */ -const QUOTA_CODES: ReadonlySet = new Set(["AUTH004", "AUTH014"]); - /** * `error.cause.code` values that mean the request was never written to the * API: the connection was refused, the name did not resolve, the connect timed @@ -142,8 +135,8 @@ function buildUrl(apiBase: string, apiKey: string, params: ScraperParams): { ful * Retries 429, 502, 503 and 504, and network errors that happened before the * request was written, up to `retries` times, with full jitter or the server's * Retry-After. No retry starts after `RETRY_BUDGET_MS` from the first request. Our own timeout is never retried, and neither is any other - * status, a 429 out-of-credits code, or a status that belongs to the target - * site (`original_status`, `allowed_status_codes`). + * status or a status that belongs to the target site (`original_status`, + * `allowed_status_codes`). */ export async function scrape( apiBase: string, @@ -171,7 +164,7 @@ export async function scrape( continue; } const body = sent.raw.toString("utf8"); - if (n < retries && isRetryable(sent.res.status, body, params)) { + if (n < retries && isRetryable(sent.res.status, params)) { const delayMs = retryAfterDelayMs(sent.res.headers.get("retry-after")) ?? fullJitterMs(n, opts.randomImpl); if (now() - startedAt + delayMs <= RETRY_BUDGET_MS) { const code = zrErrorCode(body); @@ -189,13 +182,12 @@ export async function scrape( } /** - * True for a status worth another attempt. Not retried: a 429 whose code says - * the account or key is out of credits, and any status the target site sent. + * True for a status worth another attempt. Not retried: any status the target + * site sent. The out-of-credits codes (`AUTH004`, `AUTH014`) arrive as a 402, + * which is not a retried status, so they need no rule here. */ -function isRetryable(status: number, body: string, params: ScraperParams): boolean { - if (!RETRY_STATUSES.has(status)) return false; - if (isTargetStatus(status, params)) return false; - return !(status === 429 && QUOTA_CODES.has(zrErrorCode(body) ?? "")); +function isRetryable(status: number, params: ScraperParams): boolean { + return RETRY_STATUSES.has(status) && !isTargetStatus(status, params); } /** diff --git a/tests/fetch-retry.test.ts b/tests/fetch-retry.test.ts index defa486..0a4d8f8 100644 --- a/tests/fetch-retry.test.ts +++ b/tests/fetch-retry.test.ts @@ -351,23 +351,30 @@ test("conformance: 422 no_data_extracted and any other 422 are not retried by th } }); -test("a 429 that says the account is out of credits is not retried", async () => { - const body = JSON.stringify({ code: "AUTH004", title: "Usage exceeded", detail: "No credit available" }); - const { impl, calls } = seqFetch([{ status: 429, body }, OK]); - const { sleepImpl } = recordSleep(); +// AUTH004 and AUTH014 are 402 answers in the API, and 402 is not retried. A 429 +// never carries them, so the retry decision must not special-case them either. +test("a 429 is retried even when its body carries an out-of-credits code (AUTH004, AUTH014)", async () => { + for (const code of ["AUTH004", "AUTH014"]) { + const body = JSON.stringify({ code, title: `Usage exceeded (${code})`, detail: "No credit available" }); + const { impl, calls } = seqFetch([{ status: 429, body }, OK]); + const { sleepImpl, waits } = recordSleep(); + const r = await scrape(API, "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl }); + assert.equal(r.status, 200, code); + assert.equal(calls(), 2, code); + assert.equal(waits.length, 1, code); + } +}); + +test("a 402 AUTH004 (account out of credits) is sent once and reported as out of credits", async () => { + const body = JSON.stringify({ code: "AUTH004", title: "Usage exceeded (AUTH004)", detail: "No credit available", status: 402 }); + const { impl, calls } = seqFetch([{ status: 402, body }, OK]); + const { sleepImpl, waits } = recordSleep(); await assert.rejects( () => scrape(API, "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl }), (err: unknown) => (err as ToolkitError).code === "POLICY_MAX_CREDITS_EXCEEDED", ); assert.equal(calls(), 1); -}); - -test("a 429 whose code says the key reached its credit cap (AUTH014) is not retried", async () => { - const body = JSON.stringify({ code: "AUTH014", title: "API key credit cap reached (AUTH014)" }); - const { impl, calls } = seqFetch([{ status: 429, body }, OK]); - const { sleepImpl } = recordSleep(); - await assert.rejects(() => scrape(API, "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl })); - assert.equal(calls(), 1); + assert.deepEqual(waits, []); }); test("a 429 is retried by its code, not by keywords: concurrency, rate limit and non-JSON pages all retry", async () => { From 01b1b8d241153adcbd98c903f5c1eeebad7151b5 Mon Sep 17 00:00:00 2001 From: gonzalo-zenrows Date: Tue, 20 Oct 2026 22:24:20 +0200 Subject: [PATCH 4/5] fix(fetch): retry a connect timeout that arrives from happy eyeballs When a host has several addresses, Node tries one connect per address family. If they all fail, fetch rejects with an AggregateError. A connect timeout on one family carries the code ETIMEDOUT with the syscall connect. That code was not in the never-sent list, so the CLI sent such a request once, although a failed connect never wrote the request. Node also copies the first inner error's code onto the AggregateError. The check read that copied code first, so a refused connect followed by a reset still counted as never sent. The inner errors now decide: every one of them must be a connect failure, and an empty list does not count. An ETIMEDOUT now counts only when its syscall is connect. A read timeout is still sent once, because the API may still finish and charge it. --- src/core/http.ts | 18 +++++++---- tests/fetch-retry.test.ts | 68 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 6 deletions(-) diff --git a/src/core/http.ts b/src/core/http.ts index 25bcd8e..56291f8 100644 --- a/src/core/http.ts +++ b/src/core/http.ts @@ -84,7 +84,8 @@ const RETRY_STATUSES: ReadonlySet = new Set([429, 502, 503, 504]); /** * `error.cause.code` values that mean the request was never written to the * API: the connection was refused, the name did not resolve, the connect timed - * out or the network was unreachable. Only these network errors are retried. + * out or the network was unreachable. Only these network errors are retried, + * plus an `ETIMEDOUT` whose `syscall` is `connect` (see `neverSentCause`). * A reset or close after the request was sent (`UND_ERR_SOCKET`, `ECONNRESET`) * is not: the API may still be working on it and may charge it. */ @@ -207,8 +208,9 @@ function isTargetStatus(status: number, params: ScraperParams): boolean { /** * True when a fetch() rejection means the request never reached the API. * undici wraps every failure as `TypeError("fetch failed")` with the real - * error in `cause`; a refused connection to a host with several addresses - * arrives as an AggregateError whose `errors` all carry a code. + * error in `cause`. A connect to a host with several addresses (happy + * eyeballs) fails as an AggregateError with one error per address; it counts + * only when every one of them is a connect failure. */ export function failedBeforeSend(err: unknown): boolean { return neverSentCause(err instanceof Error ? err.cause : undefined); @@ -216,10 +218,14 @@ export function failedBeforeSend(err: unknown): boolean { function neverSentCause(cause: unknown): boolean { if (!cause || typeof cause !== "object") return false; - const code = (cause as { code?: unknown }).code; - if (typeof code === "string") return NEVER_SENT_CODES.has(code); + // Node copies the first inner error's `code` onto the AggregateError, so + // the inner errors decide, never that copied code. const errors = (cause as { errors?: unknown }).errors; - return Array.isArray(errors) && errors.length > 0 && errors.every(neverSentCause); + if (Array.isArray(errors)) return errors.length > 0 && errors.every(neverSentCause); + const { code, syscall } = cause as { code?: unknown; syscall?: unknown }; + // The OS connect timeout. ETIMEDOUT on a read is after the request was sent. + if (code === "ETIMEDOUT") return syscall === "connect"; + return typeof code === "string" && NEVER_SENT_CODES.has(code); } /** Shell-safe form of one argument for a suggested command. */ diff --git a/tests/fetch-retry.test.ts b/tests/fetch-retry.test.ts index 0a4d8f8..52a8fbc 100644 --- a/tests/fetch-retry.test.ts +++ b/tests/fetch-retry.test.ts @@ -258,6 +258,61 @@ test("a refused connection to every address of a host (AggregateError) counts as assert.equal(failedBeforeSend(new TypeError("fetch failed")), false); }); +/** One connect attempt's error, shaped like Node's: `code` and `syscall`. */ +function sysErr(code: string, syscall?: string): Error { + return Object.assign(new Error(`${syscall ?? "op"} ${code}`), { code, ...(syscall ? { syscall } : {}) }); +} + +/** + * undici's rejection for a happy-eyeballs failure. Like Node, the + * AggregateError carries the first inner error's `code` as its own. + */ +function happyEyeballs(errors: Error[]): TypeError { + const agg = Object.assign(new AggregateError(errors, "connect failed"), { code: (errors[0] as { code?: string } | undefined)?.code }); + return new TypeError("fetch failed", { cause: agg }); +} + +// Shared retry policy A4 3.1: a happy-eyeballs AggregateError is never sent +// only when every inner error is a connect failure. +const AGGREGATE_VECTORS: Array<[string, Error[], boolean]> = [ + ["[ECONNREFUSED, ECONNREFUSED]", [sysErr("ECONNREFUSED", "connect"), sysErr("ECONNREFUSED", "connect")], true], + ["[ETIMEDOUT/connect x2]", [sysErr("ETIMEDOUT", "connect"), sysErr("ETIMEDOUT", "connect")], true], + ["[ECONNREFUSED, ETIMEDOUT/connect]", [sysErr("ECONNREFUSED", "connect"), sysErr("ETIMEDOUT", "connect")], true], + ["[ETIMEDOUT/read]", [sysErr("ETIMEDOUT", "read")], false], + ["[ECONNREFUSED, ECONNRESET]", [sysErr("ECONNREFUSED", "connect"), sysErr("ECONNRESET", "read")], false], + ["[ETIMEDOUT/connect, ETIMEDOUT/read]", [sysErr("ETIMEDOUT", "connect"), sysErr("ETIMEDOUT", "read")], false], + ["[] (empty)", [], false], +]; + +for (const [name, errors, retried] of AGGREGATE_VECTORS) { + test(`happy eyeballs: AggregateError ${name} is ${retried ? "" : "not "}retried`, async () => { + assert.equal(failedBeforeSend(happyEyeballs(errors)), retried); + let n = 0; + const impl = (async () => { + if (n++ === 0) throw happyEyeballs(errors); + return new Response("ok", { status: 200, headers: { "content-type": "text/html" } }); + }) as unknown as typeof fetch; + const { sleepImpl, waits } = recordSleep(); + const run = scrape(API, "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl }); + if (retried) { + assert.equal((await run).status, 200); + assert.equal(n, 2); + assert.equal(waits.length, 1); + } else { + await assert.rejects(run, (err: unknown) => (err as ToolkitError).code === "BACKEND_UNAVAILABLE"); + assert.equal(n, 1); + assert.deepEqual(waits, []); + } + }); +} + +test("a single ETIMEDOUT counts as never sent only when its syscall is connect", () => { + const single = (e: Error) => new TypeError("fetch failed", { cause: e }); + assert.equal(failedBeforeSend(single(sysErr("ETIMEDOUT", "connect"))), true); + assert.equal(failedBeforeSend(single(sysErr("ETIMEDOUT", "read"))), false); + assert.equal(failedBeforeSend(single(sysErr("ETIMEDOUT"))), false); +}); + /** Start a real HTTP server on a free port; resolves with its base URL. */ async function listen(handler: Parameters[1]): Promise<{ server: Server; base: string }> { const server = createServer(handler); @@ -291,6 +346,19 @@ test("real socket: a refused port is retried, because the request was never sent assert.equal(waits.length, 2); }); +test("real socket: a refused port on localhost (one connect per address) is retried", async () => { + const { server, base } = await listen(() => {}); + await new Promise((resolve) => server.close(() => resolve())); + const { impl, calls } = countingFetch(); + const { sleepImpl, waits } = recordSleep(); + await assert.rejects( + () => scrape(base.replace("127.0.0.1", "localhost"), "k", { url: "https://x" }, { fetchImpl: impl, sleepImpl }), + (err: unknown) => (err as ToolkitError).code === "BACKEND_UNAVAILABLE", + ); + assert.equal(calls(), 3); + assert.equal(waits.length, 2); +}); + test("real socket: a connection closed after the API read the request is not retried", async () => { let hits = 0; const { server, base } = await listen((req) => { From 33d7818e946e91f39f2f9ef4d9dc819b9f55f817 Mon Sep 17 00:00:00 2001 From: gonzalo-zenrows Date: Tue, 20 Oct 2026 22:24:20 +0200 Subject: [PATCH 5/5] docs(fetch): say what the Fetch timeout covers and what it defaults to The comment on one Fetch attempt said its timeout also covers the body read. It does not: the timer is cleared before the body is read, so a slow body has no timeout and a failure there is not a normalized error. The comment now says that. The behaviour is unchanged. The timeoutMs comment in protected-fetch now gives the numbers that http.ts uses: 200 s by default, per HTTP attempt, above the API's own 180 s budget. It names both constants, so a reader can check them. --- src/adapters/protected-fetch.ts | 7 ++++--- src/core/http.ts | 7 ++++++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/adapters/protected-fetch.ts b/src/adapters/protected-fetch.ts index 0f62f43..f2bd156 100644 --- a/src/adapters/protected-fetch.ts +++ b/src/adapters/protected-fetch.ts @@ -46,9 +46,10 @@ export interface FetchOptions { outputs?: string; jsonResponse?: boolean; /** - * Client-side timeout in milliseconds. Not an API parameter — it never goes - * into the query string. Unset means `http.ts`'s default, which is above the - * gateway's own request budget on purpose. + * Client-side timeout in milliseconds, per HTTP attempt. Not an API + * parameter — it never goes into the query string. Unset means + * `DEFAULT_TIMEOUT_MS` in `http.ts` (200 s), which sits above the API's own + * 180 s budget (`SERVER_BUDGET_MS`) on purpose. */ timeoutMs?: number; /** diff --git a/src/core/http.ts b/src/core/http.ts index 56291f8..6ad6811 100644 --- a/src/core/http.ts +++ b/src/core/http.ts @@ -244,7 +244,12 @@ type SendResult = | { ok: true; res: Response; raw: Buffer } | { ok: false; error: ToolkitError; neverSent: boolean }; -/** One HTTP attempt with its own timeout; the body is read inside it. */ +/** + * One HTTP attempt. Its timeout covers only the wait for the response + * headers: the timer is cleared before the body is read, so reading the body + * has no timeout, and a failure while reading it rejects with the raw error + * (for example a TypeError), not a ToolkitError. + */ async function sendOnce(full: string, targetUrl: string, opts: ScrapeOptions): Promise { const timeoutMs = opts.timeoutMs ?? DEFAULT_TIMEOUT_MS; const controller = new AbortController();