Skip to content

Commit 34d1b02

Browse files
committed
Fix the reset_client not working issue.
1 parent dc644b9 commit 34d1b02

8 files changed

Lines changed: 143 additions & 55 deletions

File tree

python_agent_harness/agent.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -635,6 +635,13 @@ def safe_delta(text: str) -> None:
635635
# sub-agents must not stream into the parent's live
636636
# stream row — their text is private until returned
637637
on_delta=(safe_delta if self.top_level else None),
638+
# a connection error mid-stream discards the partial
639+
# output and retries on a fresh client: tell the TUI to
640+
# clear the partial text and show that the request is
641+
# being restarted
642+
on_retry=(
643+
(lambda: session.notify("retry")) if self.top_level else None
644+
),
638645
# poll cancellation during retry backoff so Ctrl-C
639646
# aborts promptly instead of after the full sleep
640647
cancel_check=self._is_cancelled,

python_agent_harness/client.py

Lines changed: 39 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,11 @@ def __init__(
171171
config.API_RETRY_MAX_DELAY if retry_max_delay is None else retry_max_delay
172172
)
173173
self._http = httpx.Client(timeout=timeout, verify=self.verify)
174+
# True while the in-flight request was aborted (Ctrl-C): a
175+
# connection error on an aborted request must NOT be retried —
176+
# the user asked to stop. Cleared at the start of each chat()
177+
# so a fresh turn may retry normally.
178+
self._aborted = False
174179
self.log_path: Path | None = _llm_log_path() if config.LLM_LOG_ENABLED else None
175180

176181
def close(self) -> None:
@@ -188,6 +193,7 @@ def abort(self) -> None:
188193
the loop treats as a cancel) and then close the pool. A fresh
189194
client is swapped in for the next request.
190195
"""
196+
self._aborted = True
191197
old = self._http
192198
self._http = httpx.Client(timeout=self.timeout, verify=self.verify)
193199
try:
@@ -202,9 +208,9 @@ def abort(self) -> None:
202208
def _reset_http(self) -> None:
203209
"""Replace the httpx client with a fresh instance.
204210
205-
Called after connection-level retries are exhausted so a
206-
poisoned pool (stale/dead connections) does not doom every
207-
subsequent request in the session.
211+
Called before every connection-error retry (and on exhaustion)
212+
so a poisoned pool (stale/dead connections) never dooms the
213+
retry itself or every subsequent request in the session.
208214
"""
209215
old = self._http
210216
self._http = httpx.Client(timeout=self.timeout, verify=self.verify)
@@ -267,6 +273,7 @@ def chat(
267273
on_tool_call: Callable[[str, str, str], None] | None = None,
268274
stream: bool = True,
269275
cancel_check: Callable[[], bool] | None = None,
276+
on_retry: Callable[[], None] | None = None,
270277
) -> tuple[Message, Usage]:
271278
"""Send a chat request, return (assistant msg, usage).
272279
@@ -279,18 +286,27 @@ def chat(
279286
280287
Transient failures (HTTP 429 / 5xx, connection errors) are
281288
retried with exponential backoff + jitter up to ``retry_max``
282-
attempts, honoring ``Retry-After`` when present. A retry only
283-
happens before any delta has been delivered to the callbacks,
284-
so streaming output is never duplicated for the caller. Other
285-
4xx errors are permanent and fail immediately. ``cancel_check``
286-
(when given) is polled during backoff sleeps so an abort lands
287-
promptly instead of after the full wait.
289+
attempts, honoring ``Retry-After`` when present. A connection
290+
error always swaps in a fresh httpx client first (``_reset_http``)
291+
so a dead connection never poisons the retry — and a stream that
292+
died mid-body IS retried even when deltas already reached the
293+
callers: the partial stream is discarded on retry (``on_retry``
294+
lets the caller drop its live text), so nothing is duplicated in
295+
the returned message. Other 4xx errors are permanent and fail
296+
immediately. ``cancel_check`` (when given) is polled during
297+
backoff sleeps so an abort lands promptly instead of after the
298+
full wait. ``on_retry`` (when given) is invoked right before a
299+
retry after a connection error, so a UI can clear the partial
300+
output and show that the request is being restarted.
288301
"""
289302
payload = self._payload(
290303
messages, tools, stream=stream, temperature=temperature,
291304
max_tokens=max_tokens, system=system,
292305
reasoning_effort=reasoning_effort,
293306
)
307+
# a fresh turn may retry connection errors even if a previous
308+
# in-flight request was aborted (see abort/_aborted)
309+
self._aborted = False
294310
usage = Usage()
295311
emitted = False
296312

@@ -326,19 +342,22 @@ def wrap_tool_call(name: str, call_id: str, fragment: str) -> None:
326342
raise
327343
except httpx.HTTPError as e:
328344
# connection-level failures: connect errors, timeouts,
329-
# dropped streams — all transient unless a delta already
330-
# reached the caller (then a retry would duplicate it)
331-
if emitted or attempt >= self.retry_max:
332-
# Replace the client so a poisoned connection pool
333-
# does not doom all subsequent requests in this
334-
# session. Without this, a single network hiccup
335-
# can leave the session stuck in a permanent error
336-
# state (the dead connection stays in the pool and
337-
# keeps getting reused).
338-
self._reset_http()
345+
# dropped streams. Swap in a fresh client immediately —
346+
# a dead connection must not stay in the pool for the
347+
# retry — then retry the request, even when deltas
348+
# already reached the caller: the partial stream is
349+
# discarded on retry (on_retry lets the caller clear
350+
# its live text), so nothing is duplicated in the
351+
# stored message. Only give up once the per-request
352+
# attempt budget is exhausted — or immediately when
353+
# the request was aborted (Ctrl-C: the user asked to
354+
# stop, so a fresh attempt must not be started).
355+
self._reset_http()
356+
if self._aborted or attempt >= self.retry_max:
339357
raise ApiError(f"network error: {e}") from e
358+
if on_retry is not None:
359+
on_retry()
340360
if self._sleep_backoff(attempt, None, cancel_check):
341-
self._reset_http()
342361
raise ApiError(f"network error: {e}") from e
343362

344363
content = "".join(content_parts)

python_agent_harness/tui.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,14 @@ def _on_notify(self, kind: str, data: Any = None) -> None:
423423
elif kind == "compact":
424424
self.status = " compacted"
425425
self._history_dirty = True
426+
elif kind == "retry":
427+
# A connection error mid-stream: the client discarded the
428+
# partial response and is retrying on a fresh connection —
429+
# drop the partial stream text so the restarted stream
430+
# doesn't duplicate it on screen.
431+
with self.lock:
432+
self.stream_text = ""
433+
self.status = " connection lost — retrying"
426434
elif kind == "todos":
427435
# TodoWrite updated the task list: the cached history rows
428436
# (which include the Todos panel) must be rebuilt

tests/test_agent.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ def __init__(self, script):
2929

3030
def chat(self, messages, tools=None, system=None, temperature=None,
3131
max_tokens=None, reasoning_effort=None, on_delta=None, stream=True,
32-
cancel_check=None):
32+
cancel_check=None, on_retry=None):
3333
self.calls.append([m.to_api() for m in messages])
3434
self.kwargs.append({
3535
"tools": tools, "system": system, "temperature": temperature,

tests/test_client.py

Lines changed: 85 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -814,8 +814,10 @@ def test_connect_error_non_stream_retried_then_raises(self):
814814

815815
c = make_client(retry_max=2, retry_base_delay=0.01, retry_max_delay=0.05)
816816
try:
817+
# patch the class so the fresh client created by
818+
# _reset_http between attempts is covered too
817819
with mock.patch.object(
818-
c._http, "post", side_effect=httpx.ConnectError("refused")
820+
httpx.Client, "post", side_effect=httpx.ConnectError("refused")
819821
):
820822
with self.assertRaises(ApiError):
821823
c.chat([Message(role="user", content="hi")], stream=False)
@@ -835,46 +837,98 @@ def test_budget_exhausted_immediately_raises(self):
835837
finally:
836838
c.close()
837839

838-
def test_error_after_delta_not_retried(self):
839-
"""Once a delta reached the caller, a retry would duplicate it:
840-
a stream dying mid-body raises ApiError immediately (no backoff
841-
sleep, no second request)."""
840+
def test_error_after_delta_retried_on_fresh_client(self):
841+
"""A stream dying mid-body after deltas is retried on a fresh
842+
client: the partial stream is discarded, the retried response
843+
is returned, and nothing is duplicated in the stored message."""
844+
c = make_client(retry_max=2, retry_base_delay=0.01, retry_max_delay=0.01)
845+
old = c._http
846+
try:
847+
state = {"n": 0}
848+
849+
def flaky(self_, *a, **kw):
850+
state["n"] += 1
851+
if state["n"] == 1:
852+
return DieAfterDelta()
853+
resp = FakeStreamResp([
854+
'data: {"choices": [{"delta": {"content": "full"}}]}',
855+
"data: [DONE]",
856+
])
857+
return FakeStreamCM(resp)
858+
859+
# patch the class so the fresh client created by
860+
# _reset_http is covered too
861+
with mock.patch.object(httpx.Client, "stream", flaky):
862+
retries = []
863+
deltas = []
864+
msg, _ = c.chat(
865+
[Message(role="user", content="hi")],
866+
on_delta=deltas.append,
867+
on_retry=lambda: retries.append(1),
868+
)
869+
self.assertEqual(msg.content, "full")
870+
self.assertEqual(state["n"], 2)
871+
self.assertEqual(retries, [1])
872+
# the retry ran on a fresh (reset) client
873+
self.assertIsNot(c._http, old)
874+
# the caller saw the partial delta, but the stored message
875+
# only carries the retried response
876+
self.assertEqual(deltas, ["partial", "full"])
877+
finally:
878+
c.close()
879+
880+
def test_error_after_delta_retries_exhausted_then_raises(self):
881+
"""A stream dying mid-body after deltas retries (reset client
882+
each time); when the attempt budget is exhausted it raises
883+
ApiError — with the client left fresh for the next call."""
842884
from python_agent_harness.client import ApiError
843885

844-
c = make_client(retry_max=3, retry_base_delay=60.0, retry_max_delay=60.0)
886+
c = make_client(retry_max=2, retry_base_delay=0.01, retry_max_delay=0.01)
887+
old = c._http
845888
try:
846-
class DieAfterDelta:
847-
def __init__(self):
848-
self._sent = False
889+
with mock.patch.object(
890+
httpx.Client, "stream", return_value=DieAfterDelta()
891+
):
892+
retries = []
893+
with self.assertRaises(ApiError):
894+
c.chat(
895+
[Message(role="user", content="hi")],
896+
on_retry=lambda: retries.append(1),
897+
)
898+
self.assertEqual(retries, [1]) # one retry after the first drop
899+
self.assertIsNot(c._http, old) # pool was reset
900+
self.assertFalse(c._http.is_closed)
901+
finally:
902+
c.close()
849903

850-
def __enter__(self):
851-
return self
852904

853-
def __exit__(self, *exc):
854-
return False
905+
class DieAfterDelta:
906+
"""Streaming response that yields one delta then dies with a
907+
connection error (mimics a connection reset mid-body)."""
855908

856-
@property
857-
def status_code(self):
858-
return 200
909+
def __init__(self):
910+
self._sent = False
859911

860-
@property
861-
def headers(self):
862-
return {}
912+
def __enter__(self):
913+
return self
863914

864-
def read(self):
865-
return b""
915+
def __exit__(self, *exc):
916+
return False
866917

867-
def iter_lines(self):
868-
yield 'data: {"choices": [{"delta": {"content": "partial"}}]}'
869-
raise httpx.ReadError("connection reset")
918+
@property
919+
def status_code(self):
920+
return 200
870921

871-
with mock.patch.object(c._http, "stream", return_value=DieAfterDelta()):
872-
deltas = []
873-
with self.assertRaises(ApiError):
874-
c.chat([Message(role="user", content="hi")], on_delta=deltas.append)
875-
self.assertEqual(deltas, ["partial"])
876-
finally:
877-
c.close()
922+
@property
923+
def headers(self):
924+
return {}
925+
926+
def read(self):
927+
return b""
928+
929+
def iter_lines(self):
930+
yield 'data: {"choices": [{"delta": {"content": "partial"}}]}'
931+
raise httpx.ReadError("connection reset")
878932

879933

880934
class TestClientResetHttp(unittest.TestCase):

tests/test_subagent.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ def __init__(self):
2020

2121
def chat(self, messages, tools=None, system=None, temperature=None,
2222
max_tokens=None, reasoning_effort=None, on_delta=None, stream=True,
23-
cancel_check=None):
23+
cancel_check=None, on_retry=None):
2424
self.systems.append(system)
2525
return Message(role="assistant", content="sub-agent done"), Usage(input_tokens=10)
2626

tests/test_subagent_isolation.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ def __init__(self, script):
2424

2525
def chat(self, messages, tools=None, system=None, temperature=None,
2626
max_tokens=None, reasoning_effort=None, on_delta=None, stream=True,
27-
cancel_check=None):
27+
cancel_check=None, on_retry=None):
2828
self.n += 1
2929
self.sent.append([m.to_api() for m in messages])
3030
self.sent_tools.append([t.name for t in tools] if tools else None)

tests/test_todos_scope.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ def __init__(self, sub_todos):
2222

2323
def chat(self, messages, tools=None, system=None, temperature=None,
2424
max_tokens=None, reasoning_effort=None, on_delta=None, stream=True,
25-
cancel_check=None):
25+
cancel_check=None, on_retry=None):
2626
self.n += 1
2727
self.sent_tools.append([t.name for t in tools] if tools else None)
2828
if self.n == 1:

0 commit comments

Comments
 (0)