Skip to content

fix(api): enforce idempotent retries and classified failure handling - #436

Open
Flames4fun wants to merge 2 commits into
AnchorNet-Org:mainfrom
Flames4fun:fix/433-api-client-resilience
Open

fix(api): enforce idempotent retries and classified failure handling#436
Flames4fun wants to merge 2 commits into
AnchorNet-Org:mainfrom
Flames4fun:fix/433-api-client-resilience

Conversation

@Flames4fun

@Flames4fun Flames4fun commented Aug 20, 2026

Copy link
Copy Markdown

This PR implements bounded, idempotency-aware retries and a stable error taxonomy in the shared API client. Errors reach components through the existing errorReporter/toast path, AbortSignal propagation is preserved, and deliberate cancellation never becomes user-visible.

Closes #433.

Pre-change audit

Previously, GET/HEAD retried every 5xx and network rejection, with two retries after the initial attempt. Other methods, timeouts, malformed responses, 4xx responses, and caller aborts were not retried. HTTP failures used equal jitter, while network failures used fixed 500/1000 ms delays. Abort recognition was limited to fetch-path DOMException values, and timeout/signal cleanup happened before response-body consumption completed.

api.test.ts already covered 400/404, 503 success/exhaustion, GET network failures, POST exclusion, caller abort, timeout classification, abort during backoff, malformed JSON, and jitter. Missing coverage included the status allowlist, body timeout/abort, timeout retries, semantic quote idempotency, the elapsed budget, and UI integration.

Alternatives evaluated

  1. Tests/messages only: smallest change, but it preserves incorrect 5xx, network, timeout, and abort behavior.
  2. Policy in each resource client: precise per endpoint, but duplicates transport, timing, cancellation, and parsing logic and invites drift.
  3. Central policy with explicit semantic idempotency (selected): GET/HEAD are safe by default, read-only POST /quote opts in, and mutations remain single-attempt. It keeps one auditable policy without assuming HTTP method alone defines safety.

The selected design fixes the defects without changing the jitter algorithm, modifying useAsync, adding e2e infrastructure, or trusting process-local backend deduplication.

Retry and security policy

Idempotent operations retry only network failures, client timeouts, and HTTP 408/500/502/503/504. All other responses stop immediately, including 400, 404, 409, 422, 429, 501, and 505.

Why 429 remains non-retryable

429 can be temporary, but correct rate-limit recovery requires Retry-After-aware scheduling. Reusing the client's sub-two-second jitter could send another request before the server permits it and amplify throttling. This change therefore excludes 429 instead of introducing a second scheduling policy without an API contract.

503 remains retryable because transient service unavailability is part of the existing resilience policy and retries are restricted to idempotent operations. If a 503 includes Retry-After, this client does not yet consume that hint; server-directed scheduling for either status is a separate follow-up. See RFC 6585 section 4 and RFC 9110 section 10.2.3.

The allowlist is semantic: 408 permits repeating an incomplete request; 501/505 describe unsupported capability/protocol conditions that immediate retries cannot resolve.

Anchor registration/deactivation and settlement open/execute/cancel remain non-retryable. A lost response therefore cannot trigger a client retry of an operation already applied on another replica, where the process-local idempotency cache offers no protection. Quotes may retry because they do not mutate state.

Retry bound

The count, base values, and equal-jitter algorithm are unchanged: two retries, three fetches maximum, and delays in [500, 1000) then [1000, 2000) ms. The maximum configured retry budget is 3 * perAttemptTimeout + 3000 ms: less than 33 seconds of configured request/backoff time with the default 10-second timeout. It excludes event-loop scheduling delay and is not a hard wall-clock guarantee. Timeout inputs are validated against the browser timer range.

Error taxonomy and UI flow

Components can branch on aborted, timeout, network, not_found, invalid_response, client, server, and unknown. ApiRequestError adds retryability, attempts, status/code, request ID, and cause. toast.ts owns safe copy; ToastProvider.notifyError composes it with errorReporter, reports exhausted operational failures, and suppresses aborts. Route 404s, mutation toasts, and inline quote errors remain distinct; no parallel mechanism was added.

Defects found and hardening applied

  • Response bodies outlived timeout/cancellation after headers.
  • Idempotent timeouts were not retried; 501/505 were.
  • Network retries bypassed jitter and lacked typed metadata.
  • Abort recognition rejected compatible non-DOMException errors.
  • Invalid timeout inputs broke the documented budget.
  • The first UI integration converted the abort mapper's null back into "Quote failed."; an adversarial regression exposed this, and QuoteForm now returns to idle without rendering an error.

Adversarial coverage also includes abort/timeout races, stalled error bodies, malformed envelopes, JSON/text parity, final metadata, all excluded statuses, and non-idempotent mutation failures.

Verification

  • npm ci - passed
  • npm test -- src/lib/api.test.ts - passed (98 tests)
  • Focused taxonomy/UI run - passed (5 files, 159 tests)
  • npm test && npm test && npm test - passed three consecutive runs (61 files, 592 tests each)
  • npm run lint - passed with 0 errors; 9 pre-existing warnings remain
  • npm run build - passed
  • git diff --check - passed
  • Changed executable lines - 98.84% covered (255/258); focused coverage: 99.19% lines/statements, 96.92% branches, 100% functions
  • Manual UI - anchor/settlement network failures, resource 404, and quote exhaustion rendered distinct states

TypeScript baseline comparison

After npm ci, the same npx tsc --noEmit command was executed in a clean detached worktree at the PR base and on this branch:

Checkout Commit Diagnostics
PR base feb9b67836d25327ccbb8db8d2c61cb7fe24b6c5 9
PR branch a266ff41b3f2a75d670a2110fd60ec6072f95cb1 9
Delta introduced by this PR - 0

The diagnostic text was identical: four incomplete useAsync mocks in unchanged MetricsBar.test.tsx and five Element/HTMLElement mismatches in unchanged SettlementTable.test.tsx. The PR adds no TypeScript diagnostic, next build passes, and the unrelated baseline remains untouched.

Scope

This PR does not alter jitter timing, modify useAsync or its tests, add e2e infrastructure, or change backend idempotency behavior.

Retry only explicitly idempotent operations for transient failures, preserve abort propagation through response consumption, and expose deterministic attempt and elapsed-time bounds. Cover the status allowlist, jittered backoff, timeout handling, abort races, and non-idempotent exclusions with fake-timer tests.
Route classified API failures through the existing reporter and toast infrastructure, suppress deliberate aborts, and preserve actionable inline states. Document the pre-change audit, defects, retry rationale, elapsed ceiling, coverage, and baseline constraints.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Audit src/lib/api.ts's retry and abort behaviour — establish which failure modes are classified and which surface as generic errors

1 participant