Skip to content

Commit 6c4c1a9

Browse files
committed
fix(retry): add 408 to retryable status codes, align docs, add POST coverage tests
## Purpose Legacy AGENTS.md incorrectly documented that only GET/HEAD requests are retried. The implementation has always retried all HTTP methods (no method filter exists in _RetryTransport). Additionally, 408 Request Timeout was missing from the default retryable status codes despite being a well-known retry-safe transient signal (RFC 7231 §6.5.7). Test coverage for POST-specific retry scenarios was absent, creating a regression risk. ## Solution - Added `408` to `RetryConfig.retryable_status_codes` default in `_internal/config.py`. - Corrected AGENTS.md retry-behavior paragraph to document that all HTTP methods are retried, with rationale (Pinecone data-plane writes are idempotent by ID). - Added 10 new targeted test cases in `tests/unit/_internal/test_retry_transport.py`: POST upsert/query retry on ConnectError and 503, 408 retry, 429+Retry-After, non-retryable 400, exhaustion returning 503 or raising ConnectError; plus async mirrors for upsert/query/408. - Created `tests/unit/_internal/test_http_client.py` with fast-path POST retry guard: verifies that `HTTPClient.post()` still goes through `_RetryTransport` even though the fast path calls `_client._transport.handle_request()` directly. - Updated `tests/unit/test_retry_config.py` to reflect the new 408 default. ## Follow-ups gRPC retry policy broadening (ResourceExhausted, Aborted) is covered by P-0228.
1 parent 8db5de8 commit 6c4c1a9

5 files changed

Lines changed: 183 additions & 5 deletions

File tree

