Python client: stdio retry budget never resets on progress, so long-lived sandbox and exec streams die on a routine drop

[shehio]

Summary

Long-lived sandbox and exec stdio streams eventually fail on a routine, individually retryable connection drop, because _stream_stdio_with_retries in py/modal/_utils/task_command_router_client.py scopes its reconnect budget to the lifetime of the read instead of to a single interruption. A one-line change fixes it. A ready patch with a regression test is linked at the bottom; this is filed as an issue because the repository does not accept pull requests.

Verified present at current main (de0d692).

What You See

A stream of sandbox output (Sandbox stdout/stderr, or the output of sandbox.exec(...)) that has been running for a long time dies with a raw StreamTerminatedError, GRPCError, or ConnectionError, even though the sandbox itself is healthy and the drop that killed the read is of the same transient kind the client had been quietly recovering from all along. The failure looks random: the read survives several interruptions, sometimes long ones, and then dies on a shorter one later.

There is also no workaround from outside the client. The three tuning knobs (stream_stdio_retry_delay_secs, stream_stdio_retry_delay_factor, stream_stdio_max_retries) are keyword-only constructor arguments on TaskCommandRouterClient that nothing forwards: _connect instantiates the class without them, none of the init* classmethods accept them, and there is no entry in modal/config.py and no environment variable. Every real caller gets the defaults.

(This is distinct from #4121, which was about missing keepalives on an idle channel. Here every drop is legitimately transient and individually retryable; the budget accounting is what kills the read.)

Mechanism

_stream_stdio_with_retries initializes its retry state once, above the while True reconnect loop:

delay_secs = self.stream_stdio_retry_delay_secs        # default 0.01
delay_factor = self.stream_stdio_retry_delay_factor    # default 2
num_retries_remaining = self.stream_stdio_max_retries  # default 10

Inside the chunk loop, only half of that state is restored after a successful chunk:

# Reset retry backoff after any successful chunk.
delay_secs = self.stream_stdio_retry_delay_secs

num_retries_remaining is only ever decremented, so:

1. A stream gets ten reconnects in total, however long it lives. Nine drops spread over hours of otherwise healthy streaming leave one reconnect for the tenth; the next drop after that raises. Clean streaming earns nothing back.
2. The uncapped delay doubling means a single interruption can spend the whole budget at once: the sleeps sum to 0.01 * (2**10 - 1), about 10.2 seconds, so one outage a little longer than ten seconds exhausts all ten attempts in one burst and raises.

Both behaviors ignore how healthy the stream has been, which is what makes the failure feel random to users.

The Fix and Why It Cannot Spin

Restore the count in the same place the delay is already restored:

delay_secs = self.stream_stdio_retry_delay_secs
num_retries_remaining = self.stream_stdio_max_retries

The budget then covers a single interruption rather than the read's lifetime. The justification is the one the existing delay reset already relies on: this branch runs only when a chunk was actually received, which means the reconnect succeeded and the offset advanced. Every reset is paid for with delivered bytes, so the reconnect loop cannot spin:

• A stream that never yields anything can never reach the reset; it exhausts the budget and raises exactly as before (the existing test_exec_stdio_read_unavailable_forever_raises_grpcerror covers this).
• A stream that yields and then dies repeatedly makes forward progress on every cycle.
• Deadline handling is untouched; a read with a deadline still terminates on it.

The unbounded delay growth is left alone on purpose. It is arguably a separate pacing question (with no cap, raising stream_stdio_max_retries stops being useful long before the count runs out), and it is not needed to fix the scoping bug.

Regression Test

The patch adds test_exec_stdio_read_retry_budget_resets_after_a_successful_chunk: a fake stream delivers exactly one chunk per attempt and is then terminated, four drops against a budget of two, never more than one drop between chunks. The test asserts the full byte sequence arrives and that the attempt count is one connection per chunk plus the final attempt that sees the stream end. On unpatched main it fails with StreamTerminatedError; with the fix, the full file passes (26 passed) under the CONTRIBUTING.md setup (Python 3.11, uv pip install -e . --group dev, inv protoc). ruff check and ruff format --check are clean on both touched files.

Ready Patch

The fix plus the regression test are staged here, rebased on current main: shehio#1 (branch reset-stdio-retry-budget-on-progress). Since this repository does not accept pull requests, I am happy to send the change through whatever channel Modal prefers for external contributions, or the team is welcome to just take the diff.

🤖 Generated with Claude Code