[FagnerMartinsBrack]

Slice 1 (step 1) of the transient-crawl retry plan: the fetch-failure classifier and nothing else. It changes no retry behaviour, no log line, no deadline and no queue setting.

What changed

• decideFetchFailure (src/packages/crawl-article/src/fetch-retry-decision.ts) classifies a thrown fetch failure as retryable (local or durable), terminal or cancelled, keeping the original cause.
  • Transient classes: ECONNRESET, UND_ERR_SOCKET, UND_ERR_CLOSED, H2_STREAM_ENDED_WITHOUT_RESPONSE, and curl exits 52 and 56.
  • A deadline (TimeoutError) is durable.
  • A parent abort is cancelled, unless its reason is a deadline, in which case it is durable.
  • DNS EBUSY, curl exit 35, block-class errors, bare "fetch failed" and "503"-in-message stay terminal.
• Every-leg rule. A TransportLadderFailure is read across its previousErrors and cause, following each leg's cause chain and recursing into nested ladders. It is durable only when every recorded leg was transient or timed out; one non-transient leg makes it terminal. This does not gate on retryDisposition, which is left unchanged.
• Coded h2 failure. fetchH2 now rejects a stream that ended without a response with an unexported H2StreamEndedWithoutResponse whose code is H2_STREAM_ENDED_WITHOUT_RESPONSE. The message is unchanged.
• Required decision field. decision: FetchRetryDecision | undefined is now required on the failed variants of CrawlArticleResult, of the internal FetchOutcome and of CrawlAndFinalizeResult.
  • It is set from the conditional-GET catch and passed through by crawlAndFinalizeArticle.
  • Every other failed return, an HTTP status or a site-rule failure, passes undefined.
  • Test fakes across save-link, inbox, hutch, article-parser and finalize-article were updated to match.

Tests

• fetch-retry-decision.test.ts:
  • classes and deadlines, parent cancellation, terminal cases;
  • the every-leg cases: timeout plus reset is durable, timeout plus curl 35 is terminal, a block-class leg is terminal, nested ladders;
  • a real runTransportLadder ending on curl exit 56 after transient direct legs;
  • a real ladder whose primary answers 403 and later legs reset, which is durable;
  • a real initCrawlFetch direct ladder against a node:net server that calls socket.destroy() (curl faked as exit 52), which is durable. It uses undici's fetch, because jest's sandboxed fetch drops the socket error code.
• h2-fetch.test.ts: the code on the clean-close case, plus a destroyed-socket server.
• crawl-article.test.ts: an all-legs ECONNRESET crawl gives a durable decision, and a 503 gives decision: undefined.
• crawl-and-finalize-article.test.ts: decision passes through unchanged.

pnpm check passes (61 projects), and the pre-commit hook passed.

Notes for review

• The classifier walks the cause chain as plain objects, not with instanceof Error, so errors from another realm (as under jest) still classify.
• CLAUDE.md says to commit to main, but this session was set to push to rp/confident-einstein-e4cxtl, so this lands as a draft PR. The plan's question of one PR per slice is still for the owner to answer.

---

Generated by Claude Code

[FagnerMartinsBrack]

@claude Review this PR. Use your Read tool to read the local file .github/workflows/claude-PR-code-reviewer.md for instructions. The file is in the checked-out repository.

Context

• Repo: Readplace/readplace.com
• PR: feat: classify transient fetch failures on failed crawl results #1342
• Commit: 52a41bf

[claude]

Claude finished @FagnerMartinsBrack's task in 35s —— View job

---

I'll analyze this and get back to you.

[FagnerMartinsBrack]

High Priority Issues (Must Fix)

None found

Medium Priority Issues

1. A primary-leg 403 followed by resets is classified as durable-retryable

src/packages/crawl-article/src/fetch-retry-decision.test.ts has the test "is durable when the primary leg answered 403 and the later legs reset, since the ladder records only the resets". That test locks in a classification that will matter as soon as a later slice acts on decision.

In runTransportLadder (transport-ladder.ts:136-166), an escalating 403 is kept only as lastResponse, and it is never pushed to errors. The resets that follow are not recoverable, so allFailuresRecoverable turns false and the ladder throws a TransportLadderFailure that holds only the resets. The 403 is dropped. classifyLadder then finds every recorded leg transient and returns durable.

A 403 on the primary leg followed by resets on the h2 and curl legs is the usual pattern for an edge that blocks by fingerprint, which the canary section in CLAUDE.md describes (Cloudflare TLS, Fastly JA3). If those saves become durable retries, each retry will spend a full ladder budget against an origin that is blocking us, and the save will still fail.

This slice doesn't change any behaviour, so nothing breaks today. Still, the test states the outcome as intended. Before slice 2 consumes the decision, choose one of these:

• have the ladder record the escalated status, for example by including it in TransportLadderFailure, so the every-leg rule can see it and return terminal;
• rename the test so it describes a known gap rather than the intended outcome.

Fix this →

Low Priority Suggestions

1. parentSignal in the conditional GET is the fetch's own controller