‎AGENTS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ except PineconeError as e:
283283
print(e) # Validation, timeout, or connection error
284284
```
285285

286-
**Retry behavior:** Only GET/HEAD requests are automatically retried (on 500/502/503/504). POST operations (upsert, query, search) are not retried — implement your own retry logic for transient errors. Configure via `RetryConfig`.
286+
**Retry behavior:** All HTTP methods (GET, HEAD, POST, PUT, PATCH, DELETE) are automatically retried on transient failures: connection errors (`httpx.TransportError`), 408 Request Timeout, 429 Too Many Requests (honoring `Retry-After`), and 5xx (500, 502, 503, 504). Pinecone's data-plane writes are idempotent at the server (upsert overwrites by ID, delete-by-ID is idempotent, update-by-ID is idempotent), so retrying upsert/query/fetch/delete/update on transient errors is safe. Backoff is floored full jitter: `uniform(0.1 * base, base)` where `base = min(backoff_factor**attempt, max_wait)`. Configure via `RetryConfig`.
287287

288288
## Response Objects
289289

‎pinecone/_internal/config.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,14 +80,14 @@ class RetryConfig:
8080
backoff_factor: Exponential backoff base multiplier in seconds. Defaults to 2.0.
8181
max_wait: Maximum backoff delay in seconds. Defaults to 60.0.
8282
retryable_status_codes: HTTP status codes that trigger a retry. Defaults to
83-
``{429, 500, 502, 503, 504}``.
83+
``{408, 429, 500, 502, 503, 504}``.
8484
"""
8585

8686
max_retries: int = 3
8787
backoff_factor: float = 2.0
8888
max_wait: float = 60.0
8989
retryable_status_codes: frozenset[int] = field(
90-
default_factory=lambda: frozenset({429, 500, 502, 503, 504})
90+
default_factory=lambda: frozenset({408, 429, 500, 502, 503, 504})
9191
)
9292

9393

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
"""Tests for HTTPClient.post fast-path retry behavior."""
2+
3+
from __future__ import annotations
4+
5+
from unittest.mock import MagicMock
6+
7+
import httpx
8+
import pytest
9+
10+
from pinecone._internal.config import PineconeConfig, RetryConfig
11+
from pinecone._internal.http_client import HTTPClient
12+
13+
14+
def _make_client(max_retries: int = 2) -> HTTPClient:
15+
retry_config = RetryConfig(max_retries=max_retries, backoff_factor=0.0, max_wait=0.0)
16+
config = PineconeConfig(
17+
api_key="test-key-1234",
18+
host="https://test.example.com",
19+
retry_config=retry_config,
20+
)
21+
return HTTPClient(config, "2025-10")
22+
23+
24+
def test_fast_path_post_retried_on_connect_error() -> None:
25+
"""Fast-path POST bypasses Client.send() but still goes through _RetryTransport."""
26+
client = _make_client(max_retries=2)
27+
28+
mock_inner = MagicMock(spec=httpx.BaseTransport)
29+
mock_inner.handle_request.side_effect = [
30+
httpx.ConnectError("connection refused"),
31+
httpx.Response(200, content=b"{}"),
32+
]
33+
# Swap the inner transport inside _RetryTransport so retry logic wraps the mock.
34+
client._client._transport._transport = mock_inner # type: ignore[attr-defined]
35+
36+
result = client.post("/some/path", json={"foo": "bar"})
37+
assert result.status_code == 200
38+
assert mock_inner.handle_request.call_count == 2
39+
40+
41+
def test_fast_path_post_succeeds_on_first_try() -> None:
42+
client = _make_client()
43+
44+
mock_inner = MagicMock(spec=httpx.BaseTransport)
45+
mock_inner.handle_request.return_value = httpx.Response(200, content=b"{}")
46+
client._client._transport._transport = mock_inner # type: ignore[attr-defined]
47+
48+
result = client.post("/vectors/upsert", json={"vectors": []})
49+
assert result.status_code == 200
50+
assert mock_inner.handle_request.call_count == 1
51+
52+
53+
@pytest.mark.asyncio
54+
async def test_fast_path_uses_retry_transport_not_client_send() -> None:
55+
"""Regression guard: if _RetryTransport is bypassed, retry count would be 1."""
56+
client = _make_client(max_retries=3)
57+
58+
mock_inner = MagicMock(spec=httpx.BaseTransport)
59+
mock_inner.handle_request.side_effect = [
60+
httpx.ConnectError("reset by peer"),
61+
httpx.ConnectError("reset by peer"),
62+
httpx.Response(200, content=b"{}"),
63+
]
64+
client._client._transport._transport = mock_inner # type: ignore[attr-defined]
65+
66+
result = client.post("/query", json={"vector": [0.1, 0.2]})
67+
assert result.status_code == 200
68+
assert mock_inner.handle_request.call_count == 3

‎tests/unit/_internal/test_retry_transport.py‎

Lines changed: 111 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
from __future__ import annotations
44

5-
from unittest.mock import AsyncMock, MagicMock
5+
from unittest.mock import AsyncMock, MagicMock, patch
66

77
import httpx
88
import pytest
@@ -104,3 +104,113 @@ async def test_async_non_retryable_status_returns_immediately() -> None:
104104
result = await rt.handle_async_request(_req())
105105
assert result.status_code == 400
106106
assert inner.handle_async_request.call_count == 1
107+
108+
109+
# --- POST-specific named tests (sync) ---
110+
111+
112+
def test_post_upsert_retried_on_transport_error() -> None:
113+
rt, inner = _transport(max_retries=3)
114+
inner.handle_request.side_effect = [
115+
httpx.ConnectError("boom"),
116+
httpx.Response(200),
117+
]
118+
result = rt.handle_request(httpx.Request("POST", "https://example.com/vectors/upsert"))
119+
assert result.status_code == 200
120+
assert inner.handle_request.call_count == 2
121+
122+
123+
def test_post_query_retried_on_503() -> None:
124+
rt, inner = _transport(max_retries=3)
125+
inner.handle_request.side_effect = [httpx.Response(503), httpx.Response(200)]
126+
result = rt.handle_request(httpx.Request("POST", "https://example.com/query"))
127+
assert result.status_code == 200
128+
assert inner.handle_request.call_count == 2
129+
130+
131+
def test_post_retried_on_408() -> None:
132+
rt, inner = _transport(max_retries=3)
133+
inner.handle_request.side_effect = [httpx.Response(408), httpx.Response(200)]
134+
result = rt.handle_request(httpx.Request("POST", "https://example.com/query"))
135+
assert result.status_code == 200
136+
assert inner.handle_request.call_count == 2
137+
138+
139+
def test_post_retried_on_429_with_retry_after() -> None:
140+
rt, inner = _transport(max_retries=3)
141+
inner.handle_request.side_effect = [
142+
httpx.Response(429, headers={"Retry-After": "0"}),
143+
httpx.Response(200),
144+
]
145+
with patch("pinecone._internal.http_client.time.sleep") as mock_sleep:
146+
result = rt.handle_request(httpx.Request("POST", "https://example.com/query"))
147+
assert result.status_code == 200
148+
assert inner.handle_request.call_count == 2
149+
mock_sleep.assert_called_once_with(0.0)
150+
151+
152+
def test_post_not_retried_on_400() -> None:
153+
rt, inner = _transport(max_retries=3)
154+
inner.handle_request.return_value = httpx.Response(400)
155+
result = rt.handle_request(httpx.Request("POST", "https://example.com/query"))
156+
assert result.status_code == 400
157+
assert inner.handle_request.call_count == 1
158+
159+
160+
def test_post_exhausts_retries_then_returns_503() -> None:
161+
rt, inner = _transport(max_retries=3)
162+
inner.handle_request.side_effect = [
163+
httpx.Response(503),
164+
httpx.Response(503),
165+
httpx.Response(503),
166+
]
167+
result = rt.handle_request(httpx.Request("POST", "https://example.com/query"))
168+
assert result.status_code == 503
169+
assert inner.handle_request.call_count == 3
170+
171+
172+
def test_post_exhausts_retries_then_raises_transport_error() -> None:
173+
rt, inner = _transport(max_retries=3)
174+
inner.handle_request.side_effect = [
175+
httpx.ConnectError("fail"),
176+
httpx.ConnectError("fail"),
177+
httpx.ConnectError("fail"),
178+
]
179+
with pytest.raises(httpx.ConnectError):
180+
rt.handle_request(httpx.Request("POST", "https://example.com/query"))
181+
assert inner.handle_request.call_count == 3
182+
183+
184+
# --- POST-specific named tests (async) ---
185+
186+
187+
@pytest.mark.asyncio
188+
async def test_async_post_upsert_retried_on_transport_error() -> None:
189+
rt, inner = _async_transport(max_retries=3)
190+
inner.handle_async_request.side_effect = [
191+
httpx.ConnectError("boom"),
192+
httpx.Response(200),
193+
]
194+
result = await rt.handle_async_request(
195+
httpx.Request("POST", "https://example.com/vectors/upsert")
196+
)
197+
assert result.status_code == 200
198+
assert inner.handle_async_request.call_count == 2
199+
200+
201+
@pytest.mark.asyncio
202+
async def test_async_post_query_retried_on_503() -> None:
203+
rt, inner = _async_transport(max_retries=3)
204+
inner.handle_async_request.side_effect = [httpx.Response(503), httpx.Response(200)]
205+
result = await rt.handle_async_request(httpx.Request("POST", "https://example.com/query"))
206+
assert result.status_code == 200
207+
assert inner.handle_async_request.call_count == 2
208+
209+
210+
@pytest.mark.asyncio
211+
async def test_async_post_retried_on_408() -> None:
212+
rt, inner = _async_transport(max_retries=3)
213+
inner.handle_async_request.side_effect = [httpx.Response(408), httpx.Response(200)]
214+
result = await rt.handle_async_request(httpx.Request("POST", "https://example.com/query"))
215+
assert result.status_code == 200
216+
assert inner.handle_async_request.call_count == 2

‎tests/unit/test_retry_config.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ def test_default_retry_config(self) -> None:
7474
assert cfg.max_retries == 3
7575
assert cfg.backoff_factor == 2.0
7676
assert cfg.max_wait == 60.0
77-
assert cfg.retryable_status_codes == frozenset({429, 500, 502, 503, 504})
77+
assert cfg.retryable_status_codes == frozenset({408, 429, 500, 502, 503, 504})
7878

7979
def test_retry_config_has_no_retryable_methods(self) -> None:
8080
cfg = RetryConfig()

0 commit comments

Comments
 (0)