Repository navigation
feat(client): retry uncharged failures by default, with full-jitter backoff and a time budget - #58
gonzalo-zenrows wants to merge 8 commits into
Conversation
… and a time budget A 429, 502, 503 or 504 from the API is never charged and means "try again", so the client now retries those (and network errors before any response) by default, waiting the server's Retry-After or a full-jitter backoff instead of a fixed doubling delay. A 422 is no longer retried: the API no longer sends a retryable 422. A timeout or abort is never retried, because the server may still finish and charge the request. - retries defaults to 2 (was 0); retries: 0 turns retries off - new retryMaxWait (seconds, default 30) caps one backoff wait - new retryBudget (seconds, default 90): no retry starts past it - Retry-After in whole seconds is honoured, capped at 30 s
|
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):
|
The first version of this change retried requests that the API may already have run and charged, and it dropped a retry that callers relied on. 1. A failed fetch is retried only when error.cause.code shows the request never reached the API: connection refused, DNS failure, unreachable network or host, or a connect timeout. A reset or close after the request was sent, a header timeout, an abort and an invalid header value are not. 2. post() is retried only when its headers carry an Idempotency-Key and its body can be sent again. The API forwards the body to the target, so a blind replay can make the target act twice. Without a key it is sent once. 3. A 422 is retried again, except no_data_extracted. A 500 stays final. 4. Retry-After must be whole seconds and is clamped to 0.25 s to 30 s, so a value of 0 waits 250 ms. The 30 s cap does not depend on retryMaxWait. 5. The extract() fallback to autoparse shares the first request's budget. 6. The constructor rejects an invalid retries, retryMaxWait or retryBudget with a RangeError instead of running with NaN or a negative value. Tests run the failure cases against a real HTTP server through Node's fetch, check the shared conformance vectors, and cover body cancellation, the Retry-After cap and post() with and without an Idempotency-Key.
…table across Node versions The connection test counted open server sockets after retried 503s. On Node 20 and 22 that count is not a leak signal. There, undici drains a cancelled body and keeps the socket for its keep-alive window, about 4 s, even when the server sends Connection: close. It also buffers a 2 MB body in full, so an unread one frees its socket too. The test failed on CI with 5 open sockets, while the client had released every answer. The server now counts answers that have neither finished nor lost their connection, and the body is 8 MB, which undici does not buffer on its own. With body cancellation that count reaches 0 within about 25 ms on Node 20, 22 and 26. Without it, the 10 retried answers stay held.
jonathan-zenrows
left a comment
There was a problem hiding this comment.
Can't be merged as is.
1. Conflicts with #49, and the fallback in this branch is now dead code
Main is on 2.5.0 with #49, which removed the extract() AutoParse fallback and added prepareDomain/preparedDomainStatus. This branch still has fallbackToAutoparse = true and the shared clock (src/index.ts:189-208), and src/index.ts conflicts. If the conflict is resolved by keeping this side, #49 gets reverted without anyone noticing: the fallback comes back on and the prepared-domain exports disappear. After the rebase, delete the CallClock fallback path, the tests at tests/retries.test.ts:375-415, and the README sentence about the fallback sharing the budget.
2. Retries hold queue slots
The backoff sleep runs inside the fastq worker (src/index.ts:130, 306). With the default concurrency: 5, five calls that get 429 + Retry-After: 30 block every other call on that client for 30 s. Before this PR nobody hit it, because retries defaulted to 0. Repro: concurrency 5, a server that answers 429 Retry-After: 30 to the first 5 requests, and fire a 6th call: it waits 30 s before it even leaves. It should release the slot while it waits, or at minimum document this.
3. Release note
0 to 2 retries, plus RangeError on bad options (src/retry.ts:57-76), ships as 2.6.0 with no note under feat. It needs feat!/BREAKING CHANGE: or a written changelog line.
Small things:
{"Idempotency-Key": 123}from plain JS throws aTypeErroratvalue.trim()(src/retry.ts:116). UseString(value).trim().ENETUNREACH/EHOSTUNREACHare retried here but not in the other SDKs. They're fine to retry, but let's make all 5 the same.localhost(happy-eyeballs →AggregateError) isn't tested, only127.0.0.1. On Node 26 the code arrives correctly; add a test so we don't depend on that.
The API does not dedupe Idempotency-Key on Fetch or Extract. It only forwards the header to the target site. So the key proves nothing, and a retried post() can make the target act twice. 1. Only GET, HEAD and OPTIONS are retried. A post() goes out once, whatever its headers, the status or the failure, a refused connection included. 2. The body guard now applies to every method: a body that is not none, a string or bytes is never retried. A stream, a reader or a generator counts as one-shot. 3. The README says post() is never retried, and that a caller who needs a retry writes it. Tests send post() with and without an Idempotency-Key on a 503, a 429, a 422 and a refused connection, and check one request each time. They also check that fetch() still retries and that a GET with a stream body is not eligible.
With original_status the API returns the target site's own status. With allowed_status_codes it passes the listed statuses through from the target. Such a status is the site's answer, not the API saying "try again", so another try only asks the same site the same question. 1. When the request sets original_status (true, "true", 1 or "1"), no status is retried. 2. When it sets allowed_status_codes (a comma-separated string or an array), a listed status is not retried. Other statuses keep the usual rule. 3. A failure before the request reached the API, such as a refused connection, is still retried. 4. The rule lives in one place, targetStatusRule in retry.ts. worker() passes the request params to retryPolicy, which applies it in retryOn. 5. The README lists the rule under what the client never retries. Tests cover each accepted form of both params on the policy, fetch() and extract() on a fake clock, and a real node:http server for a 503 sent once and a refused connection that is still retried.
…timeout With happy eyeballs, Node tries each address family of a host, such as ::1 and 127.0.0.1 for localhost. When every try fails, the fetch error's cause is an AggregateError with one error per try. Node copies the first try's code onto the aggregate. So a refused IPv6 try and a reset IPv4 try looked like a plain refused connection, and the client retried it. 1. For an AggregateError, the inner errors decide. The client retries only when every inner error is a connect failure. An empty one is not retried. 2. A connect failure is a code from the existing list, or ETIMEDOUT from the connect syscall. The list itself is unchanged. 3. A single ETIMEDOUT from connect is now retried too. An ETIMEDOUT from any other syscall is a read timeout and is never retried. 4. The README lists the connect timeout under what the client retries. Unit tests cover each mix of inner errors, with and without a code on the aggregate, and plain ETIMEDOUT from connect, from read and with no syscall. A real-socket test sends fetch() to localhost on a closed port and checks three calls with retries set to 2. It runs when localhost resolves to both address families.
What this changes
fetch()andextract()now retry by default, with full-jitter backoff and a time budget. The client retries only answers that say "try again", and only requests that are safe to send twice.post()never retries.The client retries:
code: "no_data_extracted". The client retried these before this PR, and it still does.error.cause.codeand retries onlyECONNREFUSED,ENOTFOUND,EAI_AGAIN,ENETUNREACH,EHOSTUNREACHandUND_ERR_CONNECT_TIMEOUT.ETIMEDOUTfrom theconnectsyscall. AnETIMEDOUTfrom any other syscall is a read timeout, so the client never retries it.::1and127.0.0.1forlocalhost. When every try fails, the cause is anAggregateError. The client retries it only when every inner error is a connect failure from item 3 or 4.The client never retries:
original_status. The API then returns the target site's own status, so another try asks the same site again.allowed_status_codes. That status also comes from the target site. A refused connection is still retried in both cases.Which requests the client retries:
fetch(),get()andextract()send. The policy also accepts HEAD and OPTIONS.post(). The client sends it once, whatever its headers, the status or the failure. A refused connection is not retried either.Idempotency-Keyheader changes nothing. The API does not dedupe on it for Fetch or Extract. It only forwards the header to the target site.post()retry writes it.How long the client waits:
Retry-Afterin whole seconds is honoured and clamped to 0.25 s to 30 s. A value of0waits 250 ms. The 30 s cap does not depend onretryMaxWait.Retry-Afterformat is ignored. The client then waits a random time between 0.25 s andmax(0.25, min(retryMaxWait, 2^attempt))seconds.attemptcounts from 0, so the first retry waits 0.25 s to 1 s.retryBudget). The budget does not cut a running try short, so a call can take longer than the budget by one try.New options:
retryMaxWait(seconds, default 30) andretryBudget(seconds, default 90). The constructor now checksretries,retryMaxWaitandretryBudget. It throws aRangeErrorfor a value it cannot honour, such asNaN, a negative number or a fraction forretries.client.batchandclient.crawlkeep their own retry rules. This PR does not touch them.prepareDomain()andpreparedDomainStatus()never retry. They send one request and reject on an error answer. Their 429 and 503 are answers for the caller to read, so the retry policy does not apply to them.Behaviour change
retriesdefaults to 2. It was 0. Setretries: 0to send every request once.post()that the old client retried whenretrieswas set now gets a single attempt. This holds with or without anIdempotency-Keyheader.Retry-Afteror full jitter.retries,retryMaxWaitorretryBudgetnow throws. Before,retries: -1meant no retries andretries: NaNmeant no limit.original_statuson gets no retry on any status. A request withallowed_status_codesgets no retry on a status it lists. The client reads both params in the form it sends them, sotrue,"true",1and"1"all turnoriginal_statuson. A comma-separated string and an array both work forallowed_status_codes.AggregateErrorwith one connect failure and one other error is no longer retried. Node copies the first try's code onto the aggregate, so the old check retried it.ETIMEDOUTfromconnectis now retried.feat, so the version bump is a minor one.How it was tested
node:httpserver through Node's ownfetch. A refused connection is retried. A refused connection on apost()is sent once, with and without anIdempotency-Key. A socket closed after the server read the request is not retried, for GET and for a keyedpost(). Headers that arrive after undici'sheadersTimeoutare not retried. An invalid header value throws once.Retry-After, jitter bounds and the retry decision.cancel.Retry-Afterof 20 s withretryMaxWait: 5on a fake clock and checks the full 20 s wait.post()with and without anIdempotency-Keyon a 503, a 429 and a 422 withretries: 3. The server sees one request each time. The same answers sent tofetch()are still retried.prepareDomain()andpreparedDomainStatus(). Each call rejects after one request, with default options and withretries: 5.original_statusin each accepted form on a 429, 502, 503, 504 and 422. Off values, such asfalseand"0", still retry.allowed_status_codesas a string, a spaced list, arrays and a number. A listed 503 is final, and an unlisted 502 is retried.fetch()andextract()withoriginal_status: trueand see one request. Without it, the same 503 is sent four times withretries: 3.node:httpserver, a refused connection withoriginal_status: trueis still retried, and a 503 is sent once.fetch()tolocalhoston a closed port withretries: 2. It checks three calls and anAggregateErrorcause withECONNREFUSEDinside. It runs only whenlocalhostresolves to both address families.AggregateErrortwice, with and without a code on the aggregate. Every try refused, every try timed out on connect, or one of each: retried. A read timeout or a reset on one try, or no inner error: not retried.ETIMEDOUT: retried fromconnect, not retried fromreador with no syscall.Not covered
AbortSignal, so a caller cannot cancel a call while it waits between tries. The README says so. Adding a signal is a separate change.post()turns any objectdata, a Buffer or a stream included, into a JSON string before it sends it. This PR does not change that.Idempotency-Key, a new decision can addpost()retries back.Agentclass from Node's global dispatcher, because Node does not export undici. It depends on that internal shape.ZenRowsConfigtype declaresoriginal_statusas a boolean. JavaScript callers can still pass"true"or1, and the client honours them. This PR does not widen the type.AggregateError. So thelocalhosttest also passes without the new check. The unit tests are the ones that fail when the check is reverted.::1try with a code outside the list. The client would then not retry. Nobody has tested that case.1and"1"as on fororiginal_status. Nobody has confirmed that the API reads"1"the same way. If it does not, the client only skips a retry it could have made.Release note
Paste this text into the release PR body when it opens, and check the version number.
Retries are now on by default.
new ZenRows(key)retries a failed request up to 2 times. Before, the default was 0. Pass{ retries: 0 }to keep the old behaviour.fetch(),get()andextract()calls that get a429,502,503,504, or a422other thanno_data_extracted. It also retries a connection that failed before the request left: refused, name not found, connect timeout.post(), a timeout, a connection that closed after the request was sent, or a status that comes from the target site (original_statusset, or the status inallowed_status_codes).Retry-Afterthe API sends, when that is whole seconds (0.25 to 30 seconds). Otherwise it waits a random time between 0.25 seconds and a ceiling that starts at 1 second and doubles.What you may notice when you upgrade
429or503can now wait up to about 90 seconds before it fails. The client takes noAbortSignal, so you cannot cancel the wait.post()is no longer retried. The old client retried it whenretrieswas set.retryMaxWaitandretryBudget, in seconds. An invalidretries,retryMaxWaitorretryBudgetnow throws aRangeErrorwhen you create the client. Before,retries: -1meant no retries andretries: NaNmeant no limit.