Skip to content

Commit 35a0e5e

Browse files
committed
fix(cli): do not assume why a line was unreadable, and bound the retries
The previous commit caught ValueError from readline() and asserted it was a limit overrun. It only knew an exception had been raised. That mattered beyond the message. Skipping the line is safe only for the overrun case, where readline() has already discarded the line and resumed the transport before raising. readline() flattens LimitOverrunError into a bare ValueError, so the two are indistinguishable at the call site, and any other ValueError that consumes nothing would have spun the loop forever at 100% CPU while still not draining the pipe. That is worse than the freeze being fixed. Now: report the actual exception with repr() and no assumed cause, and bound consecutive failures at MAX_CONSECUTIVE_READ_ERRORS (100). Past that, re-raise so the outer handler reports that nothing is draining the child's stdout. The outer try/except is load-bearing, not vestigial: it is the escalation path for that re-raise, and it still covers console.print failures and non-ValueError transport errors. It does not swallow cancellation, since CancelledError derives from BaseException, so the auto-reload path that cancels these tasks is unaffected (verified). (cherry picked from commit 13fa1f5c28f1687f1f90469c22eba9bccf1bbc82)
1 parent ef6ff46 commit 35a0e5e

1 file changed

Lines changed: 22 additions & 5 deletions

File tree

src/agentex/lib/cli/handlers/run_handlers.py

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@
2828
# echoed by validation errors), so give the reader room before it has to drop one.
2929
SUBPROCESS_STREAM_LIMIT = 8 * 1024 * 1024
3030

31+
# How many consecutive unreadable lines to skip before giving up on the stream.
32+
# Skipping is only known-safe for the limit-overrun case; this bounds the damage
33+
# if some other error repeats without consuming anything.
34+
MAX_CONSECUTIVE_READ_ERRORS = 100
35+
3136

3237
class RunError(Exception):
3338
"""An error occurred during agent run"""
@@ -255,19 +260,31 @@ async def stream_process_output(process: asyncio.subprocess.Process, prefix: str
255260
try:
256261
if process.stdout is None:
257262
return
263+
consecutive_read_errors = 0
258264
while True:
259265
try:
260266
line = await process.stdout.readline()
261267
except ValueError as e:
262-
# Line longer than the stream limit. readline() has already
263-
# dropped it (or cleared the buffer) and resumed the transport,
264-
# so continuing is safe and always makes progress.
268+
# readline() raises ValueError when a line exceeds the stream limit.
269+
# In *that* case it has already discarded the line and resumed the
270+
# transport, so skipping it makes guaranteed progress. Any other
271+
# ValueError carries no such guarantee, and retrying it forever would
272+
# spin without draining. We cannot tell the two apart (readline
273+
# flattens LimitOverrunError into a bare ValueError), so bound the
274+
# retries and let the outer handler report the hang risk.
275+
consecutive_read_errors += 1
276+
if consecutive_read_errors > MAX_CONSECUTIVE_READ_ERRORS:
277+
raise
265278
logger.warning(
266-
f"Dropped an oversized log line from {prefix} ({e}); "
267-
f"raise limit= on this process's create_subprocess_exec if it recurs."
279+
f"Skipping an unreadable line from {prefix}: {e!r} "
280+
f"(consecutive failure {consecutive_read_errors}/{MAX_CONSECUTIVE_READ_ERRORS}). "
281+
f"If this says the chunk exceeded the limit, raise limit= on this "
282+
f"process's create_subprocess_exec."
268283
)
269284
continue
270285

286+
consecutive_read_errors = 0
287+
271288
if not line:
272289
break
273290

0 commit comments

Comments
 (0)