Skip to content

Commit f129e20

Browse files
committed
fix: stop reporting the CLI's own timeout as BACKEND_UNAVAILABLE
`zenrows fetch` aborted at 90s — exactly the gateway's own request budget — so any request that used the full budget was a race between our abort and the API's real error envelope, and the abort usually won. Every such failure surfaced as: "code": "BACKEND_UNAVAILABLE", "message": "Could not reach the Zenrows API.", "likely_cause": "Network error or timeout: This operation was aborted" Both claims were false. The API had been reached and was about to answer with a specific, actionable error, so the operator was sent to check connectivity instead of reading the answer that already existed. Two independent defects, both fixed here: - The client timeout equalled the server budget. The default is now 120s, deliberately above the gateway's 90s ceiling, so the API always gets to answer for itself. `zenrows fetch` gains `--timeout <ms>` (milliseconds, mirroring `batch wait --timeout`) to raise it further; a non-numeric or non-positive value is rejected as INVALID_USAGE rather than silently falling back to the default. - Every thrown error mapped to BACKEND_UNAVAILABLE. A client-side give-up is now REQUEST_TIMEOUT, carrying the elapsed time so the 90s boundary is visible, and pointing at `--timeout` / dropping `--wait-for` instead of at the network. BACKEND_UNAVAILABLE is left for genuine transport failures only, and now also reports elapsed time. Our own timer flag, not `err.name === "AbortError"`, is what separates the two: it cannot be confused with an abort from anywhere else. The trace-debug skill's failure → action map gains both codes, so an agent reading a trace is steered the same way. Refs ACT-1605 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vy8dYdUhLJoS6EHw5jn9mg
1 parent 5e52585 commit f129e20

6 files changed

Lines changed: 239 additions & 3 deletions

File tree

