Skip to content

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

Description

@Jagadeeshftw

Priority: Medium  ·  Area: API client resilience  ·  Est. effort: 6–9 h

📌 Problem

src/lib/api.ts implements retry with jitter — retryDelayMs(attempt) at api.ts:103, documented as ensuring requests that "fail together do not retry in lockstep. Each delay is between one and two" multiples of the base. That is a considered design, not boilerplate, and the per-resource clients (anchorsApi.ts, metricsApi.ts, settlementsApi.ts) thread AbortSignal through correctly (anchorsApi.ts:9, :19-22).

The infrastructure is sound. What is unestablished is whether the failure taxonomy is right, and that is what this issue asks you to determine:

  • What is retried? Retrying a 500 or a network timeout is correct. Retrying a 400 or a 422 is wasted work — the request will never succeed. Retrying a non-idempotent request that already took effect server-side is worse than wasted.
  • Is abort distinguished from failure? An AbortError from a cancelled request is not an error the user should see. If it flows into the same path as a real failure, components render error states for requests that were deliberately cancelled — including on every unmount.
  • Is retry bounded? State the maximum attempt count and total elapsed ceiling.
  • What does the user see? A timeout, a 404 and a malformed response are three different situations; if they collapse into one generic message, users cannot tell a transient problem from a permanent one.

api.test.ts exists — establish what it already covers before writing anything. The gap between covered and uncovered is the deliverable.

🎯 Design decision required

State and defend:

  1. The audit result — which status codes and error types are retried today, whether abort is distinguished, and what the retry bound is. Report it before proposing changes.
  2. Retry policy. Which failures should be retried? Argue from idempotency: which of the API calls are safe to repeat? The backend's idempotency handling is per-process (tracked in that repo), so a client-side retry of a non-idempotent call is not automatically safe.
  3. Error taxonomy. Propose the distinct categories a component should be able to branch on, and how they reach the UI. errorReporter.ts and toast.ts already exist — integrate rather than adding a parallel mechanism.

🧩 Requirements and context

  • Report any defect found rather than silently fixing it — particularly if a non-retryable status is being retried, or abort is surfacing as an error.
  • Tests must be deterministic: fake timers for backoff, no real waiting. Prove with three consecutive runs.
  • Do not change retry timing values without justification; the jitter design is deliberate.
  • Any change must keep AbortSignal propagation intact — components rely on it for unmount cleanup.
  • Coordinate with the useAsync testing issue: that hook consumes these errors, and its abort behaviour depends on this classification.
  • All 59 test files must pass.

🛠️ Suggested execution

  1. Read api.ts and api.test.ts; produce the audit of retried conditions and existing coverage.
  2. Write tests pinning current behaviour for each condition — including abort.
  3. Report any misclassification found.
  4. Implement the agreed taxonomy and retry policy.
  5. Verify determinism.

✅ Acceptance criteria

  • The PR contains the audit: what is retried, whether abort is distinguished, the retry bound, and what api.test.ts already covers.
  • Tests pin behaviour for retryable failures, non-retryable failures and aborts.
  • Aborted requests do not surface as user-visible errors, proven by a test.
  • Retry is bounded, with the maximum attempts and elapsed ceiling stated and tested.
  • The error taxonomy is implemented via the existing errorReporter/toast modules.
  • Any defect found is reported explicitly.
  • Tests are deterministic across three runs; the full suite passes.

🚫 Out of scope

  • Changing the jitter algorithm without justification.
  • useAsync's own tests — separate issue.
  • Adding an e2e layer — separate issue.

🧪 Verification

npm ci
npm test src/lib/api.test.ts
npm test && npm test && npm test
npx tsc --noEmit

📤 What your PR must include

  1. The retry/abort/coverage audit.
  2. Your retry policy, argued from which calls are idempotent.
  3. Your error taxonomy and how it reaches the UI.
  4. Any defect discovered.
  5. Closes #<n>.

🔒 Security notes

Retrying a non-idempotent request that already succeeded server-side can duplicate an operation — and the backend's idempotency protection is currently per-process, so a retry landing on a different replica is not deduplicated. Client-side retry policy and server-side idempotency are two halves of the same guarantee; getting the client half wrong turns a transient network fault into a duplicated action.

📋 Guidelines

  • Minimum 95% test coverage on changed lines
  • Clear documentation
  • Timeframe: 96 hours from assignment
  • One logical change per commit; no merge commits

💬 Join our community

Working on this, or want to sanity-check your approach before you start? Come and ask — the maintainers are there and happy to help.

Telegram: https://t.me/Grainlify

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

GrantFox OSSGrantFox open-source programMaybe RewardedGrantFox: potentially rewarded contributionThird CampaignGrantFox third campaign issuepriority:mediumMedium difficulty / self-contained but non-trivialtestingTests and coverage

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions