Skip to content

feat: add retry with exponential backoff in client.ts (DIS-143)#37

Merged
ckorhonen merged 5 commits intomainfrom
devin/1772640341-dis-143-retry-backoff
Mar 4, 2026
Merged

feat: add retry with exponential backoff in client.ts (DIS-143)#37
ckorhonen merged 5 commits intomainfrom
devin/1772640341-dis-143-retry-backoff

Conversation

@ckorhonen
Copy link
Collaborator

@ckorhonen ckorhonen commented Mar 4, 2026

feat: add retry with exponential backoff in client.ts (DIS-143)

Summary

Adds automatic retry with exponential backoff to OpenSeaClient for transient failures. Both get() and post() now delegate to a shared fetchWithRetry() private method that handles the retry loop.

Config additions (OpenSeaClientConfig):

  • maxRetries (default 0) — number of retries before giving up. SDK defaults to 0 (no retries) for backwards compatibility; CLI defaults to 3 via --max-retries.
  • retryBaseDelay (default 1000ms) — base delay for exponential backoff

CLI flags:

  • --max-retries <n> — override max retries (default "3")
  • --no-retry — disable retries entirely (sets maxRetries to 0)

Retry behavior:

  • GET requests: retries on 429 (rate limit) and 5xx (server error)
  • POST requests: retries on 429 only (not 5xx, since POST is not idempotent)
  • Other 4xx errors throw immediately regardless of method
  • Delay = max(Retry-After header, baseDelay × 2^attempt) + random jitter
  • Retry-After parsed as seconds or HTTP-date
  • Each retry attempt gets a fresh AbortSignal.timeout so the per-request timeout applies independently per attempt
  • Response bodies are safely cancelled on retryable errors to release sockets back to the connection pool
  • Retry attempts logged to stderr when --verbose is enabled

Updates since last revision

  • Resolved merge conflicts with main which introduced healthCommand (DIS-144), --fields/--max-lines options (DIS-145), getApiKeyPrefix(), and User-Agent header (DIS-41).
  • Updated CLI error test mocks (cli-api-error, cli-network-error, cli-rate-limit) to include the new healthCommand export.
  • get() and post() now use this.defaultHeaders (which includes User-Agent) instead of inline header objects.

Review & Testing Checklist for Human

  • POST idempotency: Verify that POST requests are only retried on 429, never on 5xx. The isRetryableStatus(status, method) function gates this — confirm via the test "does not retry post requests on 5xx".
  • SDK backwards compatibility: maxRetries defaults to 0 in client.ts, so new OpenSeaClient({apiKey}) has no retries (same as before). The CLI passes maxRetries: 3 explicitly. Confirm no SDK consumers are broken.
  • Retry-After HTTP-date parsing: parseRetryAfter uses Date.parse() for HTTP-date format. Only the numeric-seconds path is unit tested. Verify this is acceptable or add a test for the date path.
  • Manual E2E test: Run opensea collections get <slug> --verbose against the live API to confirm retry logging and normal request flow. Also verify --no-retry disables retries and that --max-retries 0 behaves equivalently.

Notes


Open with Devin

@devin-ai-integration
Copy link
Contributor

