diff --git a/skills/trace-debug/SKILL.md b/skills/trace-debug/SKILL.md index 5b0a195..c1e8406 100644 --- a/skills/trace-debug/SKILL.md +++ b/skills/trace-debug/SKILL.md @@ -21,6 +21,12 @@ zenrows trace export # JSON for sharing ## Failure → action map - `FETCH_FAILED` / empty content → retry `--manual --js-render`, then add `--premium-proxy`; for slow pages add `--wait-for `. +- `REQUEST_TIMEOUT` → the CLI stopped waiting; the API was reached. Raise + `--timeout` (default 120000ms, above the API's own 90s 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. +- `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 …`. - `PARAM_CONFLICT_AUTO_MANUAL` → drop the managed flags or add `--manual`. - `CAPABILITY_UNAVAILABLE` → the primitive is not available on this account (e.g. beta/invite-only); use the local-spec path where offered. diff --git a/src/adapters/protected-fetch.ts b/src/adapters/protected-fetch.ts index 4f5d6d5..660fcd4 100644 --- a/src/adapters/protected-fetch.ts +++ b/src/adapters/protected-fetch.ts @@ -45,6 +45,12 @@ 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. + */ + timeoutMs?: number; } const RESPONSE_TYPE: Partial> = { @@ -144,6 +150,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); + const result = await scrape(config.apiBase, apiKey, params, { timeoutMs: opts.timeoutMs }); return { result, params, mode }; } diff --git a/src/cli/commands/fetch.ts b/src/cli/commands/fetch.ts index 0358c60..5d555f6 100644 --- a/src/cli/commands/fetch.ts +++ b/src/cli/commands/fetch.ts @@ -35,6 +35,7 @@ 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)", " --no-signup do not auto-create a Free plan account if no key exists", " --json print a structured result", "", @@ -56,6 +57,7 @@ export const fetch_: Command = { output: { type: "string" }, screenshot: { type: "boolean" }, out: { type: "string" }, + timeout: { type: "string" }, "no-signup": { type: "boolean" }, json: { type: "boolean" }, }); @@ -97,6 +99,7 @@ export const fetch_: Command = { originalStatus: values["original-status"] === true, output: normalizeOutput(asString(values.output)), screenshot: values.screenshot === true, + timeoutMs: normalizeTimeout(values.timeout), }; const runId = newRunId(); @@ -181,6 +184,28 @@ export const fetch_: Command = { }, }; +/** + * `--timeout `, in milliseconds, matching `batch wait --timeout`. + * + * Rejects a non-numeric or non-positive value rather than silently falling back + * to the default: an agent that passes `--timeout fast` must not get a green + * result on a timeout the CLI never honored. + */ +export function normalizeTimeout(v: unknown): number | undefined { + if (v === undefined) return undefined; + const ms = asNumber(v); + if (ms === undefined || ms <= 0) { + throw new ToolkitError({ + code: "INVALID_USAGE", + message: `Invalid --timeout value '${String(v)}'.`, + likely_cause: "--timeout takes a positive number of milliseconds.", + next_action: "Pass milliseconds, e.g. --timeout 180000 for three minutes.", + suggested_commands: ["zenrows fetch --timeout 180000"], + }); + } + return ms; +} + export function normalizeOutput(v?: string): ResponseFormat | undefined { if (!v) return undefined; const map: Record = { diff --git a/src/core/errors.ts b/src/core/errors.ts index 25a4898..3075bb6 100644 --- a/src/core/errors.ts +++ b/src/core/errors.ts @@ -11,6 +11,7 @@ export type ErrorCode = | "AUTH_MISSING" | "AUTH_INVALID" | "BACKEND_UNAVAILABLE" + | "REQUEST_TIMEOUT" | "CAPABILITY_UNAVAILABLE" | "PARAM_CONFLICT_AUTO_MANUAL" | "PARAM_PROXY_COUNTRY_REQUIRES_PREMIUM" diff --git a/src/core/http.ts b/src/core/http.ts index c70f56b..aa1dc2d 100644 --- a/src/core/http.ts +++ b/src/core/http.ts @@ -43,6 +43,22 @@ export interface ScraperParams { [param: string]: string | number | boolean | undefined; } +/** + * Client-side timeout for a Fetch and Extract request, deliberately ABOVE the + * gateway's own 90s request budget. + * + * 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. + */ +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; + function buildUrl(apiBase: string, apiKey: string, params: ScraperParams): { full: string; redacted: string } { const u = new URL(apiBase); u.searchParams.set("apikey", apiKey); @@ -65,8 +81,17 @@ export async function scrape( ): Promise { registerSecret(apiKey); const { full, redacted } = buildUrl(apiBase, apiKey, params); + const timeoutMs = opts.timeoutMs ?? DEFAULT_TIMEOUT_MS; const controller = new AbortController(); - const timeout = setTimeout(() => controller.abort(), opts.timeoutMs ?? 90_000); + // Our own timer is the only thing that aborts this controller, so this flag — + // not `err.name === "AbortError"` — is what tells a client-side give-up apart + // from a genuine transport failure. The two must never share an error code. + let timedOut = false; + const timeout = setTimeout(() => { + timedOut = true; + controller.abort(); + }, timeoutMs); + const startedAt = Date.now(); let res: Response; try { @@ -77,11 +102,13 @@ export async function scrape( }); } catch (err) { clearTimeout(timeout); + const elapsedMs = Date.now() - startedAt; + if (timedOut) throw requestTimeout(params.url, timeoutMs, elapsedMs); 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 or timeout: ${cause}`, + 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"], }); @@ -244,6 +271,35 @@ export async function scrape( return result; } +/** + * 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. + */ +export function requestTimeout(url: string, timeoutMs: number, elapsedMs: number): ToolkitError { + const suggestedMs = Math.max(timeoutMs + 60_000, SERVER_BUDGET_MS + 60_000); + const atBudget = elapsedMs >= SERVER_BUDGET_MS; + return new ToolkitError({ + code: "REQUEST_TIMEOUT", + message: `The CLI stopped waiting after ${formatMs(timeoutMs)}. The Zenrows API did not respond in time.`, + likely_cause: + `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.`), + 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.", + suggested_commands: [`zenrows fetch ${url} --timeout ${suggestedMs}`], + }); +} + +/** `89677` → `89.7s`; `120004` → `120s`. One decimal, and never a bare `.0`. */ +function formatMs(ms: number): string { + return `${(ms / 1000).toFixed(1).replace(/\.0$/, "")}s`; +} + function looksLikeContent(r: ScraperResult): boolean { // allowed_status_codes / original_status can legitimately return 4xx bodies // (the target's real page content). But Zenrows' OWN error responses also diff --git a/tests/fetch-timeout.test.ts b/tests/fetch-timeout.test.ts new file mode 100644 index 0000000..df04ba0 --- /dev/null +++ b/tests/fetch-timeout.test.ts @@ -0,0 +1,142 @@ +/** + * The client timeout must not masquerade as backend unreachability (ACT-1605). + * + * The CLI used to abort at 90s — exactly the gateway's own request budget — and + * report every abort as BACKEND_UNAVAILABLE ("Could not reach the Zenrows + * API"). Both halves were wrong: the API had been reached, and it was about to + * return a specific error. These tests pin the two halves of the fix. + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + scrape, + DEFAULT_TIMEOUT_MS, + SERVER_BUDGET_MS, +} from "../src/core/http.ts"; +import { buildParams, runFetch, type FetchOptions } from "../src/adapters/protected-fetch.ts"; +import { normalizeTimeout } 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"; + +/** + * A fetch that never answers and rejects only when the caller's own timer + * aborts it — exactly what undici does when an AbortController fires + * mid-request. This is the real failure the ticket reproduced, simulated. + */ +function hangingFetch(): typeof fetch { + return ((_input: unknown, init?: { signal?: AbortSignal }) => + new Promise((_resolve, reject) => { + init?.signal?.addEventListener("abort", () => { + reject(new DOMException("This operation was aborted", "AbortError")); + }); + })) as unknown as typeof fetch; +} + +function withFetchImpl(impl: typeof fetch, fn: () => Promise): Promise { + const orig = globalThis.fetch; + globalThis.fetch = impl; + return fn().finally(() => { + globalThis.fetch = orig; + }); +} + +// Encodes: "Client timeout raised above the server budget." +test("the default client timeout is above the API's own request budget", () => { + 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", + ); +}); + +// Encodes: "Test covering a simulated abort asserting it is not BACKEND_UNAVAILABLE." +test("a client-side timeout is REQUEST_TIMEOUT, never BACKEND_UNAVAILABLE", async () => { + await withFetchImpl(hangingFetch(), async () => { + await assert.rejects( + () => scrape("https://api.zenrows.com/v1/", "k", { url: "https://x" }, { timeoutMs: 20 }), + (err: unknown) => { + assert.ok(err instanceof ToolkitError); + assert.notEqual(err.code, "BACKEND_UNAVAILABLE", "our own abort is not unreachability"); + assert.equal(err.code, "REQUEST_TIMEOUT"); + // The old message claimed the API was never reached. It was. + assert.doesNotMatch(err.message, /could not reach/i); + assert.doesNotMatch(err.next_action, /check connectivity/i); + return true; + }, + ); + }); +}); + +// Encodes: "Include the elapsed time in the error so the 90s boundary is visible." +test("REQUEST_TIMEOUT names the elapsed time and how to raise the timeout", async () => { + await withFetchImpl(hangingFetch(), async () => { + await assert.rejects( + () => scrape("https://api.zenrows.com/v1/", "k", { url: "https://x" }, { timeoutMs: 20 }), + (err: unknown) => { + const e = err as ToolkitError; + assert.match(e.message, /stopped waiting after [\d.]+s/i); + assert.match(e.likely_cause, /aborted client-side after [\d.]+s/i); + assert.match(e.next_action, /--timeout \d+/); + assert.ok( + e.suggested_commands.some((c) => /--timeout \d+/.test(c)), + "must hand the operator a runnable retry with a higher timeout", + ); + return true; + }, + ); + }); +}); + +test("a genuine transport failure is still BACKEND_UNAVAILABLE, with the elapsed time", async () => { + 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" }), + (err: unknown) => { + const e = err as ToolkitError; + assert.equal(e.code, "BACKEND_UNAVAILABLE"); + assert.match(e.likely_cause, /fetch failed/); + assert.match(e.likely_cause, /after [\d.]+s/); + return true; + }, + ); + }); +}); + +test("runFetch threads --timeout through to the HTTP client", async () => { + await withFetchImpl(hangingFetch(), async () => { + await assert.rejects( + () => + runFetch( + { url: "https://x.com", timeoutMs: 20 }, + defaultConfig(), + defaultPolicy(), + "k", + ), + (err: unknown) => (err as ToolkitError).code === "REQUEST_TIMEOUT", + ); + }); +}); + +test("timeoutMs is a client concern and never leaks into the API query string", () => { + const opts: FetchOptions = { url: "https://x.com", timeoutMs: 150_000 }; + const params = buildParams(opts, defaultConfig()); + assert.equal(params.timeout, undefined); + assert.equal(params.timeoutMs, undefined); +}); + +test("--timeout accepts milliseconds and defaults when absent", () => { + assert.equal(normalizeTimeout("180000"), 180_000); + assert.equal(normalizeTimeout(undefined), undefined); +}); + +test("--timeout rejects a non-numeric or non-positive value instead of silently ignoring it", () => { + for (const bad of ["fast", "0", "-1", ""]) { + assert.throws( + () => normalizeTimeout(bad), + (e: unknown) => e instanceof ToolkitError && e.code === "INVALID_USAGE", + `--timeout ${JSON.stringify(bad)} must fail loudly`, + ); + } +});