skills/trace-debug/SKILL.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,12 @@ zenrows trace export <run-id> # JSON for sharing
2121
## Failure → action map
2222
- `FETCH_FAILED` / empty content → retry `--manual --js-render`, then add
2323
`--premium-proxy`; for slow pages add `--wait-for <selector>`.
24+
- `REQUEST_TIMEOUT` → the CLI stopped waiting; the API was reached. Raise
25+
`--timeout` (default 120000ms, above the API's own 90s budget), or drop
26+
`--wait-for` so the request finishes inside that budget and the API returns
27+
its own error. Not a connectivity problem — do not chase the network.
28+
- `BACKEND_UNAVAILABLE` → a genuine transport failure (DNS/TCP/TLS). Check
29+
connectivity and `zenrows config show`.
2430
- `AUTH_INVALID` → re-check the key, `zenrows login --api-key …`.
2531
- `PARAM_CONFLICT_AUTO_MANUAL` → drop the managed flags or add `--manual`.
2632
- `CAPABILITY_UNAVAILABLE` → the primitive is not available on this account (e.g. beta/invite-only); use the local-spec path where offered.

src/adapters/protected-fetch.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,12 @@ export interface FetchOptions {
4545
*/
4646
outputs?: string;
4747
jsonResponse?: boolean;
48+
/**
49+
* Client-side timeout in milliseconds. Not an API parameter — it never goes
50+
* into the query string. Unset means `http.ts`'s default, which is above the
51+
* gateway's own request budget on purpose.
52+
*/
53+
timeoutMs?: number;
4854
}
4955

5056
const RESPONSE_TYPE: Partial<Record<ResponseFormat, string>> = {
@@ -144,6 +150,6 @@ export async function runFetch(
144150
validateAutoManual(opts, config);
145151
const params = buildParams(opts, config);
146152
const mode: "auto" | "manual" = params.mode === "auto" ? "auto" : "manual";
147-
const result = await scrape(config.apiBase, apiKey, params);
153+
const result = await scrape(config.apiBase, apiKey, params, { timeoutMs: opts.timeoutMs });
148154
return { result, params, mode };
149155
}

src/cli/commands/fetch.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ export const fetch_: Command = {
3535
" --output <fmt> html (default) | markdown | text | pdf",
3636
" --screenshot capture an above-the-fold screenshot",
3737
" --out <file> write the response body to a file",
38+
" --timeout <ms> client-side timeout (default 120000; above the API's own 90s budget)",
3839
" --no-signup do not auto-create a Free plan account if no key exists",
3940
" --json print a structured result",
4041
"",
@@ -56,6 +57,7 @@ export const fetch_: Command = {
5657
output: { type: "string" },
5758
screenshot: { type: "boolean" },
5859
out: { type: "string" },
60+
timeout: { type: "string" },
5961
"no-signup": { type: "boolean" },
6062
json: { type: "boolean" },
6163
});
@@ -97,6 +99,7 @@ export const fetch_: Command = {
9799
originalStatus: values["original-status"] === true,
98100
output: normalizeOutput(asString(values.output)),
99101
screenshot: values.screenshot === true,
102+
timeoutMs: normalizeTimeout(values.timeout),
100103
};
101104

102105
const runId = newRunId();
@@ -181,6 +184,28 @@ export const fetch_: Command = {
181184
},
182185
};
183186

187+
/**
188+
* `--timeout <ms>`, in milliseconds, matching `batch wait --timeout`.
189+
*
190+
* Rejects a non-numeric or non-positive value rather than silently falling back
191+
* to the default: an agent that passes `--timeout fast` must not get a green
192+
* result on a timeout the CLI never honored.
193+
*/
194+
export function normalizeTimeout(v: unknown): number | undefined {
195+
if (v === undefined) return undefined;
196+
const ms = asNumber(v);
197+
if (ms === undefined || ms <= 0) {
198+
throw new ToolkitError({
199+
code: "INVALID_USAGE",
200+
message: `Invalid --timeout value '${String(v)}'.`,
201+
likely_cause: "--timeout takes a positive number of milliseconds.",
202+
next_action: "Pass milliseconds, e.g. --timeout 180000 for three minutes.",
203+
suggested_commands: ["zenrows fetch <url> --timeout 180000"],
204+
});
205+
}
206+
return ms;
207+
}
208+
184209
export function normalizeOutput(v?: string): ResponseFormat | undefined {
185210
if (!v) return undefined;
186211
const map: Record<string, ResponseFormat> = {

src/core/errors.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ export type ErrorCode =
1111
| "AUTH_MISSING"
1212
| "AUTH_INVALID"
1313
| "BACKEND_UNAVAILABLE"
14+
| "REQUEST_TIMEOUT"
1415
| "CAPABILITY_UNAVAILABLE"
1516
| "PARAM_CONFLICT_AUTO_MANUAL"
1617
| "PARAM_PROXY_COUNTRY_REQUIRES_PREMIUM"

src/core/http.ts

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,22 @@ export interface ScraperParams {
4343
[param: string]: string | number | boolean | undefined;
4444
}
4545

46+
/**
47+
* Client-side timeout for a Fetch and Extract request, deliberately ABOVE the
48+
* gateway's own 90s request budget.
49+
*
50+
* The two must not be equal. When they were both 90s, any request that used
51+
* the full server budget became a race between our own abort and the API's
52+
* real error envelope — and the abort usually won, so a specific, actionable
53+
* server error (a 422, a 499) was reported as "could not reach the API". The
54+
* client must outlive the server budget so the API always gets the chance to
55+
* answer for itself.
56+
*/
57+
export const DEFAULT_TIMEOUT_MS = 120_000;
58+
59+
/** The gateway's own request budget. Kept here only to justify the default above. */
60+
export const SERVER_BUDGET_MS = 90_000;
61+
4662
function buildUrl(apiBase: string, apiKey: string, params: ScraperParams): { full: string; redacted: string } {
4763
const u = new URL(apiBase);
4864
u.searchParams.set("apikey", apiKey);
@@ -65,8 +81,17 @@ export async function scrape(
6581
): Promise<ScraperResult> {
6682
registerSecret(apiKey);
6783
const { full, redacted } = buildUrl(apiBase, apiKey, params);
84+
const timeoutMs = opts.timeoutMs ?? DEFAULT_TIMEOUT_MS;
6885
const controller = new AbortController();
69-
const timeout = setTimeout(() => controller.abort(), opts.timeoutMs ?? 90_000);
86+
// Our own timer is the only thing that aborts this controller, so this flag —
87+
// not `err.name === "AbortError"` — is what tells a client-side give-up apart
88+
// from a genuine transport failure. The two must never share an error code.
89+
let timedOut = false;
90+
const timeout = setTimeout(() => {
91+
timedOut = true;
92+
controller.abort();
93+
}, timeoutMs);
94+
const startedAt = Date.now();
7095

7196
let res: Response;
7297
try {
@@ -77,11 +102,13 @@ export async function scrape(
77102
});
78103
} catch (err) {
79104
clearTimeout(timeout);
105+
const elapsedMs = Date.now() - startedAt;
106+
if (timedOut) throw requestTimeout(params.url, timeoutMs, elapsedMs);
80107
const cause = err instanceof Error ? err.message : String(err);
81108
throw new ToolkitError({
82109
code: "BACKEND_UNAVAILABLE",
83110
message: `Could not reach the Zenrows API.`,
84-
likely_cause: `Network error or timeout: ${cause}`,
111+
likely_cause: `Network error after ${formatMs(elapsedMs)}: ${cause}`,
85112
next_action: "Check connectivity and retry. Verify api base in `zenrows config show`.",
86113
suggested_commands: ["zenrows status", "zenrows config show"],
87114
});
@@ -244,6 +271,35 @@ export async function scrape(
244271
return result;
245272
}
246273

274+
/**
275+
* The CLI gave up waiting before the API answered. This is NOT unreachability:
276+
* the connection was established and the gateway was still working, so telling
277+
* the operator to check connectivity would send them at the wrong problem. The
278+
* elapsed time is included so the 90s server budget is visible in the output.
279+
*/
280+
export function requestTimeout(url: string, timeoutMs: number, elapsedMs: number): ToolkitError {
281+
const suggestedMs = Math.max(timeoutMs + 60_000, SERVER_BUDGET_MS + 60_000);
282+
const atBudget = elapsedMs >= SERVER_BUDGET_MS;
283+
return new ToolkitError({
284+
code: "REQUEST_TIMEOUT",
285+
message: `The CLI stopped waiting after ${formatMs(timeoutMs)}. The Zenrows API did not respond in time.`,
286+
likely_cause:
287+
`The request was aborted client-side after ${formatMs(elapsedMs)}. The API was reached — this is not a ` +
288+
(atBudget
289+
? `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.`
290+
: `connectivity problem, and the API's own ${formatMs(SERVER_BUDGET_MS)} budget had not run out yet.`),
291+
next_action:
292+
`Retry with a longer client timeout (\`--timeout ${suggestedMs}\`). If the target needs a long render, drop ` +
293+
"`--wait-for` so the request finishes inside the API's budget and the API can return its own error instead.",
294+
suggested_commands: [`zenrows fetch ${url} --timeout ${suggestedMs}`],
295+
});
296+
}
297+
298+
/** `89677` → `89.7s`; `120004` → `120s`. One decimal, and never a bare `.0`. */
299+
function formatMs(ms: number): string {
300+
return `${(ms / 1000).toFixed(1).replace(/\.0$/, "")}s`;
301+
}
302+
247303
function looksLikeContent(r: ScraperResult): boolean {
248304
// allowed_status_codes / original_status can legitimately return 4xx bodies
249305
// (the target's real page content). But Zenrows' OWN error responses also

tests/fetch-timeout.test.ts

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
/**
2+
* The client timeout must not masquerade as backend unreachability (ACT-1605).
3+
*
4+
* The CLI used to abort at 90s — exactly the gateway's own request budget — and
5+
* report every abort as BACKEND_UNAVAILABLE ("Could not reach the Zenrows
6+
* API"). Both halves were wrong: the API had been reached, and it was about to
7+
* return a specific error. These tests pin the two halves of the fix.
8+
*/
9+
import { test } from "node:test";
10+
import assert from "node:assert/strict";
11+
import {
12+
scrape,
13+
DEFAULT_TIMEOUT_MS,
14+
SERVER_BUDGET_MS,
15+
} from "../src/core/http.ts";
16+
import { buildParams, runFetch, type FetchOptions } from "../src/adapters/protected-fetch.ts";
17+
import { normalizeTimeout } from "../src/cli/commands/fetch.ts";
18+
import { defaultConfig } from "../src/core/config.ts";
19+
import { defaultPolicy } from "../src/core/policy.ts";
20+
import { ToolkitError } from "../src/core/errors.ts";
21+
22+
/**
23+
* A fetch that never answers and rejects only when the caller's own timer
24+
* aborts it — exactly what undici does when an AbortController fires
25+
* mid-request. This is the real failure the ticket reproduced, simulated.
26+
*/
27+
function hangingFetch(): typeof fetch {
28+
return ((_input: unknown, init?: { signal?: AbortSignal }) =>
29+
new Promise<Response>((_resolve, reject) => {
30+
init?.signal?.addEventListener("abort", () => {
31+
reject(new DOMException("This operation was aborted", "AbortError"));
32+
});
33+
})) as unknown as typeof fetch;
34+
}
35+
36+
function withFetchImpl(impl: typeof fetch, fn: () => Promise<void>): Promise<void> {
37+
const orig = globalThis.fetch;
38+
globalThis.fetch = impl;
39+
return fn().finally(() => {
40+
globalThis.fetch = orig;
41+
});
42+
}
43+
44+
// Encodes: "Client timeout raised above the server budget."
45+
test("the default client timeout is above the API's own request budget", () => {
46+
assert.ok(
47+
DEFAULT_TIMEOUT_MS > SERVER_BUDGET_MS,
48+
`client default (${DEFAULT_TIMEOUT_MS}ms) must outlive the server budget (${SERVER_BUDGET_MS}ms), ` +
49+
"otherwise the abort races the API's own error envelope",
50+
);
51+
});
52+
53+
// Encodes: "Test covering a simulated abort asserting it is not BACKEND_UNAVAILABLE."
54+
test("a client-side timeout is REQUEST_TIMEOUT, never BACKEND_UNAVAILABLE", async () => {
55+
await withFetchImpl(hangingFetch(), async () => {
56+
await assert.rejects(
57+
() => scrape("https://api.zenrows.com/v1/", "k", { url: "https://x" }, { timeoutMs: 20 }),
58+
(err: unknown) => {
59+
assert.ok(err instanceof ToolkitError);
60+
assert.notEqual(err.code, "BACKEND_UNAVAILABLE", "our own abort is not unreachability");
61+
assert.equal(err.code, "REQUEST_TIMEOUT");
62+
// The old message claimed the API was never reached. It was.
63+
assert.doesNotMatch(err.message, /could not reach/i);
64+
assert.doesNotMatch(err.next_action, /check connectivity/i);
65+
return true;
66+
},
67+
);
68+
});
69+
});
70+
71+
// Encodes: "Include the elapsed time in the error so the 90s boundary is visible."
72+
test("REQUEST_TIMEOUT names the elapsed time and how to raise the timeout", async () => {
73+
await withFetchImpl(hangingFetch(), async () => {
74+
await assert.rejects(
75+
() => scrape("https://api.zenrows.com/v1/", "k", { url: "https://x" }, { timeoutMs: 20 }),
76+
(err: unknown) => {
77+
const e = err as ToolkitError;
78+
assert.match(e.message, /stopped waiting after [\d.]+s/i);
79+
assert.match(e.likely_cause, /aborted client-side after [\d.]+s/i);
80+
assert.match(e.next_action, /--timeout \d+/);
81+
assert.ok(
82+
e.suggested_commands.some((c) => /--timeout \d+/.test(c)),
83+
"must hand the operator a runnable retry with a higher timeout",
84+
);
85+
return true;
86+
},
87+
);
88+
});
89+
});
90+
91+
test("a genuine transport failure is still BACKEND_UNAVAILABLE, with the elapsed time", async () => {
92+
const failing = (() => Promise.reject(new TypeError("fetch failed"))) as unknown as typeof fetch;
93+
await withFetchImpl(failing, async () => {
94+
await assert.rejects(
95+
() => scrape("https://api.zenrows.com/v1/", "k", { url: "https://x" }),
96+
(err: unknown) => {
97+
const e = err as ToolkitError;
98+
assert.equal(e.code, "BACKEND_UNAVAILABLE");
99+
assert.match(e.likely_cause, /fetch failed/);
100+
assert.match(e.likely_cause, /after [\d.]+s/);
101+
return true;
102+
},
103+
);
104+
});
105+
});
106+
107+
test("runFetch threads --timeout through to the HTTP client", async () => {
108+
await withFetchImpl(hangingFetch(), async () => {
109+
await assert.rejects(
110+
() =>
111+
runFetch(
112+
{ url: "https://x.com", timeoutMs: 20 },
113+
defaultConfig(),
114+
defaultPolicy(),
115+
"k",
116+
),
117+
(err: unknown) => (err as ToolkitError).code === "REQUEST_TIMEOUT",
118+
);
119+
});
120+
});
121+
122+
test("timeoutMs is a client concern and never leaks into the API query string", () => {
123+
const opts: FetchOptions = { url: "https://x.com", timeoutMs: 150_000 };
124+
const params = buildParams(opts, defaultConfig());
125+
assert.equal(params.timeout, undefined);
126+
assert.equal(params.timeoutMs, undefined);
127+
});
128+
129+
test("--timeout accepts milliseconds and defaults when absent", () => {
130+
assert.equal(normalizeTimeout("180000"), 180_000);
131+
assert.equal(normalizeTimeout(undefined), undefined);
132+
});
133+
134+
test("--timeout rejects a non-numeric or non-positive value instead of silently ignoring it", () => {
135+
for (const bad of ["fast", "0", "-1", ""]) {
136+
assert.throws(
137+
() => normalizeTimeout(bad),
138+
(e: unknown) => e instanceof ToolkitError && e.code === "INVALID_USAGE",
139+
`--timeout ${JSON.stringify(bad)} must fail loudly`,
140+
);
141+
}
142+
});

0 commit comments

Comments
 (0)