In crawl-article.ts, the value passed as parentSignal is controller.signal. Only the body-read timer aborts that controller, always with a TimeoutError, so the cancelled branch of decideFetchFailure can't happen on this path today. That's fine for this slice. If real cancellation from a caller is supposed to produce cancelled later, that signal will need to be threaded through here.

2. classifyChain stops at the first link it recognises

A chain whose outer link is a deadline but whose inner cause is a non-transient error (or the other way round) is classified by whichever link comes first. That matches how the existing chain walkers (isHardCrawlFailure, isBlockClassError) work. One test pinning the precedence, for example a TimeoutError wrapping a TLS error, would make the intended order explicit.

[FagnerMartinsBrack]

@claude Fix the review issues. Use your Read tool to read the local file .github/workflows/claude-PR-code-review-auto-apply.md for instructions. The file is in the checked-out repository.

Context

• Attempt: 1 of 5
• Repo: Readplace/readplace.com
• PR: feat: classify transient fetch failures on failed crawl results #1342
• Branch: rp/confident-einstein-e4cxtl

Review to Address

See the review comment above this one for the issues to fix.

[claude]

Claude finished @FagnerMartinsBrack's task in 18m 19s —— View job

---

Fixed the medium-priority review issue

• Read the review and the ladder/classifier code
• Fix: escalated 403 followed by resets classified as durable
• Run pnpm check (61 projects pass; the pre-commit hook passed too)
• Commit and push (a8c4301)

What was wrong: when a leg got back a block status such as 403, runTransportLadder escalated to the next leg and kept that response only as lastResponse. If the later legs then reset, the TransportLadderFailure it threw held only the resets. The classifier saw every recorded leg as transient and returned durable. That is the usual pattern when an edge blocks us by fingerprint, so a later slice would have spent full retries on an origin that is blocking us.

What changed: the ladder now records the failure instead of losing it, which was the first option in the review.

• TransportLadderFailure has a required escalatedStatuses: readonly number[] field. The ladder adds the status of every response it escalates past (400/402/403/419/429 or an AWS WAF challenge).
• In classifyLadder (fetch-retry-decision.ts), any escalated status makes the ladder terminal. A blocking edge counts as a non-transient leg under the every-leg rule.
• The real-ladder test is renamed to "is terminal when the primary leg answered 403 and the later legs reset…" and now expects terminal.
• Every test that builds a TransportLadderFailure directly now passes escalatedStatuses: []. retryDisposition and the ladder's control flow are unchanged.

Not done: I left both low-priority suggestions alone (the parentSignal threading and the cause-chain precedence test), as the auto-apply instructions say.
· Branch

[FagnerMartinsBrack]

@claude Review this PR. Use your Read tool to read the local file .github/workflows/claude-PR-code-reviewer.md for instructions. The file is in the checked-out repository.

Context

• Repo: Readplace/readplace.com
• PR: feat: classify transient fetch failures on failed crawl results #1342
• Commit: a8c4301

[claude]

Claude finished @FagnerMartinsBrack's task in 26s —— View job

---

I'll analyze this and get back to you.

[FagnerMartinsBrack]

High Priority Issues (Must Fix)

None found

Medium Priority Issues

None found

Low Priority Suggestions

1. A 429 escalation now makes the whole ladder terminal, which goes against how the ladder treats a 429

classifyLadder (src/packages/crawl-article/src/fetch-retry-decision.ts:33) returns terminal whenever escalatedStatuses is non-empty. That list is filled from every status in ESCALATE_STATUS_CODES, and 429 is one of them. The ladder itself treats a 429 differently (transport-ladder.ts:151, !(lastResponse?.status === 429 && allFailuresRecoverable)): after a 429 it keeps going through recoverable network errors, because a rate limit is a "try later" signal and not a fingerprint block. So a 429 on the primary leg followed by resets is now terminal, although it is the case where a durable retry is most likely to succeed.

Nothing consumes decision yet, so this changes no behaviour today. Before slice 2 acts on the decision, decide whether 429 should be excluded from the terminal rule, for example by checking only non-429 statuses, or by treating a 429 as durable. Add one test that pins whichever choice you make.

2. parentSignal in the conditional GET is the fetch's own controller

(Carried over from the previous review and still open.) In crawl-article.ts:316, parentSignal is controller.signal. The only thing that aborts it is fetchTimeoutReason, whose name is always TimeoutError, so the cancelled branch can't be reached on this path. That's fine for this slice. If cancellation from a caller should later produce cancelled, the caller's signal will need to be passed through here.

3. Precedence in classifyChain is not tested

(Carried over from the previous review.) classifyChain returns at the first link it recognises. A TimeoutError that wraps a TLS error is therefore durable, while a block-class wrapper is caught earlier by classifyLeg. One test would make the intended order explicit.

4. The fix-up commit message has no scope

fix: address high/medium priority review issues has no scope and does not say what changed. Under the git-commit skill it would read something like fix(@packages/crawl-article): record escalated statuses on TransportLadderFailure. This is worth fixing if the commits get squashed or reworded before merge.