Conversation
WalkthroughThe pull request consolidates HTTP exception handling in both sync and async HTTP clients. Multiple Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdks/python/src/honcho/http/client.py (1)
134-147:⚠️ Potential issue | 🔴 CriticalGuard retries for non-idempotent methods on transport/protocol failures.
httpx.NetworkErrorsubclasses (WriteError,ReadError,CloseError) andhttpx.RemoteProtocolErrorcan occur after the request bytes have been sent to the server. Retrying these blindly causes duplicate writes forPOST,PATCH, and other non-idempotent methods. The current code at lines 135–147 retries all transport errors without checking method idempotency.Proposed fix
) as e: if isinstance(e, httpx.TimeoutException): error = TimeoutError(f"Request timed out after {request_timeout}s") else: error = ConnectionError(f"Connection error: {e}") + + method_is_idempotent = method.upper() in { + "GET", + "HEAD", + "OPTIONS", + "PUT", + "DELETE", + } + has_idempotency_key = any( + key.lower() == "idempotency-key" for key in request_headers + ) + if attempt < self.max_retries: + if not method_is_idempotent and not has_idempotency_key: + raise error from e last_error = error time.sleep(self._get_retry_delay(attempt)) attempt += 1 continue raise error from e🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdks/python/src/honcho/http/client.py` around lines 134 - 147, The retry logic currently retries httpx.NetworkError and httpx.RemoteProtocolError for all HTTP methods; change it so only httpx.TimeoutException is always retried, while NetworkError/RemoteProtocolError are retried only for idempotent methods. In the except block (where variables attempt, self.max_retries, request_timeout, and self._get_retry_delay are used and the local variable request represents the outgoing Request), detect the HTTP method via request.method.upper() and only set last_error and sleep/retry when the method is idempotent (e.g., GET, HEAD, OPTIONS, PUT, DELETE); for non-idempotent methods (POST, PATCH, etc.) re-raise or return the constructed ConnectionError immediately instead of retrying. Keep the existing error messages (TimeoutError vs ConnectionError) but guard retries for protocol/transport errors by method idempotency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@sdks/python/src/honcho/http/client.py`:
- Around line 134-147: The retry logic currently retries httpx.NetworkError and
httpx.RemoteProtocolError for all HTTP methods; change it so only
httpx.TimeoutException is always retried, while NetworkError/RemoteProtocolError
are retried only for idempotent methods. In the except block (where variables
attempt, self.max_retries, request_timeout, and self._get_retry_delay are used
and the local variable request represents the outgoing Request), detect the HTTP
method via request.method.upper() and only set last_error and sleep/retry when
the method is idempotent (e.g., GET, HEAD, OPTIONS, PUT, DELETE); for
non-idempotent methods (POST, PATCH, etc.) re-raise or return the constructed
ConnectionError immediately instead of retrying. Keep the existing error
messages (TimeoutError vs ConnectionError) but guard retries for
protocol/transport errors by method idempotency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fdf74c38-16ea-4eb1-941b-07012ec3f427
📒 Files selected for processing (2)
sdks/python/src/honcho/http/async_client.pysdks/python/src/honcho/http/client.py
VVoruganti
left a comment
There was a problem hiding this comment.
Increment the versions by a patch 2.1.1 and update changelogs
Summary by CodeRabbit
Bug Fixes
Refactor