Skip to content

Commit 60d6c7a

Browse files
committed
fix(http): stop replaying POST on a gateway 5xx
HotdataClient passed its own `retries=` into Configuration, which replaces the generated SDK's retry policy wholesale. That policy listed POST in `allowed_methods` alongside a (502, 503, 504) forcelist, so an intermediary timing out a long request produced a silent, identical re-POST while the server was still working on the first one. For a non-idempotent call — a data load — the duplicate collides with the write lock the original holds and is refused, and the caller sees contention rather than the timeout that actually happened. The override is removed so the SDK's own default applies. It is the policy this wrapper was reaching for: hotdata._retry retries a pre-response connection reset on any method (the stale pooled socket case, where the server did no work) while leaving read timeouts and status retries idempotent-only. hotdata_framework/http.py existed only to build that policy and had no other callers, so it goes with it.
1 parent 08bd929 commit 60d6c7a

4 files changed

Lines changed: 84 additions & 19 deletions

File tree

CHANGELOG.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- A `POST` is no longer replayed because of a response status. `HotdataClient`
13+
passed its own `retries=` into `Configuration`, which replaced the generated
14+
SDK's policy wholesale with one listing `POST` in `allowed_methods` alongside
15+
a `(502, 503, 504)` forcelist — so an intermediary timing out a long request
16+
produced a silent, identical re-`POST` while the server was still working on
17+
the first one. For a load that is not idempotent: the duplicate collides with
18+
the write lock the original holds and is refused.
19+
20+
The override is removed and the SDK's own default now applies. It is the
21+
policy this wrapper was reaching for — `hotdata._retry` retries a
22+
*pre-response* connection reset (the stale pooled socket case, where the
23+
server did no work) on any method, while leaving read timeouts and status
24+
retries idempotent-only.
25+
26+
### Removed
27+
28+
- `hotdata_framework.http` and `default_http_retries()`. The module existed only
29+
to build the retry policy above and had no other callers. It predates
30+
`hotdata._retry`, which supersedes it.
31+
1032

1133
## [0.11.0] - 2026-08-11
1234

hotdata_framework/client.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@
5252
normalize_host,
5353
pick_workspace,
5454
)
55-
from hotdata_framework.http import default_http_retries
5655
from hotdata_framework.result import QueryResult
5756

5857
# Load modes the managed-table endpoint accepts: replace overwrites, append adds
@@ -154,11 +153,16 @@ def __init__(
154153
self._host = normalize_host(host) if host else default_host()
155154
self._api_key = api_key
156155
self._workspace_id = workspace_id
156+
# No `retries=`: the generated SDK's own default is the correct policy
157+
# and passing one here replaces it wholesale. `hotdata._retry` retries a
158+
# pre-response connection reset on any method — the stale pooled socket
159+
# case this wrapper was reaching for — while leaving read timeouts and
160+
# status retries idempotent-only, so a POST that may have reached the
161+
# server is never replayed.
157162
self._config = Configuration(
158163
host=self._host,
159164
api_key=api_key,
160165
workspace_id=workspace_id,
161-
retries=default_http_retries(),
162166
)
163167
self._api = ApiClient(self._config)
164168
if request_timeout is not None:

hotdata_framework/http.py

Lines changed: 0 additions & 17 deletions
This file was deleted.

tests/test_retry_policy.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
"""A POST must never be replayed because of a response status.
2+
3+
A load is not idempotent: re-sending one that the server is still working on
4+
collides with the write lock the first attempt holds, and the duplicate is
5+
refused. The generated SDK already draws this line — ``hotdata._retry`` retries
6+
a *pre-response* connection reset on any method (the stale pooled socket case,
7+
where the server did no work) while leaving read timeouts and status retries
8+
idempotent-only. This wrapper used to pass its own ``retries=`` into
9+
``Configuration``, which replaced that policy wholesale with one that listed
10+
POST alongside a 502/503/504 forcelist — so a gateway timing out a long request
11+
produced a silent duplicate of it.
12+
13+
These tests pin the resulting policy rather than the absence of an argument,
14+
so re-introducing an override that is unsafe for POST fails here.
15+
"""
16+
17+
from __future__ import annotations
18+
19+
from hotdata_framework.client import HotdataClient
20+
21+
WS = "work_test0000000000000000000000"
22+
23+
24+
def _retry_policy():
25+
client = HotdataClient(api_key="hd_test", workspace_id=WS, host="https://example.invalid")
26+
return client._config.retries
27+
28+
29+
def test_no_status_forcelist() -> None:
30+
"""A status code means the request reached the server, so retrying one is a
31+
decision about idempotency that the transport layer cannot make."""
32+
retry = _retry_policy()
33+
assert not retry.status_forcelist, retry.status_forcelist
34+
assert retry.status == 0
35+
36+
37+
def test_post_is_not_read_or_status_retried() -> None:
38+
"""``allowed_methods`` gates both read and status retries. POST outside it is
39+
what stops a request the server may already be processing from being sent
40+
twice."""
41+
retry = _retry_policy()
42+
assert "POST" not in retry.allowed_methods
43+
assert "GET" in retry.allowed_methods
44+
45+
46+
def test_pre_response_connection_reset_still_retries_any_method() -> None:
47+
"""The case the wrapper's own override was reaching for, kept: a reset before
48+
any response means the request never landed, so a POST retry is safe."""
49+
from urllib3.exceptions import ProtocolError
50+
51+
retry = _retry_policy()
52+
cause = ConnectionResetError(54, "Connection reset by peer")
53+
reset = ProtocolError("Connection aborted.", cause)
54+
assert retry._is_connection_error(reset)
55+
# A read timeout is NOT reclassified — the server may have done the work.
56+
assert not retry._is_connection_error(ProtocolError("Connection aborted.", None))

0 commit comments

Comments
 (0)