Repository navigation
feat(fetch): let scrape and extract retry calls the API did not serve, opt-in with ZENROWS_FETCH_RETRIES - #80
Conversation
|
Same retry policy across all clients (429/502/503/504 and pre-response connection errors; 2 retries by default; full jitter; Retry-After capped at 30 s; 90 s total budget; timeouts never retried):
|
4cb72cf to
ae3292a
Compare
ae3292a to
be080cd
Compare
jonathan-zenrows
left a comment
There was a problem hiding this comment.
On stdio this is fine. On the hosted server, cancellation never reaches the retry loop.
src/http.ts:398-405 creates a new transport and server for each POST (stateless, enableJsonResponse). A client's notifications/cancelled arrives in a different POST, so it lands on another Protocol that doesn't own the request, and the handler's extra.signal never fires. On top of that, the web transport in SDK 1.32.1 doesn't read req.signal, so a client disconnect doesn't abort anything either.
Repro: a client with a 60 s tool timeout calls scrape against the hosted server while the API answers 503. The client gives up at 60 s, and the server keeps sleeping and retrying until it reaches 90 s, holding a slot and API concurrency for a client that's no longer there. The tests don't catch it because tests/fetch-retry-tool.test.ts:30 uses InMemoryTransport.
Fix in http.ts:
c.req.raw.signal.addEventListener("abort", () => transport.close(), { once: true });Protocol._onclose aborts the controllers of every in-flight handler. Add a test over the HTTP transport. Until this lands, the README/PR text about "no request after cancel" only holds for stdio.
Related: 90 s is above the ~60 s timeout of the usual clients, and scrape/extract have no per-attempt timeout (that's from before this PR). For hosted, a budget of ~55 s would make more sense.
Small: prettier --check fails on tests/extract.test.ts. Also, an AggregateError with ETIMEDOUT (happy-eyeballs) isn't retried. That's conservative, just so it's on record.
It conflicts with main, so it needs a rebase.
…, opt-in with ZENROWS_FETCH_RETRIES A 429, 502, 503 or 504, or a connection that failed before the request was sent, means the API did not serve the call, so sending it again is safe. ZENROWS_FETCH_RETRIES sets how many retries scrape and extract may make. The default 0 sends each call once, as today. Waits use Retry-After or full jitter, and no retry starts unless its wait ends within 50 s of the first request, since MCP clients cancel a tool call at about 60 s.
2c9d823 to
3582c30
Compare
Merge after #114, which adds the changelog section this PR's changelog lines use.
This PR adds an opt-in variable,
ZENROWS_FETCH_RETRIES, forscrapeandextract. The default is0, so every tool call behaves as today: one request, the same output, the same errors.With
ZENROWS_FETCH_RETRIESsetscrapeandextractretry, up to that many times, a 429, 502, 503 or 504, and a connection that failed before the request was sent (refused, name resolution, unreachable network or host, connect timeout). Every other status or error is returned at once.Retry-Afterin whole seconds (clamped to 250 ms - 30 s), or else full jitter between 250 ms and min(30 s, 1 s x 2^n).Tests
scrapeand forextract, and logs nothing.Retry-Afterand jitter vectors, which statuses and errors retry, the 50 s rule, cancel, the stderr line, and an invalid value stopping both entry points.build,check:server-json,typecheck,lintandnpm testpass;prettier --checkpasses on the touched files.BEGIN_COMMIT_OVERRIDE
feat(fetch): new ZENROWS_FETCH_RETRIES option lets scrape and extract retry calls the API did not serve; off by default
END_COMMIT_OVERRIDE