Harden the retry classifier and pin per-attempt deadlines (closes #80)
check / check (push) Successful in 31s

A POST or PUT is replayed only when every errno in the cause chain is a
connect errno, and postJSON/putJSON no longer follow redirects, so a
redirect is an ApiError that is not retried. getRetryOptions() returns
a copy. New tests pin every errno the classifier names, the cause-chain
depth limit, a two-error cycle, and a fresh deadline per attempt for
every retrying entry point. The README endpoint list is now the one
place naming the requests the replay rule covers; code comments point
to it.

Model: opus-5-5
This commit is contained in:
2026-09-22 23:58:27 +00:00
parent fe952d3e62
commit f788c0adc8
6 changed files with 200 additions and 40 deletions
+56 -4
View File
@@ -149,8 +149,11 @@ describe("isRetryable: transport failures", () => {
});
it("retries an errno carried on the error itself", () => {
// Every errno the classifier names, so none can be reclassified
// unnoticed.
for (const code of [
"ECONNRESET",
"ECONNABORTED",
"ETIMEDOUT",
"EPIPE",
"ENOTFOUND",
@@ -158,6 +161,8 @@ describe("isRetryable: transport failures", () => {
"ECONNREFUSED",
"EHOSTUNREACH",
"ENETUNREACH",
"ENETRESET",
"ENETDOWN",
]) {
expect(isRetryable(errnoError(code))).toBe(true);
}
@@ -215,6 +220,27 @@ describe("isRetryable: transport failures", () => {
looped.cause = looped;
expect(isRetryable(looped)).toBe(false);
});
it("terminates on a cause chain that loops through two errors", () => {
const first: Error & { cause?: unknown } = new Error("first");
const second = new Error("second", { cause: first });
first.cause = second;
expect(isRetryable(first)).toBe(false);
});
it("reads the error and at most seven causes below it", () => {
// The walk is bounded at eight links. An errno at the eighth link is
// found; one at the ninth is not.
const buried = (causes: number): Error => {
let err = errnoError("ECONNRESET");
for (let i = 0; i < causes; i++) {
err = new Error(`wrapper ${i}`, { cause: err });
}
return err;
};
expect(isRetryable(buried(7))).toBe(true);
expect(isRetryable(buried(8))).toBe(false);
});
});
describe("isRetryable: stream truncation versus corruption", () => {
@@ -279,10 +305,9 @@ describe("isSafeToReplay", () => {
* that is not the whole question: the other half is "could the first
* attempt already have taken effect on the server?".
*
* quak's non-idempotent calls are `/users/srp/create-session`,
* `/users/two-factor/verify` (which consumes one of a limited number of
* 2FA attempts) and `/files/thumbnail`. A blind replay of any of them can
* do real damage, so they retry only on the failures that establish no TCP
* The calls this guards are the `POST` and `PUT` requests listed in the
* README under "Endpoints used". A blind replay of some of them can do
* real damage, so they retry only on the failures that establish no TCP
* connection to the server ever existed — DNS produced no address, or the
* peer refused the connection — and therefore that no request byte can
* have been transmitted.
@@ -333,6 +358,33 @@ describe("isSafeToReplay", () => {
).toBe(false);
expect(isSafeToReplay(new TypeError("fetch failed"))).toBe(false);
});
it("does not replay any other errno the classifier names", () => {
for (const code of [
"ECONNRESET",
"ECONNABORTED",
"ETIMEDOUT",
"EPIPE",
"EHOSTUNREACH",
"ENETUNREACH",
"ENETRESET",
"ENETDOWN",
]) {
expect(isSafeToReplay(errnoError(code))).toBe(false);
}
});
it("does not replay a chain that also shows the request may have gone out", () => {
// A connect errno somewhere in the chain is not enough: any other
// errno beside it is doubt, and doubt is not replayed.
const reset = Object.assign(
new Error("read ECONNRESET", { cause: errnoError("ECONNREFUSED") }),
{ code: "ECONNRESET" },
);
const mixed = new TypeError("fetch failed", { cause: reset });
expect(isRetryable(mixed)).toBe(true);
expect(isSafeToReplay(mixed)).toBe(false);
});
});
// ---------------------------------------------------------------------------