e66b0fa616604b2e8634cc4549227e152965e87e
3
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
348f23bac9 |
Narrow the replay errno set to failures that prove no connection existed
check / check (push) Successful in 4s
`CONNECT_CODES` drives `isSafeToReplay`, which is the only thing standing between a transport failure and a replayed `POST /users/two-factor/verify`. It included `EHOSTUNREACH`, `ENETUNREACH` and `ENETDOWN` on the stated grounds that those errnos can only be reported before any request byte was written. That is not true on Linux: an ICMP destination-unreachable delivered on an already-established connection sets the socket error and the next read or write returns `EHOSTUNREACH` or `ENETUNREACH`, and a local interface going down after the request was fully written surfaces as `ENETDOWN` the same way. In each case the server may already have received and acted on the request -- exactly the ambiguity the rule exists to exclude, on the paths that consume a second-factor attempt or register a thumbnail. The three are dropped from `CONNECT_CODES` and stay in `TRANSPORT_CODES`, so they remain retryable for the idempotent calls; only replay eligibility narrows. What is left -- `ENOTFOUND`, `EAI_AGAIN`, `ECONNREFUSED` -- means no TCP connection to the server ever existed, so no request byte can have been transmitted. The justification is corrected everywhere it was stated: the comment on `CONNECT_CODES`, the one on `isSafeToReplay`, the `postJSON` call site, the README's idempotency section and the `client.test.ts` docblock. All of them now describe what the narrowed set actually establishes rather than claiming a proof it did not support. The narrowing is enforced by the suite rather than asserted in a comment: the three errnos join `ECONNRESET`/`EPIPE`/`ETIMEDOUT` in the `isSafeToReplay`-returns-false test, with companion `isRetryable` assertions so a future edit cannot make them non-retryable by accident. Putting the three back into `CONNECT_CODES` turns that test red (1 failure, verified). |
||
|
|
f3cf4af833 |
Retry transient network failures with exponential backoff (closes #2)
check / check (push) Successful in 20s
No retry on 4xx, backoff on 5xx and transport failures, and a deadline on every request. Before this, one transient 503 or TCP reset failed a file for good, and a CDN connection that went quiet after accepting the request blocked `quak backup` forever, because there was no timeout anywhere. src/retry.ts holds the policy: a classifier that decides whether another attempt could produce a different answer, and a loop that acts on it with exponential backoff and full jitter. Retried: 5xx, 408, 429, transport failures (the errno is read out of the cause chain, which is where Node's fetch puts it), deadline aborts, and truncated transfers. Not retried: every other 4xx, and anything unrecognised — a wrongly retried permanent failure delays every remaining file, while a wrongly abandoned transient one costs a single file the next run picks up. Attempt count, delays, sleep and jitter source are all configurable through ApiClientOptions; sleep being injectable is what lets the suite exercise the policy without waiting. Truncation needed a type before it could be classified. streamDecrypt threw plain Errors whose messages began "download: stream truncated", and classifying on message text would mean the next reword silently turned every truncated download into a permanent failure. It now throws TruncatedStreamError, which lives in src/errors.ts alongside ApiError so the classifier can recognise both without importing the modules that import it; api/client.ts re-exports ApiError, so it stays one class and every existing import path still resolves. Downloads retry the request, the stream consumption and the decryption together. Only the first of those happens inside ApiClient: a socket reset after the headers arrived throws in streamDecrypt, and retrying the request alone would never see it. The client's own retry is switched off for those two calls so the budgets do not multiply into sixteen requests per file, and the atomic write stays outside the loop so a download that took three attempts still performs one write and one rename. Non-idempotent requests are not blindly replayed. postJSON and putJSON reach create-session, two-factor/verify — which burns one of a few second-factor attempts — and files/thumbnail, so they retry only when the connection was never established and the server provably never saw the request. putFile is exempt and retries fully: a presigned PUT stores one whole object at one key, with no partial state to damage. It now throws ApiError with the status, as do the two null-body paths, which previously threw bare Errors that nothing could classify. Timeouts come from AbortSignal.timeout(), renewed per attempt: 30s for JSON and upload calls, 10 minutes for file bodies, since a value short enough to keep a hung API call from stalling a backup would cancel a legitimate multi-gigabyte download. The download deadline is enforced over the body rather than only the headers, by racing each read against the signal, so the guarantee does not depend on the fetch implementation tearing down a stream it already handed over. listMissingThumbnails now separates a genuine 404 from an exhausted retry. Its bare catch reported both as missing, which after this change would have let a few minutes of 500s talk fix-missing-thumbnails into regenerating and re-uploading thumbnails that were fine. runBackup and runMetadataBackup are untouched: the retry sits below them and their per-file resilience is unchanged. |
||
|
|
0cbe338b58 |
Add failing tests for the download retry policy
Tests only; the retry module they import does not exist yet, so the branch is red at this commit. New test/retry/retry.test.ts documents the classifier and the backoff: which errors are worth another attempt, which are not, and how the delay before each retry is derived. It asserts on the arguments handed to an injected sleep function rather than on elapsed time, so the suite never waits and the numbers are exact. test/api/client.test.ts gains the request-count contract for each of the six call sites, the deadline behaviour, the ApiError typing that the presigned PUT and the null-body paths need in order to be classified at all, and the replay rule for the two non-idempotent methods. test/download/download.test.ts gains the case that motivates the whole design: a socket reset after the response headers arrived, which happens below ApiClient and can only be caught by retrying the request, the stream consumption and the decryption together. It also pins that a retried download stages exactly one temp file, and that the two retry layers do not compose into a multiplied request budget. The existing truncation tests now assert on the error type rather than its wording, since that type is what the classifier reads. test/thumbnails/thumbnails.test.ts separates a genuine 404 from an exhausted retry, so a failing server can no longer make fix-missing-thumbnails re-upload thumbnails that already exist. |