fix(shared): harden fetchWithRetry — backoff cap, 429, Retry-After, abort handling (#209)#212
Merged
Merged
Conversation
…orts) Addresses the retry.ts items from the packages/shared review (#209): - Cap backoff at MAX_BACKOFF_MS (was defined + tested but never applied, so backoff was unbounded for large maxRetries/baseDelayMs). - Retry 429 Too Many Requests (previously only 5xx), so a rate-limited OLRC/CourtListener response backs off instead of failing immediately. - Honor a Retry-After header (delta-seconds or HTTP-date) on retryable responses, capped at MAX_BACKOFF_MS. - Do NOT retry AbortError — a caller that cancels (timeout/abort signal) now gets the abort back immediately instead of N pointless retries that mask it as a network error. - Preserve the underlying error via `{ cause }` so ECONNRESET/stack/code survive for diagnostics. New tests cover 429 retry, abort short-circuit (callCount === 1), and cause preservation. shared 28 / fetcher 123 / annotator 76 pass; build 8/8. Refs #209 (rate-limiter FIFO concurrency fix tracked separately) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses the
retry.tsitems from #209 (thepackages/sharedreview). The rate-limiter concurrency fix is larger and tracked separately under the same issue.Changes to
fetchWithRetryMAX_BACKOFF_MS— the constant was defined and unit-tested but never applied, so backoff grew unbounded for largemaxRetries/baseDelayMs.429 Too Many Requests— previously only5xxretried, so a rate-limited OLRC/CourtListener response failed immediately instead of backing off.Retry-After(delta-seconds or HTTP-date) on retryable responses, capped atMAX_BACKOFF_MS.AbortError— a caller that cancels (timeout / abort signal) now gets the abort back immediately rather thanNpointless retries masking it as"Network error after N attempts".{ cause }so the underlying error (ECONNRESET, stack,code) survives for diagnostics.Refactored into small helpers (
isRetryableStatus,backoffDelayMs,retryAfterMs).Verification
New tests: 429 retry, abort short-circuit (
callCount === 1), cause preservation. Existing retry tests (5xx, network, 404 non-retryable, options passthrough) unchanged.pnpm --filter @civic-source/shared test(28),fetcher(123),annotator(76) all pass;pnpm build8/8.🤖 Generated with Claude Code