Skip to content

feat: add exponential backoff retry for transient SDK errors #127

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
haripatel07 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat: add exponential backoff retry for transient SDK errors #127

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
87f49e0
feat: add exponential backoff retry for transient SDK errors
haripatel07 Mar 5, 2026
4cf8e56
fix: address review nits - clean retry loop, tighter MCP filter, fiel…
haripatel07 Mar 5, 2026
0896eac
fix: reset messages per retry, document timeout bypass and ge=0 delay…
haripatel07 Mar 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 47 additions & 5 deletions src/claude/sdk_integration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,16 @@ def __init__(
else:
logger.info("No API key provided, using existing Claude CLI authentication")

def _is_retryable_error(self, exc: BaseException) -> bool:
"""Return True for transient errors that warrant a retry.
asyncio.TimeoutError is intentional (user-configured timeout) — not retried.
Only non-MCP CLIConnectionError is considered transient.
"""
if isinstance(exc, CLIConnectionError):
msg = str(exc).lower()
return "mcp" not in msg and "server" not in msg
return False
Comment on lines +149 to +157
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

_is_retryable_error() determines MCP vs non-MCP by substring matching on str(exc), which is brittle and duplicates the MCP-detection logic used later when translating CLIConnectionError into ClaudeMCPError. To reduce the chance of misclassification and keep behavior consistent, consider centralizing this classification (single helper used for both retry decision and final exception mapping), or using structured attributes from CLIConnectionError if available.

Copilot uses AI. Check for mistakes.

async def execute_command(
self,
prompt: str,
Expand Down Expand Up @@ -288,11 +298,43 @@ async def _run_client() -> None:
finally:
await client.disconnect()

# Execute with timeout
await asyncio.wait_for(
_run_client(),
timeout=self.config.claude_timeout_seconds,
)
# Execute with timeout, retrying on transient CLIConnectionError
max_attempts = max(1, self.config.claude_retry_max_attempts)
last_exc: Optional[BaseException] = None

for attempt in range(max_attempts):
if attempt > 0:
delay = min(
self.config.claude_retry_base_delay
* (self.config.claude_retry_backoff_factor ** (attempt - 1)),
self.config.claude_retry_max_delay,
)
logger.warning(
"Retrying Claude SDK command",
attempt=attempt + 1,
max_attempts=max_attempts,
delay_seconds=delay,
)
await asyncio.sleep(delay)
try:
await asyncio.wait_for(
_run_client(),
timeout=self.config.claude_timeout_seconds,
)
break # success — exit retry loop
except CLIConnectionError as exc:
if self._is_retryable_error(exc) and attempt < max_attempts - 1:
last_exc = exc
logger.warning(
"Transient connection error, will retry",
attempt=attempt + 1,
error=str(exc),
)
continue
raise # non-retryable or attempts exhausted
Comment on lines +301 to +343
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

There are existing unit tests for ClaudeSDKManager.execute_command(), but no tests cover the new retry behavior (e.g., a transient non-MCP CLIConnectionError that succeeds on a subsequent attempt, and that claude_retry_max_attempts=0/1 results in no retries). Adding tests here would help prevent regressions in retry/backoff and logging behavior.

Copilot uses AI. Check for mistakes.
else:
if last_exc is not None:
raise last_exc
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

The retry loop’s for ... else block is effectively unreachable: max_attempts is forced to be >= 1, and on the final failed attempt the code raises inside the except, so the loop will never fall through to else. This also makes last_exc unnecessary and potentially misleading. Consider simplifying by removing last_exc/else, or restructuring to only raise after the loop completes.

Copilot uses AI. Check for mistakes.

# Extract cost, tools, and session_id from result message
cost = 0.0
Expand Down
19 changes: 19 additions & 0 deletions src/config/settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@
DEFAULT_RATE_LIMIT_BURST,
DEFAULT_RATE_LIMIT_REQUESTS,
DEFAULT_RATE_LIMIT_WINDOW,
DEFAULT_RETRY_BACKOFF_FACTOR,
DEFAULT_RETRY_BASE_DELAY,
DEFAULT_RETRY_MAX_ATTEMPTS,
DEFAULT_RETRY_MAX_DELAY,
DEFAULT_SESSION_TIMEOUT_HOURS,
)

Expand Down Expand Up @@ -121,6 +125,21 @@ class Settings(BaseSettings):
description="List of explicitly disallowed Claude tools/commands",
)

# Retry settings
claude_retry_max_attempts: int = Field(
DEFAULT_RETRY_MAX_ATTEMPTS,
description="Max retry attempts for transient SDK errors (0 = disabled)",
)
claude_retry_base_delay: float = Field(
DEFAULT_RETRY_BASE_DELAY, description="Base delay in seconds between retries"
)
claude_retry_backoff_factor: float = Field(
DEFAULT_RETRY_BACKOFF_FACTOR, description="Exponential backoff multiplier"
)
claude_retry_max_delay: float = Field(
DEFAULT_RETRY_MAX_DELAY, description="Maximum delay cap in seconds"
)

Comment on lines +132 to +155
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

These retry-related settings are user-configurable but currently have no bounds validation. Negative values (e.g., base_delay/max_delay) can lead to runtime failures (e.g., asyncio.sleep() with a negative delay) or unexpected backoff behavior. Add appropriate ge/gt constraints (and potentially cross-field validation like max_delay >= base_delay) for these fields.

Suggested change
description="Max retry attempts for transient SDK errors (0 = disabled)",
)
claude_retry_base_delay: float = Field(
DEFAULT_RETRY_BASE_DELAY, description="Base delay in seconds between retries"
)
claude_retry_backoff_factor: float = Field(
DEFAULT_RETRY_BACKOFF_FACTOR, description="Exponential backoff multiplier"
)
claude_retry_max_delay: float = Field(
DEFAULT_RETRY_MAX_DELAY, description="Maximum delay cap in seconds"
)
ge=0,
description="Max retry attempts for transient SDK errors (0 = disabled)",
)
claude_retry_base_delay: float = Field(
DEFAULT_RETRY_BASE_DELAY,
ge=0,
description="Base delay in seconds between retries",
)
claude_retry_backoff_factor: float = Field(
DEFAULT_RETRY_BACKOFF_FACTOR,
gt=0,
description="Exponential backoff multiplier",
)
claude_retry_max_delay: float = Field(
DEFAULT_RETRY_MAX_DELAY,
ge=0,
description="Maximum delay cap in seconds",
)
@model_validator(mode="after")
def validate_retry_delays(self) -> "Settings":
"""Ensure retry delay configuration is internally consistent."""
if self.claude_retry_max_delay < self.claude_retry_base_delay:
raise ValueError(
"claude_retry_max_delay must be greater than or equal to "
"claude_retry_base_delay"
)
return self

Copilot uses AI. Check for mistakes.
# Sandbox settings
sandbox_enabled: bool = Field(
True,
Expand Down
6 changes: 6 additions & 0 deletions src/utils/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,5 +85,11 @@
DEFAULT_CLAUDE_BINARY = "claude"
DEFAULT_CLAUDE_OUTPUT_FORMAT = "stream-json"

# Retry defaults
DEFAULT_RETRY_MAX_ATTEMPTS = 3
DEFAULT_RETRY_BASE_DELAY = 1.0
DEFAULT_RETRY_BACKOFF_FACTOR = 3.0
DEFAULT_RETRY_MAX_DELAY = 30.0

# Logging
LOG_FORMAT = "%(asctime)s - %(name)s - %(levelname)s - %(message)s"
Loading