Original prompt from Chris K
Please work on ticket "[opensea-cli] Add retry with exponential backoff in client.ts" ([DIS-143](https://linear.app/opensea/issue/DIS-143/opensea-cli-add-retry-with-exponential-backoff-in-clientts))

PLAYBOOK_md:
# Ticket to PR

## Overview

This playbook guides the process of taking a Linear ticket from initial scoping through implementation to final review. The workflow ensures proper context gathering, quality implementation, and thorough code review before delivery. The agent uses the Linear MCP to manage ticket status and communication throughout.

## What's Needed From User

- Linear ticket URL or ticket ID (e.g., `ENG-123` or `https://linear.app/team/issue/ENG-123/...`)
- Repository access for the codebase where changes will be made

<phase name="Disambiguation" id="1">
## Disambiguation Phase

Think about the full user intent. Tickets are sometimes sparse. Make sure you disambiguate to the full scope that the user intended.

1. Fetch the ticket details using the Linear MCP `get_issue` tool with the ticket ID
2. Before diving into code: use the devin MCP to get a high-level understanding of the relevant systems and architecture. Use `ask_question` to learn about the relevant systems – send queries for multiple repos that could be relevant to get the full picture. Use `read_wiki_contents` to then get a better understanding how different parts of the codebase connect to each other.
3. Gather additional context to understand what the ticket means and refers to:
   - Look at past tickets in the same project and from the same author to understand patterns and terminology
   - Search for related commits and PRs (by author and content) that may provide context on the affected systems
   - Check any linked documents, designs, or parent tickets
   - Investigate the actual code
4. Identify any ambiguity in what the ticket refers to or asks for, including jargon or project-specific terms and use all means necessary to answer this yourself
5. Consult your smart friend: p... (8121 chars truncated...)

@devin-ai-integration
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@devin-ai-integration devin-ai-integration bot marked this pull request as ready for review March 4, 2026 16:10
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

Copy link
Collaborator Author

@ckorhonen ckorhonen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Overall: Request changes — All 162 tests pass and E2E works, but there are correctness issues that need code changes.

Critical Issues

  1. AbortSignal reuse across retries — The same AbortSignal.timeout(this.timeoutMs) is created once in get()/post() and shared across all retry attempts. The timeout clock runs from the first attempt, not per-retry. With 30s timeout and 3 retries + backoff, later retries will be aborted prematurely. Fix: create a fresh signal per attempt inside fetchWithRetry().

  2. POST requests are retried on 5xx by default — POST is generally not idempotent. Retrying on a 500 could cause duplicate resource creation. Fix: only retry POST on 429 (request was never processed), not on 5xx.

Major Issues

  1. Default maxRetries=3 is a breaking change for SDK consumers — Anyone using new OpenSeaClient({apiKey}) as a library will now get automatic retries, changing error timing and fetch call counts. Fix: default to maxRetries: 0 for the SDK, keep CLI default at 3.

  2. response.body?.cancel() should be wrapped in try-catch — The stream may already be disturbed, which would throw and mask the retry logic.

Minor Suggestions

  1. Rename retryBaseDelayretryBaseDelayMs for self-documenting units
  2. Cap Retry-After header values at a reasonable maximum (e.g., 60s) to prevent indefinite stalls
  3. Validate --max-retries and --no-retry mutual exclusivity

I've pushed a commit addressing issues 1-4.

@devin-ai-integration
Copy link
Contributor

Addressed all four issues in 0e3538a:

  1. AbortSignal reuse — already fixed in 93a665e (fresh signal per attempt)
  2. POST idempotencyisRetryableStatus now checks method; POST only retries on 429, not 5xx
  3. SDK defaultDEFAULT_MAX_RETRIES changed to 0; CLI still passes 3 explicitly via --max-retries
  4. Stream cancel safetyresponse.body?.cancel() wrapped in try-catch

Added test for "does not retry post requests on 5xx". All 161 tests pass.

devin-ai-integration bot and others added 5 commits March 4, 2026 15:41
Co-Authored-By: Chris K <ckorhonen@gmail.com>
…eam cancel safety)

Co-Authored-By: Chris K <ckorhonen@gmail.com>
The healthCommand was added to src/commands/index.ts via PR #35 but
these test files were not updated to include it in their mock.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@ckorhonen ckorhonen force-pushed the devin/1772640341-dis-143-retry-backoff branch from 3e54b83 to e4a6fbc Compare March 4, 2026 20:43
@ckorhonen ckorhonen merged commit c4f32b5 into main Mar 4, 2026
5 checks passed
@ckorhonen ckorhonen deleted the devin/1772640341-dis-143-retry-backoff branch March 4, 2026 20:48
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.

2 participants